Repository navigation
Tell an agent's owner to disconnect this connection, not rotate its secret - #851
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
513a778 to
64b77e8
Compare
|
@codex review |
|
@cubic-dev-ai review |
@jeremy I have started the AI code review. It will take a few minutes to complete. |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Rebased onto main (943dbb5) with no conflicts. Review threads: 1 resolved (1 fixed, 0 declined).
CI is green on 64b77e8. Codex reported on 64b77e8 with no major issues, and cubic found no issues. Copilot can't review because the requester's quota is exhausted. |
auth logout and auth revoke on an agent profile said to rotate the client secret in Basecamp to end the agent's access. My > Agent has no rotate control (reconnecting is the rotation), and a rotation leaves the agent's tokens alive for their hour anyway. Disconnect is what ends its access, so that is what both now say. auth agent's help also stops calling every agent one an administrator created: a personal agent is created by its owner when they connect it.
The Agents section still told the owner to rotate the client secret to end an agent's access, the advice auth logout and auth revoke just stopped giving. Disconnecting is what ends it.
Basecamp is about to let a personal agent hold several connections, one per computer or profile, each with its own secret. Then "disconnect the agent" is My > Agent's Disconnect all, which ends every computer, and rotating the client secret rewrites only the first connection's. auth logout and auth revoke now say to disconnect this connection in Basecamp. That is right today as well: an agent has one connection, and disconnecting it is what My > Agent's Disconnect does. The README says where to find it for a personal agent and a shared one, and that Disconnect all, once My > Agent lists several, ends every one.
64b77e8 to
1a7e93a
Compare
|
@cubic-dev-ai review |
@jeremy cubic can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 80,346 of the 80,000 allowed lines of code this month. Reviews resume on 1 November 2026 (in 18 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a7e93a1c0
ℹ️ 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".
…live "This connection stays live until you disconnect it" is false when it was already disconnected, or replaced from another computer, which is often why someone is logging out. Saying what to do, as auth revoke's hint does, is true either way.
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Rebased onto main (36a58d7) and reworded for agents with several connections: logout and revoke now say "disconnect this connection in Basecamp". The wording is true before and after bc3 step 6, so this can land before #900. The PR description says why. Review threads: 1 resolved (0 fixed, 1 declined).
CI is green on 0904fa5. Copilot approved 0904fa5. Codex reported on 0904fa5 with no major issues. cubic couldn't review: the workspace has used its monthly review limit, which resets on 1 November. Trial merges with #900 (7ba7e96) and #901 (8ead732) are clean. |
basecamp auth logoutandauth revokeon an agent profile say "rotate the client secret in Basecamp" to end the agent's access. This changes both to "disconnect this connection in Basecamp".Why this connection
Rotating is wrong today. My > Agent has no rotate control (basecamp/bc3#13597 removed it), and a rotation leaves outstanding self-tokens valid until they expire. Disconnecting revokes the secret and every token at once.
This PR first said "disconnect the agent". That stops being right once a personal agent can hold several connections, one per computer or profile, each with its own secret (plan; basecamp/bc3#13817, basecamp/bc3#13822, basecamp/bc3#13819). After bc3 step 6 turns that on:
So both messages now point at the one connection this profile holds.
True before and after step 6
I chose wording that is right in both worlds, rather than gating it on step 6 the way #900 is, so this can land now and fix today's wrong advice:
The one-line messages say "in Basecamp" rather than "on My > Agent", and don't name Disconnect all. A shared agent is disconnected in Adminland, and the CLI can't tell a shared agent from a personal one. Disconnect all doesn't exist until step 2 ships. The README has room for both, and words the Disconnect all warning so it only applies once there are several connections.
Change
auth logout:Logged out (forgot the agent credential; disconnect this connection in Basecamp to end its access).profile deleteon an agent profile says the same. It says what to do rather than that the connection is still live, which is false when it was already disconnected or replaced.auth revokehint:Forget the credential locally instead (basecamp auth logout), and disconnect this connection in Basecamp to end its access.RevokeSkippedAgentcomment says why neither rotating nor disconnecting the whole agent is the remedy.auth agent --helpno longer says every agent is "an identity your Basecamp administrator created". A personal agent is created by its owner when they connect it.The JSON envelope is unchanged:
reason: "agent",revoked: false.Pairing with #900 and step 6
This lands before or with #900 (step 4, CLI copy) and joins bc3 step 6's pairing, so it is never later than step 6. It's copy only.
Trial merges (
git merge-tree) on current main: this merges cleanly with #900 and with #901. It sharesREADME.mdandinternal/commands/auth_agent.gowith #900, in different hunks. It shares no files with #901.Two lines are left to #900 on purpose. They're its lines, they stay true until step 6, and #900 already rewrites both:
auth agent connect --help;Tests
TestAuthLogoutHumanCopy/agentpins the whole summary. It also checks that "rotate" doesn't appear, that no revocation is sent, and that the credential is forgotten.TestAgentRevokeIsRefusedAndKeepsTheCredentialpins the hint.TestAgentLogoutSaysRotatingTheSecretIsTheRemedyis renamedTestAgentLogoutForgetsWithoutRevoking. It never asserted the wording.make checktargets pass. The only failures are tests that need a terminal, which fail the same way on main here, and a lint finding inproctime_darwin.gothat main already has.