You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Answer an agent's Pings from one connector only - #936
Only one computer should answer an agent's Pings. A Ping is in no project, so serving projects can't divide Pings between connections the way it divides everything else: every connector for the agent sees all of them, and each one that admitted a line answered it. Once Basecamp adds connections (bc3 #13840), that is every Ping answered once per computer. This is the act-once rule for Pings, with no server change.
What a connector hears
Started
Hears Pings
--pings
yes, and anything else outside a project
--no-pings
no; only its projects
neither, agent's first connection (credential is the agent client's own secret)
yes
neither, a connection added after the first (secret starts bcac_)
no
neither, a person's login (a bot user such as Clawdito)
yes, as before
--project
no, as before; --pings --project is refused
Every connector still serves the projects it's started with, as before.
Kept per profile. The first start without --project saves the answer as pings.json beside the profile's ledger (either flag saves its own), and restarts keep it until a flag changes it. The start decides it while holding its instance lock, so another start of the same profile can't override it in between. A flag given false means the other flag (--pings=false is --no-pings). An unreadable pings.json refuses the start, and every surface that meets one names it to remove before choosing again.
Visible. The start line says pings=true|false and why. connect status (styled and --json) and connect doctor's Pings check read one view: the saved choice (always read, so a running connector can't hide an unreadable one), the connector running now from its lock (one an older build runs counts as "may hear"), and the other connectors on this computer for the same agent that hear Pings. They warn only when two would answer. The one answerer beside a connector that leaves Pings to it is named, not flagged. Guided setup uses the same view only to decide whether to suggest pinging the agent, and to say how to recover a saved choice that can't be read. How much more it should say is open for a decision; see the summary comment.
Warned about. The running connector writes whether it hears Pings beside its instance lock. A connector that hears Pings warns at start when another one for the same agent on this computer does too. An older build's connector counts as "may hear them", since it doesn't say. Connectors on other computers can't be seen, so starting just one there is the person's job, as ruled.
Admission and handoff. Admission discards a confirmed Ping's line as pings_not_heard when the run doesn't hear Pings, after the one Ping read and before any other. The handoff hands off a Ping's request only when the run hears Pings, and never on a run narrowed by --project. A test holds that outside a served project, nothing but a Ping's line is ever admitted, so this flag decides everything there.
Docs. The connect and connect setup help, README, docs/agent-connections-rollout.md and the basecamp-connect skill say "only one computer should answer Pings". Guided setup on an added connection no longer suggests pinging the agent to try it, and says how to make this computer the one that answers. The skill's new section sits after the per-request steps, which were 6 characters inside the compaction budget.
Calls I made that you may want to overrule
--pings with --project is refused instead of supported. A narrowed run's feed is filtered to its projects server-side, so it never receives a Ping. Supporting the pair would mean dropping that filter and separating the blocked-retry sweep's scope from the feed filter: a wider change to a guarded invariant, for a case connect setup --serve already covers. If you want it, it's a follow-up.
The first start saves its default, not only an explicit flag. Without this, reconnecting a profile where Basecamp adds connections would turn its credential into a bcac_ one and quietly stop it answering Pings. The cost: a profile that started out as an added connection keeps not hearing Pings even if it later becomes the only one. That's fixed with --pings.
A shadow reads the profile's choice and saves none, so it admits what the connector it shadows would. A flag on a shadow run applies to that run only.
The choice lives in the state directory, not in connect.json. connect.json is setup's to write, under the credential lock, and an older build's strict parse refuses an unknown key. A ledger table would need a schema migration that older builds refuse. Older builds never read pings.json, so a downgrade drops the choice and every connector hears Pings again, as before. The runbook says so.
Tests
Tests came first and failed before the code.
admission: a Ping's line from the operator (direct and group) or a person let into that Ping is discarded pings_not_heard, spending one Ping read; projects are untouched; and the invariant that nothing outside a served project is admitted without Pings, checked for every matrix type.
handoff: a Ping request is handed off only with Pings, and never on a narrowed run.
lock holder and pings.json: round-trip, an older holder reads as unknown, and malformed, foreign-readable or symlinked files are refused.
commands: the resolution table, then full runs against the fake Basecamp. They cover a first connection, an added connection across five restarts, --project, the refusals, and flags given false. They cover the second-connector warning, and its absence for --no-pings, for a shadow and for a stopped connector. They also cover the shadow mirroring the profile, a legacy holder not being counted against its own profile, and status, doctor and guided setup reporting a narrowed run.
Removing the line that hands the decision to admission fails three run tests.
bin/ci passes on Linux.
The one thing these tests don't do is run a Ping through a live feed into runConnect: the fake Basecamp here serves no account feed. The start line reads the decision from the admission policy itself, so the run tests catch a dropped wiring line, and the admission and handoff tests cover the behaviour.
Every connector for an agent sees all of its Pings: a Ping is in no
project, so serving projects can't divide them the way it divides the
rest, and each connector that admitted a line answered it. With several
connections per agent that is every Ping answered once per computer.
Only a connector started with --pings now hears Pings, and anything else
outside a project; one started with --no-pings hears only its projects.
Without either, the agent's first connection (its credential is the
agent client's own secret) hears them, a connection added after it (a
bcac_ secret) doesn't, and a person's login, such as a bot user's, does
as before. A run given --project still hears none, and --pings with
--project is refused, since its feed brings no Ping.
The first start saves the answer beside the profile's ledger
(pings.json), and restarts keep it until a flag changes it, so a profile
that answered Pings keeps answering after a reconnect turns its
credential into an added connection. A shadow reads the profile's
choice and saves none. The running connector says beside its lock
whether it hears Pings; a connector that hears them warns when another
for the same agent on this computer does too, and status and doctor say
which this profile's does and why.
Admission discards a confirmed Ping's line as pings_not_heard when the
run doesn't hear them, and the handoff hands off a Ping's request only
when it does.
Adds per-profile Ping handling so one connector can answer an agent’s Pings while others serve projects only.
Changes:
Adds persisted --pings / --no-pings choices and admission/handoff controls.
Exposes Ping handling through startup logs, status, doctor, and local duplicate warnings.
Updates setup guidance, documentation, and regression 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.
The start read the saved choice before it took the instance lock, so a
start of the same profile that saved another choice while this one
waited for its token was overruled. It now takes the lock first, and
the resolver takes the held lock as its argument; the lock's metadata
says whether the connector hears Pings, and why, once that is decided.
--pings=false and --no-pings=false were read as no flag at all. A flag
given false is now the other choice.
doctor and guided setup described the next un-narrowed start, so a
connector running with --project read as hearing Pings beside the one
that does. They now say what the running connector does, and doctor
adds what a restart would do when that differs. status --shadow reads
the profile's choice, which a shadow uses, rather than the shadow's
directory, which never has one. A profile whose state is still in the
legacy directory no longer counts an older build's connector on it as
a second one.
The skill says the flags came after CLI 0.16.0, without raising its
version gate past a release that doesn't exist yet.
The diagnostics took the running connector's Pings from its lock and
skipped the saved choice, so an unreadable pings.json passed doctor
while the next start would refuse it. And a connector an older build
runs, which doesn't say, was taken for none and reported by the choice
this build saved, which the older build ignores.
doctor now always reads the saved choice, and fails on one it can't
read. It reports the running connector from its lock, and one an older
build runs as one that may hear Pings, warning about a second beside
it. Guided setup and status say the same.
status, doctor and guided setup each interpreted the saved choice, the
running connector's lock and the other connectors on their own, and
each got a corner wrong. They now read one view of the profile:
- The saved choice is always read, and one that can't be read names
the file to remove before choosing again. A flag alone can't recover
it when the file isn't this user's own, since the choice is never
written over one. The start's refusal says the same.
- The running connector is taken from its lock only when the process
holding that pid started before the lock was taken, so stale metadata
whose pid was reused describes nothing running.
- Two connectors answering is the warning. The one answerer beside a
connector that leaves Pings to it is named, not flagged.
Guided setup no longer says an added connection leaves Pings to the
agent's first connection, which an explicit choice may have moved. It
doesn't advertise Pings a start would refuse, either. The docs say
the first start without --project saves the choice, since a narrowed
run saves nothing.
With no saved choice and a credential that can't be read, nothing says
what the next start would do, while the running connector's lock still
says what it does. doctor read the next start's choice first, which
wasn't there, and crashed; it now describes the running connector
first.
Counting duplicate answerers. This is the third round in that class. Recommendation: say "more than once" when two or more other connectors hear Pings, in the same push as the guided-setup change if approved. Thread: Answer an agent's Pings from one connector only #936 (comment)
Not reported on the head: Codex, whose review of a6afc4a has been running since 20:23Z.
A running connector no longer hides an unreadable pings.json: its next
start refuses either way, so the remove-then-choose remedy shows whether
or not one runs, and whatever the running one hears. A ping is suggested
only where this profile's connector hears them, which a start that would
refuse doesn't. Guided setup speaks for this profile alone; other
connectors and duplicate answers stay with the start's warning, status
and doctor.
Beside one other connector that hears the agent's Pings, each is
answered twice; beside two or more, more than once. Doctor now says so,
and a starting connector logs one warning naming every other one rather
than an "answered twice" line per peer.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Cross-profile persistence and live-feed integration need human validation, and the doctor remediation issue remains unresolved.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Fix no-pings hint to disable all but one peer
internal/commands/connect_doctor.go:240
When this profile runs with --no-pings but two other profiles hear Pings, Twice() correctly warns about those peers. However, this hint gives a command for the already-disabled profile, so following it leaves both answerers unchanged. The older-peer hint below also offers that ineffective remedy. When hears is false, direct the person to restart all but one of the named peers with --no-pings, upgrading older builds first.
Beside two or more connectors that hear the agent's Pings, doctor on a
profile that hears none warned, then offered to restart that same
profile with --no-pings, which changes nothing. It now says this one
doesn't answer them and to restart all but one of the others, upgrading
any older one first, since an older build can't be told.
Fixed — the previously missed doctor hint in #936 (review): in bf16354, doctor on a profile that hears no Pings, beside two or more connectors here that do, no longer offers to restart that same profile with --no-pings, which changed nothing. Its hint now says this one doesn't answer them and to restart all but one of the others, upgrading any older one first. TestDoctorBesideTwoAnswerersPointsAtThem covers both cases and failed before the change. This is the remedy doctor offers, not how duplicates are counted, so it was fixed rather than declined under the operator's settlement.
Review threads: 4 resolved (3 fixed, 1 declined with the reasoning in each thread).
Review bodies: 1 finding answered (1 fixed, 0 declined).
Fixed from a review body:
The operator settled both recurring classes on this PR, and further findings in them will be declined, citing this comment: guided setup reports only this profile's own Pings choice, shows the remove-then-choose remedy whenever that choice can't be read, and suggests a ping only when this connection hears them (4252976); duplicate answerers are "twice" beside one other and "more than once" beside two or more, in doctor and in one start warning (2a5add4). Copilot and Codex both reported on bf16354 with nothing open.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
commands
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Only one computer should answer an agent's Pings. A Ping is in no project, so serving projects can't divide Pings between connections the way it divides everything else: every connector for the agent sees all of them, and each one that admitted a line answered it. Once Basecamp adds connections (bc3 #13840), that is every Ping answered once per computer. This is the act-once rule for Pings, with no server change.
What a connector hears
--pings--no-pingsbcac_)--project--pings --projectis refusedEvery connector still serves the projects it's started with, as before.
--projectsaves the answer aspings.jsonbeside the profile's ledger (either flag saves its own), and restarts keep it until a flag changes it. The start decides it while holding its instance lock, so another start of the same profile can't override it in between. A flag given false means the other flag (--pings=falseis--no-pings). An unreadablepings.jsonrefuses the start, and every surface that meets one names it to remove before choosing again.pings=true|falseand why.connect status(styled and--json) andconnect doctor's Pings check read one view: the saved choice (always read, so a running connector can't hide an unreadable one), the connector running now from its lock (one an older build runs counts as "may hear"), and the other connectors on this computer for the same agent that hear Pings. They warn only when two would answer. The one answerer beside a connector that leaves Pings to it is named, not flagged. Guided setup uses the same view only to decide whether to suggest pinging the agent, and to say how to recover a saved choice that can't be read. How much more it should say is open for a decision; see the summary comment.pings_not_heardwhen the run doesn't hear Pings, after the one Ping read and before any other. The handoff hands off a Ping's request only when the run hears Pings, and never on a run narrowed by--project. A test holds that outside a served project, nothing but a Ping's line is ever admitted, so this flag decides everything there.connectandconnect setuphelp, README,docs/agent-connections-rollout.mdand the basecamp-connect skill say "only one computer should answer Pings". Guided setup on an added connection no longer suggests pinging the agent to try it, and says how to make this computer the one that answers. The skill's new section sits after the per-request steps, which were 6 characters inside the compaction budget.Calls I made that you may want to overrule
--pingswith--projectis refused instead of supported. A narrowed run's feed is filtered to its projects server-side, so it never receives a Ping. Supporting the pair would mean dropping that filter and separating the blocked-retry sweep's scope from the feed filter: a wider change to a guarded invariant, for a caseconnect setup --servealready covers. If you want it, it's a follow-up.bcac_one and quietly stop it answering Pings. The cost: a profile that started out as an added connection keeps not hearing Pings even if it later becomes the only one. That's fixed with--pings.pings.json, so a downgrade drops the choice and every connector hears Pings again, as before. The runbook says so.Tests
Tests came first and failed before the code.
pings_not_heard, spending one Ping read; projects are untouched; and the invariant that nothing outside a served project is admitted without Pings, checked for every matrix type.Pings, and never on a narrowed run.pings.json: round-trip, an older holder reads as unknown, and malformed, foreign-readable or symlinked files are refused.--project, the refusals, and flags given false. They cover the second-connector warning, and its absence for--no-pings, for a shadow and for a stopped connector. They also cover the shadow mirroring the profile, a legacy holder not being counted against its own profile, and status, doctor and guided setup reporting a narrowed run.bin/cipasses on Linux.The one thing these tests don't do is run a Ping through a live feed into
runConnect: the fake Basecamp here serves no account feed. The start line reads the decision from the admission policy itself, so the run tests catch a dropped wiring line, and the admission and handoff tests cover the behaviour.