fleet/fleetd
3
0
Fork 0
Code Issues 80 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

A lead launch gets none of the three protections every member spawn gets: no shell-ready retry, no name-collision retry, no 1024-byte argv check #727

New Issue
Closed
opened 2026-10-04 17:42:01 +02:00 by ltms · 1 comment
ltms commented 2026-10-04 17:42:01 +02:00
Owner
Copy Link
Copy Source

The gap

LeadLauncher.launch starts a lead with a bare call:

Agent started = agents.start("lead-" + name, herdrKind(profile),
        argv.isEmpty() ? argv : argv.subList(1, argv.size()), tab.rootPaneId());
                                                   // LeadLauncher.java:326-327

A member spawn on the same herdr surface is wrapped in three protections. A lead gets none of them.
Measured 2026-10-04 — grep -c in LeadLauncher.java:

  agent_pane_busy            0
  agent_name_taken           0
  PANE_COMMAND_BYTE_LIMIT    0
  checkPaneCommandFits       0

1. No shell-ready retry

A freshly created pane may not have redrawn its prompt, and herdr answers agent_pane_busy.

  • Members: SHELL_READY_RETRIES = 20 (HerdrPeerLauncher.java:78), used by
    startAwaitingShellPrompt (:836-849).
  • Leads: none. One attempt.

This repo's own contract test measured a seed shell taking around 2.5s to settle, with
SHELL_READY_TIMEOUT_MS = 8_000 (AgentControlContractTest.java).

2. No name-collision retry, and the name is fixed

The lead's agent name is a fixed "lead-" + name — no nonce, no sequence.

  • Members: <prefix>-<profile>-<nonce>-<seq> with NAME_RETRIES = 8
    (HerdrPeerLauncher.java:71, :770-783).
  • Leads: one fixed name, no retry.

So if herdr still holds an agent record under lead-opus — after a crash, or after a session exits
and the registry has not released the name — the relaunch is refused outright with
agent_name_taken, and ensureLeads() logs a failure and moves on.

Whether herdr frees the name after a session exits is not established. agent_not_found on a
target lookup is a different question from the name registry being free, and nobody has measured
the second. That makes this gap's severity unknown rather than low.

3. No argv length check, and the failure is silent

herdr types the launch command into the pane, and a pty line buffer holds only
PANE_COMMAND_BYTE_LIMIT = 1024 bytes (HerdrPeerLauncher.java:787, :816, :833). Past that the
tail is dropped with no error at all.

  • Members: checkPaneCommandFits guards it, called from HerdrPeerLauncher.java:769.
  • Leads: the guard is private to HerdrPeerLauncher and LeadLauncher never calls it.

A lead's argv is assembled at LeadLauncher.java:370-392 and already carries --mcp-config with an
inline JSON mount, --model, and the auto-compact pin. It is not checked. A longer profile argv, a
longer model name or another flag can cross 1024 bytes and the lead will start with a silently
truncated command line.

Why this is being filed now

Found while designing #726 (a handover should restart the lead's process rather than type /clear).
All three gaps exist today, independent of that work — a lead is launched this way at every daemon
boot (FleetdAssembly.java:304).

They matter more once #726 lands, because that change makes LeadRollover call a lead launch on a
path where failure means zero leads: ensureLeads() has exactly one production call site, at
boot, with no scheduler, so nothing relaunches until the daemon restarts. #726's accepted design
therefore requires a bounded relaunch retry covering agent_pane_busy and agent_name_taken. Fixing
them here, in the launcher, is the cheaper place — the retry then has something correct to call.

Suggested shape

Extract the member protections rather than writing second copies:

  • Lift checkPaneCommandFits out of HerdrPeerLauncher into a shared seam both launchers call.
  • Reuse the member retry pattern for agent_pane_busy and agent_name_taken instead of inventing a
    second one.
  • Give the lead agent a per-start unique name, so a retry adopts or is cleanly refused rather than
    creating a second process.

Acceptance criteria

  • A lead launch retries agent_pane_busy with a bounded budget, and a test proves the budget is
    honoured.
  • A lead launch survives a taken name, and a test proves a second start does not create a second
    process.
  • A lead argv over 1024 bytes is refused with a clear error, never typed and truncated. The test
    must assert the refusal, not the absence of a crash.
  • One shared implementation of the byte check, with the member path still covered.

Not checked

Whether herdr releases an agent name after its session exits. Whether any current profile's lead argv
is actually near 1024 bytes — I did not measure the assembled length, only that nothing checks it.

