Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16aa8ce4b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const session = readCliSession({ stateDir: options.stateDir }); | ||
| if (session && !isExpired(session.expiresAt, options.io?.now)) { | ||
| const refreshed = await refreshAgentToken({ |
There was a problem hiding this comment.
The session-refresh path runs before shouldUseCloudAuth, so any command with daemonBaseUrl plus a stored CLI session will call the cloud refresh endpoint even for non-cloud daemons (for example http://127.0.0.1 or an internal host). In those cases a network/cloud failure now blocks commands that previously relied on existing daemon auth behavior. This is visible because refresh is attempted unconditionally in resolveRemoteAuth, and only afterward does the code check whether cloud auth should apply.
Useful? React with 👍 / 👎.
| if (command !== 'auth') { | ||
| const authResolution = await resolveRemoteAuthForCli({ |
There was a problem hiding this comment.
runCli now resolves remote auth for every non-auth command, including connection. But connection status is implemented as local state inspection in src/cli/commands/connection.ts and does not require daemon access, so this change can make connection status --remote-config ... fail with login/token errors when credentials are missing. That regresses a read-only diagnostic path by adding an unnecessary auth precondition.
Useful? React with 👍 / 👎.
thymikee
force-pushed
the
feat/cloud-remote-auth
branch
from
16aa8ce to
662dc6d
Compare
|
Addressed both review comments in 662dc6d.
Validation after the fix:
|
|
1 participant
Summary
Add CLI-side cloud remote authentication for
connect --remote-config: env/service token resolution, stored CLI-session refresh into short-lived agent tokens, implicit interactive login, CI/non-interactive refusal, and explicitauth status|login|logoutmanagement.Cleanup pass after security/API review:
adc_*tokens, user/device codes, and auth URLsTouched files: 12. Scope stayed within CLI remote-auth/connection surface, command schema/routing, tests, docs, and skill guidance. Cloud server/dashboard endpoints are documented as required follow-up because this repo does not contain that service.
Validation
pnpm formatpnpm vitest run src/cli/__tests__/auth-session.test.ts src/utils/__tests__/args.test.ts src/utils/__tests__/diagnostics.test.tspnpm check:quickpnpm test:unit