Skip to content

refactor(auth): delegate status + logout to @doist/cli-core/auth#72

Open
scottlovegrove wants to merge 2 commits into
mainfrom
scottl/cli-core-auth-status
Open

refactor(auth): delegate status + logout to @doist/cli-core/auth#72
scottlovegrove wants to merge 2 commits into
mainfrom
scottl/cli-core-auth-status

Conversation

@scottlovegrove
Copy link
Copy Markdown
Contributor

Summary

Mirrors Doist/twist-cli#222 for outline-cli. Replaces the hand-rolled ol auth status and ol auth logout action handlers in src/commands/auth.ts with cli-core's attachStatusCommand / attachLogoutCommand registrars, against the same shared OutlineTokenStore instance attachLoginCommand already uses.

  • ol auth status and ol auth logout gain --json / --ndjson for free from cli-core.
  • Status payload (--json / --ndjson): {id, name, email, team, baseUrl, source}.
  • Logout payload (--json): {ok: true}; --ndjson is silent (per the registrar contract).
  • A 401 from auth.info now throws CliError('NO_TOKEN', …) with a "re-authenticate" hint, replacing the ad-hoc AUTH_VERIFICATION_FAILED path.
  • No token at all → CliError('NOT_AUTHENTICATED', …).
  • Added 'NO_TOKEN' to outline's local ErrorCode union (cli-core's AuthErrorCode only declares NOT_AUTHENTICATED).
  • SKILL_CONTENT / skills/outline-cli/SKILL.md updated for the new --json / --ndjson flags.

Skipped vs twist-cli#222 — both intentional, since outline-cli has no keyring:

  • No getLastStorageResult / getLastClearResult getters on the store.
  • No onCleared warning hook on logout.
  • No revokeToken hook (outline doesn't call /oauth/revoke today; out of scope).
  • No dependency bump — already on @doist/cli-core@0.12.0, well above the 0.10.0 floor.

Test plan

  • npm run type-check
  • npm test — 127/127 across 17 files
  • npm run build
  • npm run format (oxfmt + oxlint via lefthook on commit)
  • npm run check:skill-sync
  • Manual: ol auth status (human + --json + --ndjson) against a real Outline instance
  • Manual: ol auth status with an invalid OUTLINE_API_TOKENNO_TOKEN envelope
  • Manual: ol auth status with no token at all → NOT_AUTHENTICATED envelope
  • Manual: ol auth logout + ol auth logout --json round-trip
  • Manual: full ol auth loginstatuslogout round-trip to confirm the shared store still works end-to-end

🤖 Generated with Claude Code

Replaces hand-rolled `ol auth status` / `ol auth logout` with cli-core's
`attachStatusCommand` / `attachLogoutCommand` registrars (mirrors
Doist/twist-cli#222). Both subcommands gain `--json` / `--ndjson` for
free, and status now translates 401 into the standard `NO_TOKEN`
envelope with a re-auth hint instead of the ad-hoc
`AUTH_VERIFICATION_FAILED` path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove self-assigned this May 16, 2026
Copy link
Copy Markdown
Member

@doistbot doistbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR replaces the hand-rolled auth status and auth logout commands with their @doist/cli-core equivalents, bringing built-in machine-readable output and standardized error handling. While the delegation successfully reduces boilerplate and aligns the CLI with internal patterns, the strict reliance on the token store inadvertently breaks environment variable authentication. A few adjustments will help polish this up, including reverting the status command to properly support environment-based tokens, removing PII from the machine-readable payloads to comply with AI tooling standards, resolving minor type duplications and forced casts, and adding automated tests for the new output formats.

Share FeedbackReview Logs

Comment thread src/commands/auth.ts
Comment thread src/commands/auth.ts Outdated
Comment thread src/commands/auth.ts Outdated
Comment thread src/commands/auth.ts Outdated
Comment thread src/commands/auth.ts
Comment thread src/commands/auth.ts
Comment thread src/commands/auth.ts Outdated
Comment thread src/commands/auth.ts Outdated
- Restore env-token support: `OutlineTokenStore.active()` now returns a
  placeholder snapshot when `OUTLINE_API_TOKEN` is set, so
  `attachStatusCommand` / `attachLogoutCommand` work end-to-end for
  env-only users. `fetchLive` re-derives the canonical account from
  `auth.info` so the placeholder id/label are never rendered.
- Drop PII (`name`, `email`) from the `auth status --json` / `--ndjson`
  envelope per the Internal AI Tools standard. Payload is now
  `{id, team, baseUrl, source}`.
- Slim the `statusData` stash to the two fields that don't round-trip
  through `account` (`email` for human render, `source` for both modes).
  Renderers read shared fields from `account` directly.
- Derive `source` from `process.env.OUTLINE_API_TOKEN` instead of an
  extra `getTokenSource()` disk read — `fetchLive` already proved the
  token works, the only remaining question is env vs config.
- Export `AuthInfoResponse` from `auth-provider.ts` and drop the
  duplicate in `auth.ts`.
- Narrow the error in `fetchLive`'s 401 branch with `instanceof Error`
  rather than a forced `as Error` cast.
- Add status + logout integration tests in `auth-command.test.ts`
  covering: human / `--json` / `--ndjson` renders, 401 → `NO_TOKEN`
  translation, `NOT_AUTHENTICATED` when nothing is stored, and the
  three logout output modes.
- Regenerate `skills/outline-cli/SKILL.md` for the trimmed JSON payload.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove requested review from a team and rmartins90 and removed request for a team May 16, 2026 08:51
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

👀 Show PR PR must be reviewed before or after merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants