Skip to content

Say a connect adds a connection, and a refusal means disconnected (merge with bc3 step 6) - #900

Open
robzolkos wants to merge 4 commits into
mainfrom
agent-connections/4-copy
Open

robzolkos wants to merge 4 commits into
mainfrom
agent-connections/4-copy

Conversation

@robzolkos

@robzolkos robzolkos commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

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:

  • Refusal. When Basecamp refuses the connection's secret, the connector (basecamp connect, and the line connect status keeps) and guided connect setup now say This 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-name now names the connection.
  • connect --help. Each auth agent connect adds 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.
  • Lost handover. When the CLI fails to keep a secret it was handed, it now says to disconnect the new connection, not the agent. Once My > Agent offers both Disconnect… and Disconnect all, "disconnect the agent" would point people at the button that breaks every other computer.
  • README and basecamp-connect skill. "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 agent are 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:

  • The refusal leaves out "connected on another computer", which is still a real cause today.
  • auth agent connect help: "A personal agent's other connections … keep working", and "the old one stays in Basecamp until you disconnect it there".
  • connect help: "a personal agent can run a connector on several computers at once".
  • README: the new paragraph under "Connecting an agent to this computer" (several connections, "leaves the others working", "rename it on My > Agent", which needs bc3 step 2).
  • basecamp-connect skill: 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 setup overlap note (it can't happen before step 6), and "disconnecting this connection kills the secret this holds".

Left alone, on purpose

  • Tell an agent's owner to disconnect this connection, not rotate its secret #851 (open) changes auth logout / auth revoke and 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.
  • Code comments. Comments that say connecting replaces the secret are left unedited: 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 in connect_run_disconnect_test.go, connect_setup_test.go and skill-evals/cases/basecamp-connect/unconfirmed-identity.yml. Each one still holds for shared agents.
  • Shared agents. The CLI can't tell a shared agent from a personal one, so the refusal says "disconnected in Basecamp" for both. A shared agent replaced by another computer's approval was disconnected by Basecamp, so the sentence still holds for it.

Tests

  • The copy tests now assert the new strings: connect_run_disconnect_test.go, connect_setup_guided_test.go, and the six lost-handover assertions in internal/auth/agent_connect_test.go.
  • I reverted the three production files and those tests failed: 4 in internal/commands, 6 in internal/auth. They pass with the change.
  • The repo keeps no golden help-output tests, so the help text has no test. TestHintCommandsResolve checks the new connect.go help, and make check-skill-drift checks the skill.
  • bin/ci is green locally.

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.
@robzolkos
robzolkos requested a review from a team as a code owner October 13, 2026 21:52
Copilot AI balanced review requested due to automatic review settings October 13, 2026 21:52
@github-actions github-actions Bot added commands CLI command implementations tests Tests (unit and e2e) skills Agent skills auth OAuth authentication docs labels Oct 13, 2026
Copilot AI previously approved these changes Oct 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 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 run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to 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.
Copilot AI balanced review requested due to automatic review settings October 13, 2026 21:56
Copilot AI dismissed their stale review, a newer Copilot review was requested October 13, 2026 21:56
Copilot AI previously approved these changes Oct 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approved

The coordinated copy changes are consistent, scoped, and covered by updated assertions.

0 open findings

🧠 Review effort: Balanced

The lock sentence grew the skill past the 14,000-character budget that
TestTheConnectSkillKeepsEachRequestsStepsInsideTheCompactionCut holds;
say it in fewer words.
Copilot AI balanced review requested due to automatic review settings October 13, 2026 22:08
Copilot AI dismissed their stale review, a newer Copilot review was requested October 13, 2026 22:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Its guidance intentionally depends on coordinated deployment of Basecamp step 6, which requires human rollout confirmation.

0 open findings

🧠 Review effort: Balanced

@robzolkos

Copy link
Copy Markdown
Collaborator Author

🤖 Review summary for head 7ba7e962. Hold until bc3 step 6 is ready to ship: several lines here are false until it does (listed in the description).

Who read which commit

  • Pi (gpt-6.1-sol, xhigh, adversarial, read-only), two passes:
    • Pass 1 read 27658202 (the whole change).
    • Pass 2 read the fix commit 6aa3faee.
  • Copilot:
    • Approved 6aa3faee with 0 findings.
    • Approved f178c3d7 with 0 findings.
    • Commented on 7ba7e962 with 0 findings. Its "needs a closer look" note is about the merge timing with step 6.
  • CI: green on 7ba7e962. Skill Evals were skipped because the repo has no ANTHROPIC_API_KEY.

Fixed

  • Pi 1: the skill said only one connector per agent can run anywhere. The lock is local, so it now says "on this computer" (6aa3faee).
  • Pi 1: "two connections serving one project both answer every request" overclaimed, since each answers only what its own trust admits (6aa3faee).
  • Pi 2: "a request both admit is answered by each" still overclaimed, because a held or shadow connector admits without handing off. It now says "can be answered twice" (f178c3d7).
  • CI: the skill grew past its 14,000-character compaction budget (TestTheConnectSkillKeepsEachRequestsStepsInsideTheCompactionCut). The sentence is shorter now (7ba7e962). My local bin/ci had run only on the first commit; I ran it again on 7ba7e962 and it's green.

Declined

  • Pi 1, README:242, auth/revoke.go:197, commands/auth.go:1610 still say "rotate the client secret". Tell an agent's owner to disconnect this connection, not rotate its secret #851 owns those lines, and touching them here would conflict with it. Once Tell an agent's owner to disconnect this connection, not rotate its secret #851 and step 6 have both landed, its "disconnect the agent" should read "disconnect this connection". That's a follow-up for whichever lands second.
  • Pi 1, "was disconnected" also fires for an inactive account (invalid_grant) and for bare 401/403. That classification is older than this PR, and the old message made the same claim ("was disconnected in Basecamp, or …"). Fixing it means changing behaviour in ErrAgentCredentialRefused, which is outside a copy-only step.
  • Pi 1, the new help prose has no test. The repo keeps no golden help-output tests. TestHintCommandsResolve covers the commands named in connect.go, and skill drift covers the skill. Testing prose by its wording would only freeze the wording.
  • Pi 2, "on this computer" is really "in this state directory" (XDG_STATE_HOME, shadow). Step 5 ("one connector per profile") rewrites this lock and its wording. "On this computer" is the case people actually hit, and it's narrower than the old "per agent", which had no scope at all.

There are no open review threads.

@jeremy jeremy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 refusal drops the real cause;
      • the help promises the other connections keep working;
      • the skill connects beside a credentials.json without warning, which kills it.
    • 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-name help 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 to TestTheSkillDocumentsTheRequestLineItGets.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auth OAuth authentication commands CLI command implementations docs skills Agent skills tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants