Speed up worker teardown: seven fixes from the slowness investigation by timbarreto · Pull Request #78 · timbarreto/firstmate

timbarreto · 2026-10-01T07:53:06Z

Summary

Implements the seven suggested fixes from the slow worker cleanup investigation, one commit per fix, smallest first in the report's numbering.
Teardown on this Windows/Git Bash host ran a long synchronous chain of safety and bookkeeping steps, each spawning many processes, plus fleet-wide work repeated per teardown.
These changes remove repeated work and forks from that chain and move fleet-wide follow-up work off the return path.
They do not remove or reorder any safety step.

Fix	Commit	Change
1	`fm-teardown: request the home summary instead of awaiting it`	Teardown records a coalesced request through `fm-home-summary-refresh.sh --request --service --best-effort`. One detached pending-only publisher serves the request and any burst behind it, no-ops for an empty queue or a retired home, keeps a failed request retryable, and logs the error to `state/.home-summary-refresh.log`. The watcher and session start still service requests, so publication stays eventual.
2	`fm-teardown: hand the tasks-axi verdict to the decision read`	The `fm-captain-hold.sh open` child gets teardown's already-settled verdict through the existing consumed one-hop `FM_TASKS_AXI_COMPATIBLE` handoff instead of re-probing. Nothing is persisted, and tool-availability failures are unchanged.
3	`fm-teardown: read task metadata in-process`	Adds `fm_meta_read` (one pass into named variables) and destination forms of `fm_meta_get` and `fm_backend_of_meta`, used on teardown's hot path and in the pool-slot scan. Every refusal, ownership, and landed-work check reads the same fields with the same values.
4	`fm-timeout-lib: stop the bash watchdog from holding captured output`	On the pure-Bash bound, a captured quick command could wait out its whole bound because the sleeper watchdog kept the capture pipe open. On Bash 4+ with a plain decimal bound the watchdog is now a coprocess with detached stdio waiting on a release channel. Other bounds keep the sleeper with detached stdio. Timeout status, signal forwarding, and descendant cleanup are unchanged.
5	`fm-guard: reuse the verdict's detected harness for the repair line`	The stale-supervision banner detected the harness twice. The verdict now detects once, and the guard passes `--harness` to the repair line only when the verdict validated one. The identity is never exported or cached across processes, so the supervision model is unchanged.
6	`fm-fleet-snapshot: drop per-task forks from the producer`	Per-task command substitutions and pipelines in the snapshot producer become in-shell reads and destination-variable helpers. Already-batched reads and `fm_meta_read` are kept, and a partial summary is still never emitted as success.
7	`fm-teardown: request the clone refresh instead of awaiting it`	Adds `fm-fleet-sync.sh --request`. It records a request file, starts one detached serialized server, and returns. The server coalesces a batch to one refresh per project, bounds each refresh, logs to `state/.fleet-sync.log`, and raises one check wake for any `skipped:`, `STUCK:`, or `recovered:` line. A non-directory path, missing state directory, or unrecordable request falls back to today's foreground sync, and session start's full refresh stays the freshness backstop.

Safety invariants kept

Landed-work and ownership checks, the pool-slot exclusivity scan, and exact-endpoint confirmation run unchanged and in the same order.
Durable backlog transitions and their ordering are unchanged; the lab postcondition checks the row reaches Done.
Locks and every refusal path are unchanged; no --force or bypass was added.
Merge authority is untouched, nothing deletes unlanded work, and no shared process is killed.
The background home-summary publisher and clone-refresh server only serve their own home's request files and never recreate a retired home.

Measurements

The isolated lab fixture follows the report's method:

a named Herdr lab session through fm-herdr-lab.sh (provision, run, teardown, with LAB_CLEANUP_VERIFIED and the default-session tripwire passing every series);
a throwaway FM_HOME under the worktree;
a lab-only task added and started through fm-tasks-axi.sh;
a kind=ship, mode=direct-PR, harness=copilot, backend=herdr record bound to the real lab pane;
nonexistent project/worktree paths (noproj), or a real lab-local clone with a bare origin (proj);
treehouse stubbed to refuse and no-mistakes removed from PATH.

The timer encloses only bash bin/fm-teardown.sh <id>.
"Pane gone" is when a 0.3s pane get poller first sees the lab pane disappear.
"Background settled" is when the after-change background publisher and clone-refresh server finished; base has no background work, so its value equals its total.
Base is 011c291f and after is this branch's HEAD, both as clean clones, interleaved per round so host-load drift hits both.
Every run returned 0, removed the task record, and left the backlog row Done.

The report's reduced run took 146.4s at base on a less loaded host; this host was slower throughout, so only paired base/after comparisons are meaningful.
The host was heavily loaded by an unrelated test suite during the first series, which ended part-way through, so compare base and after within a round.

noproj, series 1 (seconds):

round	base total	after total	base pane gone	after pane gone	after background settled
1	374.5	237.7	260.7	198.0	263.8
2	213.2	138.0	150.7	117.9	154.6
3	156.9	153.0	103.2	110.1	165.7

noproj, series 2, after the unrelated load ended (seconds):

round	base total	after total	base pane gone	after pane gone	after background settled
1	168.2	128.4	122.3	91.0	152.2
2	150.8	130.0	100.9	94.1	140.3
3	208.9	124.8	157.6	103.5	143.2

Series 2 medians: total return 168.2s base versus 128.4s after (about 24% less), and pane gone 122.3s versus 94.1s (about 23% less).
Across all six noproj pairs the median after/base total ratio is about 0.70.
The return path after the pane closed (total minus pane gone) went from 46-114s at base to 20-43s after, mainly because the home summary is requested rather than rebuilt synchronously.
In the noproj variant the project path is not a directory, so fix 7 deliberately falls back to the foreground single-project sync, exactly like base; the proj variant shows fix 7.

proj (real clone; fix 7 applies) (seconds):

round	base total	after total	base pane gone	after pane gone	after background settled
1	184.6	148.0	129.1	107.3	165.0
2	169.3	127.1	122.2	86.5	157.0

The Herdr pane-close span itself (presentation lock, session and workspace reads, close, confirmation) is unchanged code and ranged from about 25s to 85s with load.

Per-fix measurements on the same host:

Fix 3: 100 metadata reads took 37.3s through command substitution versus 0.7s in-process.
Fix 5: harness detection costs about 7-13s per call; the repair line took about 17.7s with detection versus about 3.0s reusing the verdict's harness.
Fix 6: on an 8-record fixture, fm-fleet-snapshot.sh --json went from 64.6s to 32.3s and --secondmate-home-summary from 56.8s to 28.7s, with identical normalized output from base and after for --json, --secondmate-home-summary, and --contribution-input.
Fix 4: the new test returns a captured quick command long before its bound, where the old watchdog could hold it for the whole bound.

Tests

New or updated colocated tests:

tests/fm-timeout-lib.test.sh: a captured quick command returns long before the bound (stdout, stderr, and plain invocation), hung commands are still bounded, fractional bounds with functions, and an orphaned watchdog still bounds the command and leaves no scratch files.
tests/fm-teardown.test.sh: teardown makes exactly one tasks-axi compatibility probe (fails on base with two); the fork-remote case waits for the background summary publication.
tests/fm-home-summary-request.test.sh: coalescing, the one-shot background service, failure retention and logging, the retired-home no-op, and the --service flag contract.
tests/fm-guard-stale-banner.test.sh: the full banner detects its own harness once (fails on base with two).
tests/fm-backend.test.sh and tests/fm-teardown-endpoint-safety.test.sh: fm_meta_read, destination forms, fm_backend_target_of_meta, and a vanished neighbour in the sole-slot case.
tests/fm-fleet-sync.test.sh: request coalescing while a refresh runs, actionable results relayed as one check wake, foreground fallback without a state directory, an immediate skip for a missing directory, and usage errors.
tests/fm-fleet-snapshot-view.test.sh, tests/fm-classify-decision-key.test.sh, tests/fm-afk-contract.test.sh: status-log scan, decision-key destination forms, and AFK contract path and presence helpers.

bin/fm-lint.sh (shellcheck) is clean on every changed script and test.

Results on this host (Windows, Git Bash 5.3, symlink cases with MSYS=winsymlinks:nativestrict):

Final regression at the branch head, run in parallel from a git archive HEAD snapshot: the six fm-teardown-endpoint-safety slot and endpoint refusal cases plus fm-teardown backlog close, tasks-axi verdict hand-off, and local-only fork remote all passed (9/9).
The new fix 2 and fix 5 tests fail on base (two probes, two detections) and pass on this branch.
fm-home-summary-request 10/10, fm-backend 31/31, fm-fleet-snapshot-view 20/20 (with a raised crew-state timeout, see below), and fm-afk-contract, fm-classify-decision-key, fm-classify-corr-token, and fm-wake-drain-open-decisions-cursor all pass.
fm-fleet-sync 33/34 and fm-timeout-lib new captured-output cases pass; the remaining failures are the pre-existing host-timing cases below.

Pre-existing host failures (identical on base)

These fail on this Windows/Git Bash host both at 011c291f and on this branch:

Symlink fixtures (fm-backend slot cases, fm-wake-drain-open-decisions test_status_symlink_is_not_followed, the endpoint-safety slot cases) need MSYS=winsymlinks:nativestrict; without it ln -s copies.
fm-fleet-sync test_bootstrap_relays_recovered_and_stuck times out at its 20s bootstrap bound.
fm-session-start runtime-bound truncation case, fm-home-summary-refresh initial publication deadline, and fm-procevent-quota provider exhaustion, all host-timing.
fm-fleet-snapshot-view test_fixture_snapshot_json needs FM_SNAPSHOT_CREW_STATE_TIMEOUT above its 10s default under load.

