Skip to content

Commit 8dd1cfd

Browse files
authored
Answer an agent's Pings from one connector only (#936)
Each connector serves the projects it's started with; only one started with --pings hears Pings and other events outside a project. --pings is implied for an agent's first connection and --no-pings is the default for added (bcac_) connections; a profile that isn't an agent connection keeps hearing Pings unless --no-pings. The choice persists per profile, shows in connect status and doctor, and the CLI warns when it sees more than one Ping-hearing connector for the same identity on this computer.
1 parent cb7db7e commit 8dd1cfd

25 files changed

Lines changed: 2141 additions & 41 deletions

‎README.md‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,27 @@ Basecamp's HTTP handshake. Do not reconnect just to upgrade the CLI.
280280
Each profile has its own projects, ledger, cursor, hold and local lock. When
281281
two connections serve the same project, a request both admit can be answered
282282
twice, so serve each project from one. Use a distinct `--device-name` for each
283-
profile on the same computer. Where Basecamp lists several connections, you
283+
profile on the same computer.
284+
285+
Only one computer should answer an agent's Pings. A Ping is in no project, so
286+
every connector for the agent sees all of them, and each one that heard them
287+
would answer. A connector started with `--pings` hears Pings, and anything
288+
else outside a project; one started with `--no-pings` hears only its projects.
289+
Without either, the agent's first connection hears them, a connection added
290+
after it doesn't, and a bot user's login does, as it always has. A run given
291+
`--project` hears none and saves nothing. The first start without `--project`
292+
saves the choice for the profile, as either flag saves its own, and restarts
293+
keep it until you pass the other flag:
294+
295+
```bash
296+
basecamp connect -P desktop --pings # this computer answers Pings
297+
basecamp connect -P laptop --no-pings # this one leaves them to the desktop
298+
```
299+
300+
A connector that hears Pings warns when another one on the same computer hears
301+
the same agent's. It can't see other computers, so starting just one there is
302+
yours to arrange. `basecamp connect status` and `basecamp connect doctor` say
303+
whether a profile's connector answers them, and why. Where Basecamp lists several connections, you
284304
can rename them and disconnect one without using Disconnect all. Reconnecting
285305
a profile on an add-capable server leaves its old connection live until you
286306
disconnect it; older credential files can remain live too. Do not delete them

‎docs/agent-connections-rollout.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,46 @@ the approval page; do not reconnect merely to upgrade the CLI.
3939
A second run of the same profile is refused. Deliberate project overlap is
4040
allowed; requests admitted by both can be answered twice. There is no lease
4141
or cross-profile claim.
42+
6. Decide which one computer answers the agent's Pings (see below), and start
43+
every other connector for the agent with `--no-pings` once.
44+
45+
## Only one computer answers Pings
46+
47+
A Ping is in no project, so serving projects can't divide Pings between
48+
connections: every connector for the agent sees all of them, and each one
49+
that heard them would answer. Only one computer should answer Pings, and the
50+
person running the agent arranges it; the CLI picks a default, keeps the
51+
choice, and warns about a second it can see on the same computer.
52+
53+
- `--pings` hears Pings and anything else outside a project. `--no-pings`
54+
hears only the connector's projects.
55+
- With neither, the agent's first connection (its credential is the agent
56+
client's own secret) hears them, and a connection added after it (its
57+
secret starts `bcac_`) doesn't. A profile that holds a person's login, a
58+
bot user's, hears them as every connector did before this build. A run
59+
given `--project` hears none, as before, and `--pings` with `--project` is
60+
refused.
61+
- The first start on this build without `--project` saves the answer in the
62+
profile's state directory (`pings.json`, beside the ledger), as `--pings`
63+
and `--no-pings` save theirs, and every restart keeps it until one of them
64+
changes it. A run given `--project` saves nothing, so a narrowed first run
65+
leaves the choice to the first one that isn't. A profile that heard Pings keeps
66+
hearing them after a reconnect turns its credential into an added
67+
connection. An unreadable `pings.json` refuses the start: remove it, then
68+
choose again with a flag.
69+
- `basecamp connect status` and `basecamp connect doctor` say whether the
70+
profile's connector answers Pings and why. A connector that hears them
71+
warns when another for the same agent on this computer does too, an older
72+
build's included (it hears them unless started with `--project`).
73+
Connectors on other computers can't be seen.
74+
- Upgrading changes nothing for one connection per agent: it is the first
75+
connection, or a bot user's login, and it still hears Pings. Where an agent
76+
already runs several connectors that hear Pings (a bot user on three
77+
computers, say), choose one, and restart the others with `--no-pings`.
78+
79+
Downgrading drops the choice: older builds ignore `pings.json` and every
80+
connector they run hears Pings unless given `--project`. The file stays, and
81+
the next start on this build reads it again.
4282

4383
State is under `$XDG_STATE_HOME/basecamp/connect` and `connect-shadow` (default
4484
`~/.local/state/basecamp`). New profile directories canonicalize numeric account

‎internal/commands/connect.go‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ the agent.
5050
Connect the agent to a profile first (basecamp auth agent connect -P <profile>),
5151
then run setup on that profile: it records who may drive the agent, maps
5252
which Basecamp projects it serves, and checks the connector is
53-
ready. Pings need no setup: an operator's ping is handed off too. Show prints what setup recorded. Then run the connector on it:
53+
ready. Show prints what setup recorded. Then run the connector on it:
5454
55-
basecamp connect -P <profile> [--project <id>]... [--shadow]
55+
basecamp connect -P <profile> [--project <id>]... [--pings | --no-pings] [--shadow]
5656
5757
It runs in the foreground until interrupted. Stdout is one JSON object per
5858
line: events seen, admission's verdicts, and a "type":"request" line for each
@@ -69,6 +69,19 @@ a new approval replaces the existing connection. Each profile
6969
serves its own projects and has its own hold. When two connections serve the
7070
same project, a request both admit can be answered twice.
7171
72+
Pings. Only one computer should answer an agent's Pings: every connector for
73+
the agent sees all of them, since a Ping is in no project. A connector started
74+
with --pings hears Pings, and anything else outside a project; one started
75+
with --no-pings hears only projects. Without either, the agent's first
76+
connection hears them, a connection added after it doesn't, and a person's
77+
login (a bot user's) does, as before. A run given --project hears none, and
78+
saves nothing. The first start without --project saves the choice for the
79+
profile, as --pings and --no-pings save theirs, and every restart keeps it
80+
until one of them changes it. A connector that hears Pings warns when
81+
another one on this computer hears the same agent's; ones on other computers
82+
can't be seen, so starting just one is yours to arrange. basecamp connect
83+
status and doctor say which this profile's does.
84+
7285
basecamp connect status what it heard and holds
7386
basecamp connect doctor what it needs to run
7487
basecamp connect discard <id> close a record without running it
@@ -78,6 +91,7 @@ same project, a request both admit can be answered twice.
7891
basecamp connect import <file> apply a cutover reconciliation file`,
7992
Example: ` basecamp connect setup -P agent --operator-profile me --serve 12345
8093
basecamp connect -P agent
94+
basecamp connect -P laptop --no-pings
8195
basecamp connect -P agent --project 12345 --shadow`,
8296
Args: cobra.NoArgs,
8397
Annotations: map[string]string{
@@ -401,7 +415,9 @@ The list is this profile's own. When two connections of one agent serve the
401415
same project, a request both admit there can be answered twice, so serve
402416
each project from one of them.
403417
404-
Pings. A Ping is in no project, so it is never served and needs no setup.
418+
Pings. A Ping is in no project, so it is never served and needs no setup,
419+
and only the connector started to hear Pings answers them (see basecamp
420+
connect --help: only one computer should answer Pings).
405421
An operator (the operator, or a person passed with --allow) gives the agent
406422
work by Ping in every trust mode: every line of theirs in a direct Ping with
407423
the agent is a request, and in a group Ping a line that @mentions it.

‎internal/commands/connect_doctor.go‎

Lines changed: 85 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"os"
88
"path/filepath"
99
"runtime"
10+
"slices"
1011
"strconv"
1112
"time"
1213

@@ -25,9 +26,10 @@ func newConnectDoctorCmd() *cobra.Command {
2526
Use: "doctor",
2627
Short: "Check what the connector needs to run",
2728
Long: `Check the connector for a set-up profile: connect.json, the token, the agent's
28-
identity, the stream ticket mint, the account feed, and the ledger (its gaps,
29-
open losses, hold, and lifecycle messages an older connector left waiting for
30-
a person).
29+
identity, the stream ticket mint, the account feed, whether it hears the
30+
agent's Pings (and whether another connector on this computer does too), and
31+
the ledger (its gaps, open losses, hold, and lifecycle messages an older
32+
connector left waiting for a person).
3133
3234
It writes nothing to the connector's ledger and posts nothing to Basecamp.
3335
Renewing the profile's own credential, which every command does when its token
@@ -65,6 +67,7 @@ func runConnectDoctor(cmd *cobra.Command, _ []string) error {
6567
feedCheck(ctx, agent.client.ForAccount(agent.account)),
6668
)
6769
}
70+
checks = append(checks, pingsCheck(ctx, p))
6871
checks = append(checks, ledgerChecks(ctx, p)...)
6972

