Skip to content

refactor(cli): auth/workspace cleanup — record-backed token store#37219

Open
GareArc wants to merge 4 commits into
mainfrom
fix/wta-479-cli-auth-cleanup
Open

refactor(cli): auth/workspace cleanup — record-backed token store#37219
GareArc wants to merge 4 commits into
mainfrom
fix/wta-479-cli-auth-cleanup

Conversation

@GareArc

@GareArc GareArc commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

WTA-479. Consolidates the difyctl auth and workspace layer.

Token store

  • Add TokenStore { read/write/remove(host, email) } with two backends:
    • FileTokenStore — versioned nested YAML; host/email are opaque keys, never dot-split. Reads legacy pre-v1 files (no version header) transparently; next write stamps version: 1.
    • KeychainTokenStore — one keyring Entry per (host, email); entry name byte-identical to the legacy key for back-compat.
    • All credential consumers route through it; tokenKey removed.
  • Backend selection split: detectTokenStore probes the keyring at login (where a write happens anyway) and persists the chosen mode to the registry; getTokenStore(mode) constructs the recorded backend on read paths without probing, so reads no longer mutate the keyring. Dispatch flows through a single TOKEN_STORE_OPENERS table.
  • Login probe cleanup is guarded in a finally so a failed probe read never orphans the probe entry in the keyring.
  • KeychainTokenStore.read and .write distinguish keyring-unavailable from credential-absent: a backend access failure throws KeyringUnavailable (exit 1, "keychain unreachable") instead of returning '' and surfacing as a false "not logged in" (exit 4).
  • Logout is resilient to a locked keychain: read() failure is caught in both call sites; local credential cleanup (hosts.yml) always runs.
  • Single StorageMode source of truth (STORAGE_MODES) shared by the type and the zod schema.

Workspace

  • Rewrite use workspace as an interactive picker: arg → direct switch; no-arg + TTY → list + select; no-arg + no-TTY → error. Persists only the active workspace from the switch response.
  • Drop the cached available_workspaces field, its readers, and the implicit-default-workspace fallback. Resolver is now flag → env → ctx.workspace.id → throw.

Testing

  • pnpm type-check, pnpm lint, pnpm test all green (985 passed / 16 skipped).

@GareArc GareArc force-pushed the fix/wta-479-cli-auth-cleanup branch from d1c293a to 390aaef Compare June 9, 2026 08:33
@GareArc GareArc marked this pull request as ready for review June 9, 2026 09:39
@GareArc GareArc requested a review from a team as a code owner June 9, 2026 09:39
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. refactor labels Jun 9, 2026
@GareArc GareArc enabled auto-merge June 9, 2026 09:39
GareArc added 4 commits June 9, 2026 03:14
WTA-479. Consolidates the difyctl auth and workspace layer.

Token store:
- Add TokenStore { read/write/remove(host,email) } with two backends:
  FileTokenStore (versioned nested YAML, never dot-splits host/email) and
  KeychainTokenStore (one keyring Entry per host+email, entry name byte-
  identical to legacy for back-compat). Route all credential consumers
  through it; delete tokenKey. File-mode users re-login once; keychain
  users are unaffected.
- Split backend selection: detectTokenStore probes the keyring at login
  (where a write happens anyway) and persists the chosen mode to the
  registry; getTokenStore(mode) constructs the recorded backend on read
  paths without probing, so reads no longer mutate the keyring. Dispatch
  through a single TOKEN_STORE_OPENERS table.
- Guard the login probe cleanup in a finally so a failed probe read never
  orphans the probe entry in the keyring.
- Distinguish keyring-unavailable from credential-absent: KeychainTokenStore
  .read throws KeyringUnavailable on backend access failure instead of
  returning '', so a transient keyring outage surfaces as 'keychain
  unreachable' (exit 1) rather than a false 'not logged in' (exit 4).
- Single StorageMode source of truth (STORAGE_MODES) shared by the type
  and the zod schema.

Workspace:
- Rewrite 'use workspace' as an interactive live picker (arg -> direct
  switch; no-arg+TTY -> list+select; no-arg+no-TTY -> error), persisting
  only the active workspace from the switch response.
- Drop the cached available_workspaces field, its readers, and the
  implicit-default-workspace fallback. Resolver is now
  flag -> env -> ctx.workspace.id -> throw.
- logout no longer blocked when OS keychain is locked: read() throw is
  caught in both logout/index.ts and logout.ts; local credential cleanup
  (hosts.yml) always runs regardless of keychain state
- FileTokenStore transparently reads pre-upgrade tokens.yml (no version
  header = legacy format); next write stamps version: 1
- KeychainTokenStore.write() now wraps native keyring errors as
  KeyringUnavailable, matching the error surface of read()
- tests for all three: logout-with-locked-keychain, legacy-file
  migration, write-error wrapping
…ace validation

Rebased onto main which added UUID validation for --workspace flag and
DIFY_WORKSPACE_ID env values (#37212). Short fixture IDs ('ws-2', 'ws-9')
now fail that validation. Replace them with zero-padded UUIDs
(00000000-0000-0000-0000-000000000002 / ...0009) throughout unit tests
and the dify-mock fixture scenarios so all 996 tests pass.

Also forward-ports the DIFY_E2E_NO_KEYRING bypass from main into
detectTokenStore (replacing the old getTokenStore probe entrypoint).
…S strict fix

- resolver.ts: validate active-context workspace ID as UUID (same guard
  already applied to flag/env paths); throws UsageInvalidFlag with hint
  to run `difyctl use workspace` when stored ID is stale/non-UUID
- manager.ts: initialise `got` to '' so strict TS assignment analysis
  does not flag it as potentially-unassigned after a keyring read throw
- use/workspace/use.ts: change error code for "no workspaces available"
  from UsageMissingArg (exit 2 / usage) to AccessDenied (exit 4); it is
  a server-state/permissions issue, not a missing argument
- test fixtures: update ws-1 → aaaaaaaa-0000-0000-0000-000000000001
  across dify-mock scenarios, server, and all command tests so fixture
  data uses valid UUIDs consistent with the resolver's new validation
@GareArc GareArc force-pushed the fix/wta-479-cli-auth-cleanup branch from 4c262eb to e8b0850 Compare June 9, 2026 10:30
@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.59664% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.13%. Comparing base (19d2a4d) to head (e8b0850).

Files with missing lines Patch % Lines
cli/src/commands/auth/logout/index.ts 0.00% 3 Missing ⚠️
cli/src/store/token-store.ts 95.00% 3 Missing ⚠️
cli/src/commands/_shared/authed-command.ts 0.00% 2 Missing ⚠️
cli/src/commands/use/workspace/use.ts 94.11% 1 Missing ⚠️
cli/src/store/manager.ts 94.44% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #37219   +/-   ##
=======================================
  Coverage   86.12%   86.13%           
=======================================
  Files        4808     4809    +1     
  Lines      236734   236800   +66     
  Branches    43668    43682   +14     
=======================================
+ Hits       203897   203973   +76     
+ Misses      28934    28924   -10     
  Partials     3903     3903           
Flag Coverage Δ
cli 87.24% <91.59%> (+0.47%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@GareArc GareArc requested review from lin-snow and wylswz June 9, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant