Repository navigation
Conversation
Copy for multiple connections per agent (step 4 of the plan). Once Basecamp lets a personal agent hold several connections, connecting on another computer no longer ends this one, so: - The connector's and guided setup's refusal says this computer's connection was disconnected in Basecamp, without "or connected on another computer". - auth agent connect help says each approval adds a connection, that a personal agent's others keep working, that a shared agent keeps one, and that disconnecting this connection leaves the others. - connect help says a personal agent can run on several computers, and connect setup help that two connections serving one project both answer. - A lost handover says to disconnect the new connection, not the agent, so nobody reaches for Disconnect all. - README and the basecamp-connect skill drop "every connection rotates the agent's secret". Merges with bc3 step 6; before it, connecting elsewhere still replaces.
…s what both admit Pi's review: the connector's lock is local, so a personal agent's other computers can run one; and two connections serving one project each answer only what their own trust admits.
There was a problem hiding this comment.
🟢 Approved
The copy changes are internally consistent, safely rendered, and covered by focused assertions where applicable.
0 open findings
What changed in this PR
Updates CLI documentation and messages for Basecamp’s multiple-agent-connections model without changing behavior.
Changes:
- Clarifies personal versus shared agent connection behavior.
- Updates disconnect and lost-handover guidance.
- Aligns README, command help, skill guidance, and tests.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
skills/basecamp-connect/SKILL.md |
Updates connection and recovery guidance. |
README.md |
Documents multiple personal-agent connections. |
internal/commands/connect.go |
Expands connector and setup help. |
internal/commands/connect_setup_guided.go |
Revises refused-connection warning. |
internal/commands/connect_setup_guided_test.go |
Updates warning assertion. |
internal/commands/connect_run.go |
Revises disconnect error copy. |
internal/commands/connect_run_disconnect_test.go |
Updates disconnect-message tests. |
internal/commands/auth_agent.go |
Documents connection lifecycle and naming. |
internal/auth/agent_connect.go |
Revises lost-handover remediation. |
internal/auth/agent_connect_test.go |
Updates remediation assertions. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Pi's second pass: a held or shadow connector admits without handing off, so admission is not a guarantee of an answer.
The lock sentence grew the skill past the 14,000-character budget that TestTheConnectSkillKeepsEachRequestsStepsInsideTheCompactionCut holds; say it in fewer words.
|
🤖 Review summary for head Who read which commit
Fixed
Declined
There are no open review threads. |
jeremy
left a comment
There was a problem hiding this comment.
Reviewed at 7ba7e96. This assumes option (a) for #851.
- Gate this on step 6's switch, not its merge.
- Step 6 ships behind a switch, and switch-off brings back replace-on-connect. In that state:
- The plugin tracks main with no pinned ref, so merging alone publishes the skill.
- Suggest:
- "…was disconnected in Basecamp, or replaced by a newer connection" in both refusal strings (guided);
- have the skill ask whether the agent is shared rather than assume, since that can't be known before approval;
- merge and release once the switch is on in production.
- Logout and revoke. auth.go:1610 and revoke.go:197 say "rotate the client secret". After step 6 that rewrites only connection #1's secret, so a lane on another connection that follows it stays live and cuts #1's computer instead. I'll change #851 to "disconnect this connection on My > Agent (Disconnect all ends every computer)" so it can land before this; please add #851 to step 6's pairing.
- The SKILL.md lock line conflicts with #901. L61-L63 is the only conflicting hunk, and either side taken whole is wrong after #901. Drop this edit, or use "If another connector already holds its lock, it refuses to start and says so."
- The lost-handover hint. With several profiles on one host, every row carries the hostname. This hint could say "the newest connection, last in the list; it can wait, its secret is gone". The
--device-namehelp could add "give each profile its own name when several on this computer connect the same agent". - Name the profile in the refusal. On a host with several connections, "This computer's connection" reads as the whole machine. The profile is already a parameter.
- credentials.json. After this bullet, add: if nothing uses that file any more, disconnect its connection on My > Agent and remove the file. It stays a live secret now.
- Overlap where projects are chosen. Guided setup defaults Yes to all projects. Printing the overlap sentence just before that question would catch a second computer.
- A contract test that pulls the stop fragment and matches it against
errAgentDisconnected, next toTestTheSkillDocumentsTheRequestLineItGets.
Merge with bc3 step 6 ("Turn on adding connections"). Several lines here describe behaviour Basecamp doesn't have until that step ships; they're listed below.
Step 4, "CLI copy", of the plan in Follow-up: multiple connections per agent (card description, "Plan (final)"; adjusted in this comment). Copy only: no behaviour changes.
What changes
Once a personal agent can hold several connections, connecting on another computer no longer ends this one. The CLI's copy is updated to match:
basecamp connect, and the lineconnect statuskeeps) and guidedconnect setupnow sayThis computer's connection to <agent> was disconnected in Basecamp. They no longer say "or connected on another computer".auth agent connect --help. Each approval adds a connection with its own secret, named after this computer. A personal agent's other connections keep working. A shared agent keeps one connection and replaces it. Connecting the same profile again leaves the old personal connection in Basecamp until it's disconnected. Disconnecting this connection leaves the others alone.--device-namenow names the connection.connect --help. Eachauth agent connectadds a connection, so a personal agent can run a connector on several computers.connect setup --help. The project list belongs to the profile. When two connections serve the same project, a request both admit there can be answered twice.basecamp-connectskill. "Every connection rotates the agent's secret" is gone. The skill's personal-vs-shared advice on reconnecting is updated, and so is the refusal text it matches. Its rule that only one connector per agent can run now says "on this computer", because the lock is local.shared agent/personal agentare the words bc3 uses in Adminland and on agent profile cards.False before bc3 step 6
Until step 6 ships, a second connect still replaces the first, for every agent. These lines are wrong or incomplete until then:
auth agent connecthelp: "A personal agent's other connections … keep working", and "the old one stays in Basecamp until you disconnect it there".connecthelp: "a personal agent can run a connector on several computers at once".basecamp-connectskill: the credentials.json bullet ("both keep working"), "adds another connection", and the full-access reconnect bullet.--device-name"in Basecamp": the name appears in the connections list from bc3 step 2.These are true both before and after: the lost-handover remedy (before step 6, the new connection is the only one), the
connect setupoverlap note (it can't happen before step 6), and "disconnecting this connection kills the secret this holds".Left alone, on purpose
auth logout/auth revokeand the README's "rotate the client secret" line. This PR doesn't touch those lines, so the two don't conflict. Once both land with step 6, Tell an agent's owner to disconnect this connection, not rotate its secret #851's "disconnect the agent in Basecamp" should probably read "disconnect this connection", for the same Disconnect-all reason as the handover remedy. That's a follow-up for whichever lands second.connect_setup_guided.go(the "Before anything is connected" comment),auth/agent.go(ErrAgentCredentialRefused),connect_run.go(connectorStoppedBy),agent_connect.go(adoptAgentGrantOrSayWhatWasLost), and the test comments inconnect_run_disconnect_test.go,connect_setup_test.goandskill-evals/cases/basecamp-connect/unconfirmed-identity.yml. Each one still holds for shared agents.Tests
connect_run_disconnect_test.go,connect_setup_guided_test.go, and the six lost-handover assertions ininternal/auth/agent_connect_test.go.internal/commands, 6 ininternal/auth. They pass with the change.TestHintCommandsResolvechecks the newconnect.gohelp, andmake check-skill-driftchecks the skill.bin/ciis green locally.