7073
result := summarizeChecks(asDoctorChecks(checks))
@@ -171,3 +174,82 @@ func connectUnsupportedOSCheck(goos string) setup.Check {
171174
Message: fmt.Sprintf("The connector does not run on %s: %s", goos, connectSupportedOSReason),
172175
Hint: "Run the connector on Linux or macOS; the rest of the CLI runs here."}
173176
}
177+
178+
// pingsCheck says whether the profile's connector hears the agent's Pings —
179+
// the one running now, as its lock says, else a start with neither --pings,
180+
// --no-pings nor --project — and warns when two connectors on this computer
181+
// answer them. Only one should; the ones on other computers can't be seen
182+
// from here. The saved choice is read either way: a running connector
183+
// doesn't make an unreadable one the next start's to discover.
184+
func pingsCheck(ctx context.Context, p connectProfile) setup.Check {
185+
c := setup.Check{Name: "Pings"}
186+
v := readConnectPingsView(ctx, p.app, p.name, p.file)
187+
if v.SavedErr != nil {
188+
c.Status, c.Message = setup.StatusFail, "Whether it hears the agent's Pings was saved, and can't be read, so the next start without a flag refuses: "+errorMessage(v.SavedErr)
189+
c.Hint = v.Remedy(p.name)
190+
return c
191+
}
192+
hears, known := v.Hears()
193+
if !known {
194+
c.Status, c.Message = setup.StatusSkip, "Whether it hears the agent's Pings can't be told without the profile's credential"
195+
return c
196+
}
197+
// A running connector is described by its lock; Next may be unknown
198+
// then, with no saved choice and no credential to read a default from.
199+
who, why := "The connector running now", v.Running.Describe()
200+
if !v.Running.Running {
201+
who, why = "It", v.Next.Describe()
202+
}
203+
switch {
204+
case !hears:
205+
c.Message = who + " doesn't hear the agent's Pings: " + why
206+
case v.Running.Running && v.Running.Pings == nil:
207+
c.Message = who + " may hear the agent's Pings: " + why
208+
default:
209+
c.Message = who + " hears the agent's Pings: " + why
210+
}
211+
// What a restart without flags or --project would do, when it differs.
212+
if v.Running.Running && v.Next != nil && v.Next.Hears != hears {
213+
if v.Next.Hears {
214+
c.Message += "; restarted without --project, it would hear them (" + v.Next.Describe() + ")"
215+
} else {
216+
c.Message += "; restarted, it would hear none (" + v.Next.Describe() + ")"
217+
}
218+
}
219+
switch {
220+
case v.Twice() && hears && len(v.Others) == 1:
221+
c.Status = setup.StatusWarn
222+
c.Message += ", and so does " + peerList(v.Others) + " on this computer: each Ping is answered twice"
223+
case v.Twice() && hears:
224+
c.Status = setup.StatusWarn
225+
c.Message += ", and so do " + peerList(v.Others) + " on this computer: each Ping is answered more than once"
226+
case v.Twice():
227+
c.Status = setup.StatusWarn
228+
c.Message += ", and " + peerList(v.Others) + " on this computer all do: each Ping is answered more than once"
229+
case len(v.Others) == 1:
230+
c.Status = setup.StatusPass
231+
c.Message += "; " + peerList(v.Others) + " answers them on this computer"
232+
return c
233+
case !hears:
234+
c.Status = setup.StatusPass
235+
c.Message += ". Only one connector should answer them; start this one with --pings if it's that one"
236+
return c
237+
default:
238+
c.Status = setup.StatusPass
239+
return c
240+
}
241+
// A profile that hears none can only point at the others: restarting it
242+
// with --no-pings changes nothing.
243+
older := slices.ContainsFunc(v.Others, func(o connectPingsPeer) bool { return o.Older })
244+
switch {
245+
case !hears && older:
246+
c.Hint = "Only one connector should answer Pings, and this one doesn't: upgrade any older one, then restart all but one of the others with --no-pings: basecamp connect -P <profile> --no-pings"
247+
case !hears:
248+
c.Hint = "Only one connector should answer Pings, and this one doesn't: restart all but one of the others with --no-pings: basecamp connect -P <profile> --no-pings"
249+
case older:
250+
c.Hint = "Only one connector should answer Pings, and an older build can't be told: upgrade it, or start this one with --no-pings: basecamp connect -P " + richtext.ShellQuote(p.name) + " --no-pings"
251+
default:
252+
c.Hint = "Only one connector should answer Pings. Start all but one of them with --no-pings: basecamp connect -P " + richtext.ShellQuote(p.name) + " --no-pings"
253+
}
254+
return c
255+
}