Follow-ups noticed (not changed here)

The pool-slot scan does not refuse when another record names the slot only through an aliased home= path (for example .../worktree/../worktree) with a missing worktree; base behaves the same, so this branch keeps it.
The snapshot still runs one jq -n per row; batching those is the next producer win.
Most remaining crew-state cost is per-process library sourcing.
fm-fleet-sync.sh --request skips the guard the cleanup already ran; bootstrap's full sync remains the freshness backstop.

CI notes

Commit 6dbab795 updates the macOS snapshot count in .github/workflows/ci.yml from 19 to 20 for the new snapshot test case.
Commit 2350f3ac keeps the teardown lint root under the ShellCheck memory cap.
bin/fm-teardown.sh re-sourced bin/fm-wake-lib.sh with a directed source inside its herdr prerequisite fallback even though the same file already sources it at top level, so ShellCheck expanded that library (and its timeout and classify dependencies) twice.
The fallback now uses the existing # shellcheck source=/dev/null analysis-boundary idiom from fm-lease-lib.sh and fm-task-inbox-lib.sh; the library is still analyzed once in this root and as its own lint root.
Local peak ShellCheck RSS for the teardown root: base 011c291f 8,167 MiB, branch before this commit 8,336 MiB, branch after 7,977 MiB.
Upstream main's Lint 1 currently fails the same way on this root.
The Behavior portable serial 3 and 5 failures (tests/fm-pi-branch-extension.test.sh, tests/fm-calm-pi-extension.test.sh) come from CI installing Pi unpinned: the base's last green run used Pi 0.87.1 and this branch's run got Pi 0.99.2.
Upstream fixed them in fix: restore portable CI behavior across Pi rendering and remote provisioning kunchenguid/firstmate#6162, which this fork does not have yet; no file in this PR touches Pi rendering.

On hosts that use the pure-Bash bound, a quick command whose output was captured could wait out its whole bound: the sleeper watchdog sometimes missed its stop signal and kept the capture pipe open. On Bash 4+ with a plain decimal bound the watchdog is now a coprocess with detached stdio that waits inside read on a release channel, so no sleeper is left to signal, and an orphaned watchdog still bounds the command to its deadline and clears its scratch files. Other bounds keep the sleeper watchdog, now with detached stdio. Monitor mode is dropped right after the forks so a finished command's job notice cannot leak into a capturing caller.

Teardown's backlog gate already settles tasks-axi compatibility in-process, but the fm-captain-hold.sh open child it runs to check for an open captain call re-ran the three compatibility probes. On a Windows/Git Bash host those probes cost several seconds per teardown. Pass the verdict through the existing consumed one-hop FM_TASKS_AXI_COMPATIBLE handoff owned by bin/fm-tasks-axi-lib.sh. Nothing is persisted and tool-availability failures are unchanged. The new teardown test counts every tasks-axi call and requires exactly one compatibility probe; it fails against the previous teardown with two.

Every teardown ended by synchronously refreshing the home summary, which rebuilds the whole fleet snapshot and repeats fleet-wide work after each cleanup. That wait dominated the completion tail on Windows/Git Bash. Teardown now records a coalesced request through fm-home-summary-refresh.sh --request --service --best-effort. --service starts one detached pending-only publisher (FM_HOME_SUMMARY_IF_PENDING=1) that serves the request and any burst behind it. It does nothing when no request is pending or the home has been retired, so it never recreates a removed home. The watcher and session start still service requests as before, so publication stays eventual even when the background publisher cannot start. A failed background publication keeps the request retryable and records its error in state/.home-summary-refresh.log, so the failure stays visible. Tests cover the coalescing, the one-shot background service, failure retention and logging, the retired-home no-op, and the --service flag contract. The fork-remote teardown test now waits for the background publication before checking the published summary.

A guard call that prints the full stale-supervision banner detected its own harness twice: once inside fm_watcher_supervision_verdict to choose the supervision model, and again when fm-supervision-instructions.sh rendered the repair line without --harness. On this Windows/Git Bash host one detection costs about 7-13 seconds, so the repair line alone took about 17.7 seconds with detection versus about 3.0 seconds with --harness, and teardown pays the guard more than once. fm_supervision_own_harness now records the detected identity and its status in non-exported variables of the calling shell. The verdict detects once, exposes the validated harness as FM_WATCHER_VERDICT_HARNESS, and fm_supervision_model --detected reuses that result. fm-guard passes --harness only when the verdict validated one, so an override model or a failed detection keeps today's behavior, where the instructions detect for themselves. The identity lives for one verdict call and is never exported or cached across processes, so the supervision model is unchanged. The new guard test counts harness detections through a stub and fails on the previous code with two detections.

Teardown and its pool-slot scan read each metadata field through a command substitution, which costs a fork per read; on a Windows/Git Bash host 100 such reads took about 37 seconds against under one second in-process. Add fm_meta_read, which loads a record's fields into named variables in one pass, and destination forms of fm_meta_get and fm_backend_of_meta, then use them on teardown's hot path and in the slot scan, which now reads a record's worktree and home together. Canonical-directory and live-slot lookups write into destination variables for the same reason. Every refusal, ownership, and landed-work check reads the same fields with the same values as before; only the way each value reaches its variable changes.

Every non-scout cleanup ran a full fetch and fast-forward of its project's clone after the worker was already gone, so a batch of cleanups paid one clone refresh each, serially, on their return path. Add fm-fleet-sync.sh --request, which records a request file, starts one detached server, and returns at once. The server serializes on a service lock, keeps at most one waiter, and serves each snapshot of request files once per distinct project, so a cleanup batch for one project costs one refresh. Each project refresh is bounded, output is appended to a bounded state/.fleet-sync.log, and any skipped:, STUCK:, or recovered: line that session start would relay raises one check wake naming that log, so the diagnostics a synchronous call printed still reach firstmate. A path that is not a directory, a missing state directory, or a request that cannot be recorded falls back to the foreground single-project sync, so clone freshness never depends on the server, and session start's full refresh stays the backstop. The request form skips the guard the cleanup already ran. Direct invocations and every other caller are unchanged.

The fleet snapshot producer feeds the home summary that teardown republishes, and a producer trace showed per-task command substitutions and pipelines dominating its runtime on hosts where a fork costs 0.1-0.3s. - status_log_scan reads the latest non-blank event line and, only when metadata carries no pr=, the first PR display URL in one in-shell pass, replacing the grep|tail and grep -Eo|head pipelines. - Backend, target, verb, note, and open-decision reads assign through destination variables (fm_backend_target_of_meta, status_open_decisions, _fm_status_kind) instead of $(...). - basename and head -1 become parameter expansions. - fm_afk_contract_path gains a destination form, so fm_afk_contract_present (reached per task through merge-authority resolution) no longer forks. Stdout forms and return codes of every changed helper are unchanged, and already-batched reads and fm_meta_read stay as they were. A partial summary is still never emitted as success. Equivalence: an 8-record fixture home (two PRs on one line, an Azure DevOps URL with a query, CRLF and no final newline, blank and whitespace-only lines, meta pr=, orca/herdr/tmux targets, a scout report, a missing status log) produced identical --json, --secondmate-home-summary, and --contribution-input output from base and this change after normalizing home paths and timestamps. Full --json on that fixture took 64.6s at base and 32.3s here, and the home summary 56.8s and 28.7s, on the same loaded host.

The fleet-snapshot producer change added a status-log scan test to tests/fm-fleet-snapshot-view.test.sh, and the stock macOS Bash 3.2 job pins the number of passing tests in that suite.

The herdr prerequisite fallback re-sourced bin/fm-wake-lib.sh under a directed source, so ShellCheck's external-source traversal expanded the whole wake-lib graph a second time inside the bin/fm-teardown.sh root, which already expands it once through its top-level directed source. That duplicate pushed the root past CI's per-root analysis memory bound once the teardown fixes grew the shared libraries. Mark the fallback source=/dev/null with the same analysis-boundary rationale bin/fm-lease-lib.sh and bin/fm-task-inbox-lib.sh already use for their lazy wake-lib fallbacks. Runtime sourcing is unchanged and wake-lib is still analyzed once in this root and as its own lint root. Local peak working set for the root drops from about 8336 MiB to about 7977 MiB, below the pre-branch 8167 MiB.

Timothy Barreto (Accenture International Limited) added 9 commits September 30, 2026 17:18

ci: expect 20 stock-Bash snapshot/fleet-view tests

6dbab79

The fleet-snapshot producer change added a status-log scan test to tests/fm-fleet-snapshot-view.test.sh, and the stock macOS Bash 3.2 job pins the number of passing tests in that suite.

Navigation Menu

Speed up worker teardown: seven fixes from the slowness investigation - #78

Speed up worker teardown: seven fixes from the slowness investigation#78
timbarreto wants to merge 9 commits into
mainfrom
fm/teardown-speedup

timbarreto commented Oct 1, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

timbarreto commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Safety invariants kept

Measurements

Tests

Pre-existing host failures (identical on base)

Follow-ups noticed (not changed here)

CI notes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

timbarreto commented Oct 1, 2026 •

edited

Loading