## The gap `LeadLauncher.launch` starts a lead with a bare call: ```java Agent started = agents.start("lead-" + name, herdrKind(profile), argv.isEmpty() ? argv : argv.subList(1, argv.size()), tab.rootPaneId()); // LeadLauncher.java:326-327 ``` A member spawn on the same herdr surface is wrapped in three protections. A lead gets none of them. Measured 2026-10-04 — `grep -c` in `LeadLauncher.java`: ``` agent_pane_busy 0 agent_name_taken 0 PANE_COMMAND_BYTE_LIMIT 0 checkPaneCommandFits 0 ``` ### 1. No shell-ready retry A freshly created pane may not have redrawn its prompt, and herdr answers `agent_pane_busy`. - Members: `SHELL_READY_RETRIES = 20` (`HerdrPeerLauncher.java:78`), used by `startAwaitingShellPrompt` (`:836-849`). - Leads: none. One attempt. This repo's own contract test measured a seed shell taking around 2.5s to settle, with `SHELL_READY_TIMEOUT_MS = 8_000` (`AgentControlContractTest.java`). ### 2. No name-collision retry, and the name is fixed The lead's agent name is a fixed `"lead-" + name` — no nonce, no sequence. - Members: `<prefix>-<profile>-<nonce>-<seq>` with `NAME_RETRIES = 8` (`HerdrPeerLauncher.java:71`, `:770-783`). - Leads: one fixed name, no retry. So if herdr still holds an `agent` record under `lead-opus` — after a crash, or after a session exits and the registry has not released the name — the relaunch is refused outright with `agent_name_taken`, and `ensureLeads()` logs a failure and moves on. **Whether herdr frees the name after a session exits is not established.** `agent_not_found` on a *target lookup* is a different question from the *name registry* being free, and nobody has measured the second. That makes this gap's severity unknown rather than low. ### 3. No argv length check, and the failure is silent herdr **types** the launch command into the pane, and a pty line buffer holds only `PANE_COMMAND_BYTE_LIMIT = 1024` bytes (`HerdrPeerLauncher.java:787`, `:816`, `:833`). Past that the tail is dropped with no error at all. - Members: `checkPaneCommandFits` guards it, called from `HerdrPeerLauncher.java:769`. - Leads: the guard is `private` to `HerdrPeerLauncher` and `LeadLauncher` never calls it. A lead's argv is assembled at `LeadLauncher.java:370-392` and already carries `--mcp-config` with an inline JSON mount, `--model`, and the auto-compact pin. It is not checked. A longer profile `argv`, a longer model name or another flag can cross 1024 bytes and the lead will start with a silently truncated command line. ## Why this is being filed now Found while designing #726 (a handover should restart the lead's process rather than type `/clear`). All three gaps exist **today**, independent of that work — a lead is launched this way at every daemon boot (`FleetdAssembly.java:304`). They matter more once #726 lands, because that change makes `LeadRollover` call a lead launch on a path where failure means **zero leads**: `ensureLeads()` has exactly one production call site, at boot, with no scheduler, so nothing relaunches until the daemon restarts. #726's accepted design therefore requires a bounded relaunch retry covering `agent_pane_busy` and `agent_name_taken`. Fixing them here, in the launcher, is the cheaper place — the retry then has something correct to call. ## Suggested shape Extract the member protections rather than writing second copies: - Lift `checkPaneCommandFits` out of `HerdrPeerLauncher` into a shared seam both launchers call. - Reuse the member retry pattern for `agent_pane_busy` and `agent_name_taken` instead of inventing a second one. - Give the lead agent a per-start unique name, so a retry adopts or is cleanly refused rather than creating a second process. ## Acceptance criteria - A lead launch retries `agent_pane_busy` with a bounded budget, and a test proves the budget is honoured. - A lead launch survives a taken name, and a test proves a second start does not create a second process. - A lead argv over 1024 bytes is **refused with a clear error**, never typed and truncated. The test must assert the refusal, not the absence of a crash. - One shared implementation of the byte check, with the member path still covered. ## Not checked Whether herdr releases an agent name after its session exits. Whether any current profile's lead argv is actually near 1024 bytes — I did not measure the assembled length, only that nothing checks it.
ltms commented 2026-10-04 17:54:07 +02:00
Author
Owner
Copy Link
Copy Source

Measured the two things this ticket listed as "Not checked" (2026-10-04)

I filed this ticket saying I had not measured the assembled lead argv length. I have now, and it
changes how gap 3 should be weighted. Gaps 1 and 2 are unaffected.

Gap 3: there is a lot of headroom, so this is latent, not imminent

I replayed leadArgv (LeadLauncher.java:371-385) for the two profiles that hold a lead seat, and
charged each argument the same way checkPaneCommandFits does — its UTF-8 bytes plus
QUOTING_OVERHEAD_PER_ARG = 3 (HerdrPeerLauncher.java:803-811, :838):

opus    args=7  estimate= 152 bytes  limit=1024  headroom=872  (14% used)  longest=74B
sonnet  args=7  estimate= 154 bytes  limit=1024  headroom=870  (15% used)  longest=74B

The longest argument is the inline MCP mount at 74 bytes.

Why the lead sits so far under the limit is structural, not luck. A lead gets no
--append-system-prompt, and LeadLauncher's own javadoc says that is deliberate and must not be
"unified" back (LeadLauncher.java:364-368). That flag carries the worker reply charter, and it is
the argument that pushed the member command to 978 bytes in #214 — the incident that bought the
check in the first place. So the member path is near the limit for a reason the lead path does not
share.

What this does not change: nothing checks the length, so the failure mode is still a silent
truncation with no error (the javadoc at HerdrPeerLauncher.java:785-792 describes it). It stays
worth fixing as part of the shared seam this ticket already proposes. It should not be the reason
anyone rushes the ticket, and it does not need its own design.

Gap 3's acceptance criterion needs a synthetic argv

Because no real profile comes close to 1024, a test cannot reach the refusal with today's config.
The test must build an over-long argv on purpose. That is the right shape anyway — it asserts the
refusal rather than the absence of a crash, which is what the criterion already asks for.

The live lead is not a LeadLauncher lead at all

Worth recording, because it limits what can be measured on this host:

pid 96771  argv_bytes 46  has_mcp_config=0  has_model=0

The running lead's command line is 46 bytes and carries neither --mcp-config nor --model, so it
was not assembled by leadArgv. It is an operator-started session that holds the lead role purely
by its tab label, which is what FleetConfig.java:1112-1115 says identity now rests on. So the
numbers above come from replaying the code, not from observing a launched lead. No lead launched
by LeadLauncher was running here to measure.

Re-measure the estimate with the profile values from fleetd.yaml (argv, model, mcpUrl,
autoCompactWindow) replayed through the checkPaneCommandFits formula. If a future profile adds a
flag and the estimate climbs toward 1024, this comment is stale and gap 3 becomes urgent.

Still not checked

Whether herdr releases an agent name after its session exits. That is gap 2's severity, and it needs
a live probe rather than a reading of this repo. It is unchanged by the above.

## Measured the two things this ticket listed as "Not checked" (2026-10-04) I filed this ticket saying I had not measured the assembled lead argv length. I have now, and it changes how gap 3 should be weighted. Gaps 1 and 2 are unaffected. ### Gap 3: there is a lot of headroom, so this is latent, not imminent I replayed `leadArgv` (`LeadLauncher.java:371-385`) for the two profiles that hold a lead seat, and charged each argument the same way `checkPaneCommandFits` does — its UTF-8 bytes plus `QUOTING_OVERHEAD_PER_ARG = 3` (`HerdrPeerLauncher.java:803-811`, `:838`): ``` opus args=7 estimate= 152 bytes limit=1024 headroom=872 (14% used) longest=74B sonnet args=7 estimate= 154 bytes limit=1024 headroom=870 (15% used) longest=74B ``` The longest argument is the inline MCP mount at 74 bytes. **Why the lead sits so far under the limit is structural, not luck.** A lead gets no `--append-system-prompt`, and `LeadLauncher`'s own javadoc says that is deliberate and must not be "unified" back (`LeadLauncher.java:364-368`). That flag carries the worker reply charter, and it is the argument that pushed the member command to 978 bytes in #214 — the incident that bought the check in the first place. So the member path is near the limit for a reason the lead path does not share. What this does **not** change: nothing checks the length, so the failure mode is still a silent truncation with no error (the javadoc at `HerdrPeerLauncher.java:785-792` describes it). It stays worth fixing as part of the shared seam this ticket already proposes. It should not be the reason anyone rushes the ticket, and it does not need its own design. ### Gap 3's acceptance criterion needs a synthetic argv Because no real profile comes close to 1024, a test cannot reach the refusal with today's config. The test must build an over-long argv on purpose. That is the right shape anyway — it asserts the refusal rather than the absence of a crash, which is what the criterion already asks for. ### The live lead is not a `LeadLauncher` lead at all Worth recording, because it limits what can be measured on this host: ``` pid 96771 argv_bytes 46 has_mcp_config=0 has_model=0 ``` The running lead's command line is 46 bytes and carries neither `--mcp-config` nor `--model`, so it was not assembled by `leadArgv`. It is an operator-started session that holds the lead role purely by its tab label, which is what `FleetConfig.java:1112-1115` says identity now rests on. So the numbers above come from replaying the code, **not** from observing a launched lead. No lead launched by `LeadLauncher` was running here to measure. Re-measure the estimate with the profile values from `fleetd.yaml` (`argv`, `model`, `mcpUrl`, `autoCompactWindow`) replayed through the `checkPaneCommandFits` formula. If a future profile adds a flag and the estimate climbs toward 1024, this comment is stale and gap 3 becomes urgent. ### Still not checked Whether herdr releases an agent name after its session exits. That is gap 2's severity, and it needs a live probe rather than a reading of this repo. It is unchanged by the above.
ltms closed this issue 2026-10-04 18:30:21 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#727
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?