‎internal/commands/connect_operator.go‎

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,9 @@ func newConnectStatusCmd() *cobra.Command {
175175
feed position (whether one is held, never the position), the last poll-served id,
176176
gaps and losses, queue depths, live tasks, lifecycle
177177
messages waiting for a person, held records, and the last 20 dispatches with
178-
their outcomes.
178+
their outcomes. It also says whether the profile's connector answers the
179+
agent's Pings and why, and which other connectors on this computer answer
180+
them too: only one should.
179181
180182
It reads the ledger read-only and takes no lock, so it works while the
181183
connector runs. It shows no content, no feed position and no token; a held
@@ -196,9 +198,39 @@ type connectStatusReport struct {
196198
Profile string `json:"profile"`
197199
Shadow bool `json:"shadow"`
198200
LockHolder *connectLockHolder `json:"lock_holder,omitempty"`
201+
Pings connectStatusPings `json:"pings"`
199202
Status connector.Status `json:"status"`
200203
}
201204

205+
// connectStatusPings is whether the profile's connector hears the agent's
206+
// Pings: the choice its starts keep, the connector running now, and the
207+
// other connectors on this computer that hear them. It is the profile's,
208+
// read the same with --shadow: a shadow uses the profile's choice and
209+
// answers nothing.
210+
type connectStatusPings struct {
211+
// Saved is the choice the profile's starts keep; nil before one has.
212+
Saved *connector.PingsChoice `json:"saved,omitempty"`
213+
// Error is why the saved choice can't be read, when it can't, and Hint
214+
// how to recover.
215+
Error string `json:"error,omitempty"`
216+
Hint string `json:"hint,omitempty"`
217+
// Running is the profile's connector running now, as its lock says; nil
218+
// when none runs.
219+
Running *connectStatusRunning `json:"running,omitempty"`
220+
// Others are the connectors running on this computer for the same agent,
221+
// under other profiles, that hear its Pings, or may.
222+
Others []connectPingsPeer `json:"others,omitempty"`
223+
// Twice says two connectors on this computer answer them.
224+
Twice bool `json:"twice,omitempty"`
225+
}
226+
227+
// connectStatusRunning is the profile's connector running now. Hears is nil
228+
// when an older build runs it, which doesn't say.
229+
type connectStatusRunning struct {
230+
Hears *bool `json:"hears,omitempty"`
231+
Because string `json:"because,omitempty"`
232+
}
233+
202234
// connectLockHolder is what a connector wrote beside its instance lock. It is
203235
// diagnostic, not an answer: the metadata is written best effort after the
204236
// lock is taken, it stays behind after a crash, and a pid may since belong to
@@ -210,6 +242,10 @@ type connectLockHolder struct {
210242
// platform can give one. Present says a process has that pid, not that it
211243
// is that connector.
212244
PIDStatus string `json:"pid_status"`
245+
// Pings is whether that connector hears the agent's Pings, as it wrote,
246+
// and PingsBecause why; absent when an older build wrote the file.
247+
Pings *bool `json:"pings,omitempty"`
248+
PingsBecause string `json:"pings_because,omitempty"`
213249
}
214250

215251
func runConnectStatus(cmd *cobra.Command, shadow bool) error {
@@ -242,7 +278,20 @@ func runConnectStatus(cmd *cobra.Command, shadow bool) error {
242278
}
243279
report := connectStatusReport{Profile: p.name, Shadow: shadow, Status: status}
244280
if holder, ok := connector.InstanceHolder(dir, p.file.AccountID, p.file.Agent.PersonID); ok {
245-
report.LockHolder = &connectLockHolder{PID: holder.PID, StartedAt: holder.StartedAt, PIDStatus: processPresence(holder.PID)}
281+
report.LockHolder = &connectLockHolder{PID: holder.PID, StartedAt: holder.StartedAt, PIDStatus: processPresence(holder.PID), Pings: holder.Pings, PingsBecause: holder.PingsBecause}
282+
}
283+
// Status reads no credential, so a profile that has saved no choice has
284+
// none to show.
285+
pings := readConnectPingsView(cmd.Context(), nil, p.name, p.file)
286+
report.Pings = connectStatusPings{Saved: pings.Saved, Others: pings.Others, Twice: pings.Twice()}
287+
if pings.SavedErr != nil {
288+
report.Pings.Error, report.Pings.Hint = errorMessage(pings.SavedErr), pings.Remedy(p.name)
289+
}
290+
if r := pings.Running; r.Running {
291+
report.Pings.Running = &connectStatusRunning{}
292+
if r.Pings != nil {
293+
report.Pings.Running.Hears, report.Pings.Running.Because = &r.Pings.Hears, r.Pings.Because
294+
}
246295
}
247296
if p.app.Output.EffectiveFormat() == output.FormatStyled {
248297
renderConnectStatus(cmd.OutOrStdout(), report)
@@ -299,6 +348,7 @@ func renderConnectStatus(w io.Writer, r connectStatusReport) {
299348
}
300349
fmt.Fprintln(w)
301350
}
351+
renderConnectStatusPings(w, r)
302352
if s.Hold != nil {
303353
fmt.Fprintf(w, " Hold set by %s at %s (%s, generation %d): nothing dispatches or posts until release\n",
304354
clean(s.Hold.HeldBy), stamp(s.Hold.HeldAt), clean(s.Hold.Cause), s.Hold.Generation)
@@ -805,3 +855,46 @@ func readReconciliation(path string) ([]byte, error) {
805855
}
806856
return data, nil
807857
}
858+
859+
// renderConnectStatusPings is status's Pings lines: whether the profile's
860+
// connector hears them and why, what the running one does when that differs,
861+
// and who else on this computer hears them.
862+
func renderConnectStatusPings(w io.Writer, r connectStatusReport) {
863+
p := r.Pings
864+
switch {
865+
case p.Error != "":
866+
fmt.Fprintf(w, " Pings the saved choice can't be read (%s), so the next start without a flag refuses. %s\n", richtext.SanitizeSingleLine(p.Error), p.Hint)
867+
case p.Saved == nil:
868+
fmt.Fprintf(w, " Pings not chosen yet: the first start without --project chooses (an agent's first connection hears them, a connection added after it doesn't)\n")
869+
default:
870+
fmt.Fprintf(w, " Pings %s\n", connectPingsWords(connectPings{Hears: p.Saved.Hears, Because: p.Saved.Because}))
871+
}
872+
if run := p.Running; run != nil {
873+
switch {
874+
case run.Hears == nil:
875+
fmt.Fprintf(w, " the connector running now may hear them: %s\n", connectRunning{Running: true}.Describe())
876+
case p.Saved == nil || *run.Hears != p.Saved.Hears:
877+
now := connectPings{Hears: *run.Hears, Because: run.Because}
878+
heard := "hears none"
879+
if now.Hears {
880+
heard = "hears them"
881+
}
882+
fmt.Fprintf(w, " the connector running now %s: %s\n", heard, now.Describe())
883+
}
884+
}
885+
for _, other := range p.Others {
886+
if p.Twice {
887+
fmt.Fprintf(w, " Pings heard by %s (pid %d) on this computer too: only one connector should answer them\n", other.Words(), other.PID)
888+
} else {
889+
fmt.Fprintf(w, " Pings answered on this computer by %s (pid %d)\n", other.Words(), other.PID)
890+
}
891+
}
892+
}
893+
894+
// connectPingsWords is whether a connector hears Pings, and why, for a line.
895+
func connectPingsWords(p connectPings) string {
896+
if p.Hears {
897+
return "heard: " + p.Describe()
898+
}
899+
return "not heard: " + p.Describe() + "; start with --pings to answer them from here"
900+
}

0 commit comments

Comments
 (0)