Skip to content

docs(fleet): Fleet 101, a teaching guide to Fleet and its formal models - #329

Open
itsHabib wants to merge 3 commits into
mainfrom
docs/fleet-101
Open

docs(fleet): Fleet 101, a teaching guide to Fleet and its formal models#329
itsHabib wants to merge 3 commits into
mainfrom
docs/fleet-101

Conversation

@itsHabib

@itsHabib itsHabib commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Superseded teaching implementation: #335. Do not merge both PRs. The original detailed material is preserved in #335 as fleet-reference.md; the focused fleet-101.md and Workbench entry path also live there. Keep this PR open as review history while the operator reads #335. After #335 lands through Gate, close #329 as superseded without merging. The operator read-through hold remains in force.

Summary

Adds cmd/fleet/docs/fleet-101.md: a teaching guide to Fleet and its formal models, written so the operator can learn the whole thing from one document. It follows docs/workbench-101.md: a one-screen orientation, verified / live / intent markers, a path:line citation for every claim, and a drift log at the end. It is linked from the Guides section of cmd/fleet/README.md and from docs/README.md.

Do not merge yet. This is an operator-facing teaching doc. It is held for the operator's own read: no gate run and no merge until they have read it and said so.

What it covers

  1. What Fleet is for and its five rules, each tied to the code that enforces it.
  2. The store under ~/.fleet, with the process that writes each file: hook, verb, watcher, the watcher's exit goroutine, the Node bridge, the macOS process observer.
  3. The hook's six events and what each writes. The hook is registered only for write-capable tools (verbs/role.go:141, :167): Read, Grep, Glob and MCP tools never reach it.
  4. Leases: branch and resource keys; the order of checks in CheckLease; dead vs unreadable holders; takeover, revoke and release; where preWriteVerdicts and switchDestinations apply it; what a lease does not stop.
  5. Rows, dispatch, receipts at an exact clean head, and fleet done.
  6. Mail: idempotent ids, delivery stamps as the reservation, acknowledgement, lateness reports.
  7. The watcher's fold and one delivery end to end in 34 numbered steps, each naming its process, its durable write (or none) and its citation; when a directory is free again; release, resume, at-most-once.
  8. The models: how to read the Quint; what TLC's check means and what Apalache's means; why the mutants exist; the crash/replacement model; how TestCrashReplacementModelTrace replays the frozen trace against real Go and a real process tree.
  9. What is not modeled: the launch/reservation protocol from Run Fleet wakeups through Claude and Codex sessions #318 and fleet watch release: an attested end for a reservation evidence cannot close #326.
  10. Limits and open gaps, a self-test, a glossary, and the drift log.

Findings recorded, not fixed

  • The stuck starting launch record (deliberately unfixed; kept as the first target of the planned crash-point harness). If the watcher dies between run() writing the launch record as starting and rewriting it as running, readLaunch refuses every fold, watch release and watch cancel both refuse (processState is unknown for pid 0), and the reserved mail stays stamped. Only deleting the launch file by hand clears it. Re-verified at 5f9d837 and again at ea76b62 with a throwaway test (not committed). The same state is reachable without a crash when providerCommand returns an error (watch/runtime.go:134-137). The Simplify Org roles and make headless Fleet observable #310 review raised this as P2-5; it never reached that PR's dispositions or FOLLOWUPS.md.
  • A one-shot fold can lose the bridge's request (found while checking main's red CI). provider.Command passes the request via cmd.Stdin = strings.NewReader(...), so os/exec copies it from a goroutine; fleet watch --once can exit before the copy, and the bridge dies on empty stdin (SyntaxError: Unexpected end of JSON input) with its mail stamped and the directory reserved. Reproduced 3 of 24 runs under parallel load at ea76b62. main's CI failures at delivery-scenario.py:121 (on 779cb75 and ea76b62) are consistent with it; the CI logs don't keep the bridge log, so that link is not proven. The fix is split into its own task, not this PR.
  • A silent unverified session blocks delivery indefinitely. present() applies no age limit (watch/deliver.go:374-393). A throwaway test confirmed a 30-day-old parent-unverified record, which SessionAlive calls dead, still prevents a launch.
  • One model invariant checks a label, not the property. noSilentResourceTakeover passes because no action in the reference records a Silent fact; its mutant fails it only because the mutant adds that label itself. A scratch copy of the mutant without the label passes all four invariants under TLC. The "no silent resource takeover" property holds by construction of write/takeover, but no invariant checks it.
  • TLC ignores --max-steps. Measured at 6, 12, 20 and 30 steps (reference modules) and 3 and 12 (crash modules): TLC explores the complete reachable graph every time (325, 349, 20 and 26 distinct states, nothing left on the queue). The "to twelve steps" wording in model/README.md, CLAIMS.md, CRASH-REPLACEMENT.md and judge.sh understates the result.
  • Eighteen more drift-log rows where docs or code comments disagree with the code, among them: role resolution is exact-match, not longest-prefix; the watcher also writes lateness mail and a GitHub cache outside watch/; the README and the boundary spec still call provider sessions future work; fleet done does not read receipts posted to pull requests.

Validation

  • An independent fact-check pass over the draft found 19 wrong or overstated claims and 7 bad citation ranges; all were verified against the code and fixed before this PR opened.
  • Written against main at ea76b62 (Record run 7 and clear a stale refusal from completed turns #328). Every path:line citation (422 unique) resolves to an existing range, and each was read against the cited code; citations in files Record run 7 and clear a stale refusal from completed turns #328 changed were re-pinned after the rebase.
  • (cd cmd/fleet/model && ./judge.sh) on macOS, Quint 0.32.0: ALL CHECKS PASS at 5f9d837 (exit 0, 33 s) and again after rebasing onto ea76b62. The output is reproduced in section 9.
  • gofmt -l . clean; go vet ./... clean; golangci-lint run ./... 0 issues; go test ./... ok (88 packages).
  • The throwaway tests and scratch copies of the scenario were deleted or kept outside the tree; the diff contains only the guide and two link lines.

No code changes.

🤖 Generated with Claude Code

One document that teaches Fleet top to bottom in the style of
docs/workbench-101.md: an orientation block, verified/live/intent
markers, a path:line citation for every claim, and a drift log.

It covers the five rules, the store and the process that writes each
file, the hook's six events, leases and CheckLease, rows and receipts,
mail, the watcher's fold and one delivery end to end, and the Quint
models (TLC, Apalache, the mutants, and the crash/replacement replay).
It records the stuck "starting" launch record as an open gap without
fixing it, and notes that the launch/reservation protocol has no model.

Linked from cmd/fleet/README.md's Guides section and docs/README.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@itsHabib itsHabib added the documentation Improvements or additions to documentation label Sep 12, 2026
@itsHabib

Copy link
Copy Markdown
Owner Author

@codex review

@itsHabib

Copy link
Copy Markdown
Owner Author

@claude review

@itsHabib
itsHabib requested a lite review from Copilot September 12, 2026 06:44
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@claude

claude Bot commented Sep 12, 2026

Copy link
Copy Markdown

Claude finished @itsHabib's task in 3m 28s —— View job


Review: docs(fleet): Fleet 101

  • Gather context and understand request
  • Read cmd/fleet/docs/fleet-101.md (all 1321 lines)
  • Read README link additions (cmd/fleet/README.md, docs/README.md)
  • Compare structure against docs/workbench-101.md pattern
  • Spot-check citations against source code
  • Review findings/drift log section
  • Post full review

Summary

This is a high-quality documentation PR with rigorous self-imposed standards. All spot-checked citations are accurate, the structure follows the workbench-101 pattern cleanly, and the drift log alone justifies the PR. The noted findings below are suggestions and minor observations — none are blocking.


Spot-checked citations (all passed)

claim citation result
Claude hook matcher `^(Bash Edit Write
Codex matcher adds apply_patch role.go:141 ✅ matches codexRules path
CheckLease 9-step order: fast path → lock → free → malformed → unreadable → alive → dead-resource → dead-branch → timeout policy.go:379-448 ✅ order matches exactly
Role needs exact match; tenant is longest-prefix lanes.go:143-167 ✅ code comment at :144 says the same thing verbatim
noSilentResourceTakeover checks label, not property reference.qnt:151-154 ✅ invariant is not(SilentA.in(facts) or SilentB.in(facts))
present() applies no age limit for parent-unverified sessions deliver.go:374-393 present() only skips pid_kind == "harness" sessions; unverified sessions bypass SessionAlive's StaleS check entirely
Watcher claims to write "only outside watch/ via mail stamps" watch.go:20-21 ✅ comment is there; drift log catch is real

Observations

1. Orientation block vs Section 4 completeness

The Orientation screen (line 46) says:

"Read, Grep and Glob never reach it"

Section 4 is more complete:

"Read, Grep, Glob, web tools and MCP tools never reach the hook"

The Orientation explicitly says "read this for the shape; read the sections for the evidence," so the abbreviation is stylistically appropriate. However, the one omission worth considering: web tools and MCP tools being excluded from the hook has a security/trust implication (an MCP tool that writes files bypasses all lease checks), and this is exactly why Section 4 includes the fuller list. Consider whether the Orientation should at least say "read-only and MCP tools" to set the right mental model before the reader reaches Section 4.

2. present() gap — mechanistic explanation could be tighter

Section 11 (line 1200) says:

"a 30-day-old unverified record that SessionAlive reports dead still prevented a launch"

This is accurate, but a careful reader will immediately ask: "if SessionAlive has a StaleS-based age check, why does it still block?" The answer is that present() doesn't call SessionAlive for parent-unverified sessions — it only gates on pid_kind == "harness" sessions via processPresent. SessionAlive's age check is never reached. This mechanism is implicit in the Section 8 delivery checks description, but making it explicit in Section 11 (one parenthetical: "because present() bypasses SessionAlive's age check") would make the gap easier to debug when someone hits it.

3. --once stdin race: CI linkage is honest

The guide correctly marks the CI linkage as unproven: "The CI logs do not keep the bridge's log, so the two CI failures are consistent with this mechanism rather than proven to be it." This is the right epistemic standard. The reproduction methodology (serial vs parallel runs; 3/24 failures under load) is solid enough to treat as the likely cause.

4. noSilentResourceTakeover analysis

The guide finds and proves a real weakness:

"A mutant that handed a dead holder's resource over without adding the label passes all four invariants (verified 2026-09-12: TLC on a scratch copy of the mutant with the label removed found no violation)"

This is the sharpest finding in the guide. The invariant holds because of how write and takeover are constructed, not because the invariant itself checks it — a subtle but important distinction. The analysis is correct and the methodology (running TLC on the de-labeled scratch mutant) is sound. Worth noting in the CLAIMS.md drift log entry when the docs are eventually updated.

5. ReleaseSessionState source map entry

Drift log, second-to-last row:

ReleaseSessionState is in session.go | cmd/fleet/model/SOURCE_MAP.md:25 | It is in cmd/fleet/internal/fleet/lease.go:233-240

Verified: lease.go:233-240 contains func ReleaseSessionState. This is a real discrepancy in the source map that a model following SOURCE_MAP.md would misroute on.

6. The watcher's "transitional interface" paragraph

The drift log correctly flags cmd/fleet/README.md:456-458:

"The command launcher is a transitional interface; durable Claude/Codex session integration is the next runtime step"

And notes: "Behind code since #318" and confirmed at FOLLOWUPS.md:699-700. This paragraph has presumably survived several PRs without anyone catching that it was stale before this guide was written. The drift log catches 18 such items; this is one of the more embarrassing ones because it directly contradicts how Fleet is actually running today.

7. Five rules vs Four rules (OVERVIEW.md)

The guide says "This guide follows the README" rather than OVERVIEW's four rules. This is a clean decision. The drift log records it at line 1302. Worth flagging for the follow-up where OVERVIEW gets updated: the missing "one holder per key" and "no domain word" from OVERVIEW are load-bearing rules that every operator needs to understand.


README link additions

Both link additions look correct:

  • cmd/fleet/README.md — adds fleet-101.md as the first entry under "Guides" with an accurate one-line description matching the guide's scope
  • docs/README.md — adds the guide to the table with a description that matches the guide's actual content

The docs/README.md link uses a relative path (../cmd/fleet/docs/fleet-101.md) which is correct for cross-directory linking in this structure.


Structure quality

Follows workbench-101.md pattern exactly:

  • One-screen orientation block with explicit "read this for the shape" framing ✅
  • verified / live / intent markers defined with commit SHA ✅
  • path:line citation for every claim ✅
  • Drift log at the end ✅
  • Self-test section is well-designed — 13 questions at the right difficulty, all answerable from the guide ✅
  • Glossary is clean; no term defined elsewhere contradicted here ✅

The 34-step delivery walkthrough in Section 8 (process → durable write → code citation for every step) is the guide's highest-value section. This is the kind of mapping that takes weeks to reconstruct from code; having it written down and verified is worth more than the rest of the guide.


Minor nits

  • Section 3 store table: the dispatch row says "verbs dispatch, reassign, undispatch, request" — worth checking if reassign actually writes to dispatch/ or only rewrites a column (the code suggests it rewrites the dispatch file, so this is likely correct, but the citation cmd/fleet/internal/verbs/work.go:44-48 for reassign could be confirmed)
  • Section 9 "The judge at this head": the judge output reproduced verbatim is excellent. One observation: the output says "12-step configuration" in four PASS lines — which the guide correctly explains understates the result. This creates a small loop where the guide's evidence section contains text it has already reframed as inaccurate. This is fine as a quoted artifact; just noting the slight cognitive dissonance for a first-time reader.

Findings recorded, not fixed (from PR body)

All four recorded findings are correctly marked as not fixed:

  1. Stuck starting launch record — gap is real, deliberately deferred, first target of planned crash-point harness. The three paths that reach it (watcher dies after starting write but before running write; providerCommand returns error; cmd.Start fails and rewrite-to-failed also fails) are all documented. ✅
  2. --once stdin race — fix correctly split into its own task. ✅
  3. Silent unverified session blocks delivery indefinitely — confirmed by throwaway test. The gap between SessionAlive's StaleS check and present()'s bypass is correctly identified. ✅
  4. noSilentResourceTakeover label dependency — proven weakness, honest characterization. ✅

Verdict

No blocking issues. The guide is accurate where checked, honest about its own limits, and provides genuine value beyond what's available anywhere else in the tree. The drift log is the most actionable section for follow-up work. The three structural suggestions above (Orientation wording, present() mechanistic explanation, noSilentResourceTakeover CLAIMS.md note) are all optional improvements for a future pass, not requirements for this PR.

The PR body's "Do not merge yet" note is respected — flagging here for completeness rather than as a review verdict.

@github-actions

Copy link
Copy Markdown

Reviewer: claude
Reviewed commit: be75d2f1e28981e111abeafcfd61d7520d01e1d0

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The guide contains unresolved factual overstatements and misleading explanations that should be corrected before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds an operator-facing Fleet 101 guide covering Fleet behavior, storage, leases, delivery, formal models, and known gaps.

Changes:

  • Added fleet-101.md.
  • Linked it from both documentation indexes.
  • Made no runtime code changes.
File summaries
File Description
docs/README.md Links the Fleet 101 guide.
cmd/fleet/README.md Adds the guide under Guides.
cmd/fleet/docs/fleet-101.md Comprehensive Fleet teaching guide; several accuracy corrections remain needed.
Review details

Suppressed comments (7)

cmd/fleet/docs/fleet-101.md:473

  • This overstates the two-record write: cmdDispatch calls CmdAssign only when slot != "" (cmd/fleet/internal/verbs/work.go:141-148), so an unseated dispatch writes only the dispatch row. Please qualify this paragraph to seated dispatches so operators do not infer that every dispatch has an assignment record.
A dispatch writes two records, the assignment and the row, and a failure between them can
leave an assignment that wakes its worker with no row. This is recorded, not fixed
(`FOLLOWUPS.md:63-79`).

cmd/fleet/docs/fleet-101.md:1304

  • This drift entry says the new store table omits paths that the table itself lists: deliver.json, handoff/, stop/, prs/, cache/github/, inflight/, costs.jsonl, actions.jsonl, hook-errors.jsonl, migrated-keys.v1, watch/delivery/, watch/late.json, and watch/owner.lock all appear in section 3. Remove the false omission list and retain only the verified mail-path discrepancy.
| the store table | `cmd/fleet/README.md:78-90` | The mail path shows a raw `<tenant>` and `<address>` (`cmd/fleet/README.md:86`); the code uses their SHA-256 digests (`cmd/fleet/internal/fleet/mail_address.go:15-17`). The table also omits `deliver.json`, `handoff/`, `role-handoff/`, `stop/`, `last-word/`, `prs/`, `cache/github/`, `inflight/`, `locks/`, `decisions.jsonl`, `costs.jsonl`, `overrides.jsonl`, `actions.jsonl`, `hook-errors.jsonl`, `migrated-keys.v1`, `watch/delivery/`, `watch/late.json` and `watch/owner.lock` (section 3) |

cmd/fleet/docs/fleet-101.md:99

  • roles.json is present in this new guide itself, so the parenthetical is literally false even though the intended claim about the implementation is sound. Limit this statement to Fleet source code so the guide does not contradict its own text.
never reads Org's registry (`roles.json` appears nowhere in `cmd/fleet`); the two share only

cmd/fleet/docs/fleet-101.md:692

  • A successful run does not guarantee process_identity: processIdentity errors are ignored at runtime.go:148, leaving a valid launch record with only the PID, which later produces unknown status. Qualify this as an attempted/available identity so the delivery evidence is not overstated.
| 13 | watcher | on success: launch record rewritten `status: "running"` with `pid` and `process_identity` (the process start time) | `cmd/fleet/internal/watch/runtime.go:147-152` |

cmd/fleet/docs/fleet-101.md:735

  • fleet receipt is not verifier-only: any live session may record any kind, with role/lane kept as provenance (receipts.go:42-70). This wording contradicts section 6 and can incorrectly teach workers that they cannot publish receipts.
| 28 | verb run by the agent | `acked_at` on each message it handled; a `handoff/` checkpoint; a receipt, if it is a verifier | `cmd/fleet/internal/fleet/mail.go:249-277`, `cmd/fleet/internal/verbs/keys.go:403-427`, `cmd/fleet/internal/verbs/receipts.go:108-129` |

cmd/fleet/docs/fleet-101.md:1077

  • The gap is not recoverable only by deleting a file: the guide later documents manually editing the launch record or adding an exit file as another way to make readLaunch accept it (lines 1113-1116). Describe this as manual removal or editing, while retaining that no Fleet verb recovers it.
is a **progress** failure: a state from which nothing but a person deleting a file ever leads
back to a directory that can launch. A safety invariant cannot express that. A model of the

cmd/fleet/docs/fleet-101.md:865

  • The model explanation conflates the model's atomic write action with the implementation's locking. CheckLease returns through an unlocked same-session fast path at policy.go:385-391; only the contended acquisition and takeover path is inside KeyLock. Please describe the model step as an abstraction and call out this fast path, otherwise readers may infer that the lock closes races it does not.
- **The decision.** `action write(s)` is `CheckLease`, taken as one atomic step because
  the real code runs it under `KeyLock`. Read it next to
  `cmd/fleet/internal/fleet/policy.go:392-440`: free takes the key; the holder's own write
  proceeds; a rival that is `Alive` or `Unreadable` is refused; a `Dead` rival's branch is
  taken over and its resource refused (`cmd/fleet/model/model/reference.qnt:44-71`).
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cmd/fleet/docs/fleet-101.md Outdated
Copilot: scope at-most-once to started launches (a failed start gives
its stamps back); two dispatch records only with --slot; process
identity may be missing and then reads unknown; any live session may
record a receipt; the model's atomic write abstracts CheckLease, whose
holder fast path is unlocked; the stuck record needs removal or editing;
roles.json is absent from Fleet's Go source, not from cmd/fleet; name
the README's table in the store-table drift row.

Claude: name web and MCP tools in the orientation; say present() never
calls SessionAlive; cite reassign's dispatch write; note the judge's
"12 steps" wording under its output.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@itsHabib

Copy link
Copy Markdown
Owner Author

@claude review

@itsHabib
itsHabib requested a lite review from Copilot September 12, 2026 07:01
@claude

claude Bot commented Sep 12, 2026

Copy link
Copy Markdown

Claude finished @itsHabib's task in 3m 22s —— View job


Review: docs(fleet): Fleet 101 (round 2)

  • Load tools and post initial status
  • Read current fleet-101.md post-e6bffcf fixes
  • Verify all 8 copilot findings were correctly addressed
  • Independent review pass for new issues
  • Post full review

Copilot Round 1 Findings: All 8 Addressed

Reviewed e6bffcf against each of the 8 copilot findings (7 suppressed + 1 explicit).

1. Unseated dispatch two-record write (line ~473)
Fixed. Original: "A dispatch writes two records" (implied all dispatches). Now: "A dispatch with --slot writes two records, the assignment and then the row; without a seat it writes only the row." Accurate per work.go:141-157.

2. Drift entry false omission list (line ~1304)
Fixed cleanly. Changed from "The table also omits" (which wrongly implied this guide's table was missing entries) to "The README's table also omits ... all of which this guide's section 3 lists." The drift finding is now accurately scoped to the README's incomplete store table, not this guide's.

3. roles.json parenthetical (line ~99)
Fixed. Changed from "roles.json appears nowhere in cmd/fleet" (literally false, since this guide is under cmd/fleet/docs/) to "no Go source under cmd/fleet mentions roles.json" — accurate.

4. processIdentity guarantee overstated (line ~692)
Fixed. Row 13 now reads: "with pid and, when it can be read, process_identity (the process start time). An identity read error is ignored here; while the process is alive, later folds then read the launch as unknown (cmd/fleet/internal/watch/status.go:85-95)." Accurately reflects that processIdentity errors are swallowed at runtime.go:148, leaving the identity absent, causing later status checks to return unknown for a pid that's still alive.

5. fleet receipt verifier-only claim (line ~735)
Fixed. Row 28 now reads "a receipt when it records one (any live session can; section 6)" — correctly cross-references the #312 change documented at receipts.go:42-44.

6. starting record recovery overstated (line ~1077)
Fixed consistently across three locations: the stuck-record description (step 6: "remove ... or edit it"), the progress-failure sentence ("nothing but a person removing or editing the launch record"), and self-test Q11 ("remove or edit"). The edit paths (status: "released" or adding an exit file) are now described.

7. Model atomicity conflated with locking (line ~865)
Fixed thoroughly. The explanation now distinguishes: (a) rivals under KeyLock (policy.go:392-440) — the lock makes the rival's decision atomic; (b) the holder's own repeat write — takes an unlocked fast path that "only reads its own lease and writes nothing" (policy.go:385-391). "The model assumes that and does not model the lock itself" is the correct framing.

8. At-most-once claim for failed start (explicit comment)
Fixed. The paragraph now opens "Each message and each assignment is carried by at most one launch that started" and explicitly adds "A launch whose start fails gives its stamps back, so a later fold may carry the same message again (deliver.go:316-320)." The distinction between a failed start (stamps returned) and a started turn (stamps permanent) is clear.

Also confirmed: the Orientation block now reads "Read, Grep, Glob, web and MCP tools never reach it" — matching section 4's fuller list. (The original round 1 review noted this discrepancy; it has been addressed in e6bffcf.)


Independent Review Pass

No blocking issues found. One observation worth noting:

Present()-and-parent-unverified wording
Section 11 at line 1207–1208 reads: "A parent-unverified record left by a session that died without SessionEnd (on Windows, any session that dies that way) blocks delivery..." The parenthetical "(on Windows, any session that dies that way)" is slightly ambiguous — "that way" refers to dying without SessionEnd, but the deeper point is that Windows sessions are always parent-unverified (per section 5). A reader who hasn't reached section 5 yet may not catch this. Optional improvement: "(on Windows, any session that dies without SessionEnd, since all Windows sessions are parent-unverified)". Not blocking.


Summary

All 8 copilot findings from round 1 are correctly addressed. The fixes are technically accurate, consistent with the cited code, and don't introduce new issues. The guide's internal consistency holds — the delivery walkthrough, the processState table, the at-most-once paragraph, and the self-test answers are all aligned with the fixes. The drift log entry for the store table is now a sharper, more useful finding than before.

No blocking issues. Ready for operator read per PR body.

@github-actions

Copy link
Copy Markdown

Reviewer: claude
Reviewed commit: e6bffcf13ea357c6c91eb349b99814308c6f73bc

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The guide has multiple outstanding documentation corrections and is explicitly held for final human review.

Review details
  • Files reviewed: 2/3 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread cmd/fleet/docs/fleet-101.md Outdated
Comment thread cmd/fleet/docs/fleet-101.md Outdated
Copilot: the orientation now says CheckLease locks only when the key is
free or held by someone else, and allows the holder's own write through
an unlocked read; walk row 12 is limited to cmd.Start() failures and
points at the providerCommand and failed-rewrite paths that leave the
starting record (section 11).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@itsHabib

Copy link
Copy Markdown
Owner Author

@claude review

@itsHabib
itsHabib requested a lite review from Copilot September 12, 2026 07:17
@itsHabib

itsHabib commented Sep 12, 2026

Copy link
Copy Markdown
Owner Author

Review disposition after two fix rounds (head d82d654)

Status: held for the operator's read. Not for gate or merge until the operator has read the guide.

Rounds

cycle head Claude Copilot Codex
initial be75d2f no blocking issues; 4 optional suggestions changes recommended: 8 findings unavailable (usage limit; needs credits)
round 1 e6bffcf all 8 Copilot findings confirmed fixed; no blocking issues 2 findings not re-requested
round 2 (final) d82d654 both round-2 fixes confirmed; no blocking issues 0 new comments; all 3 earlier threads resolved not re-requested
  • Round 1 (e6bffcf) fixed all 8 Copilot findings, each verified in the code first: at-most-once scoped to started launches; two dispatch records only with --slot; process_identity may be missing; any session may record a receipt; the model's atomic write versus the unlocked holder fast path; removing or editing the stuck record; roles.json absent from Fleet's Go source; the store-table drift row names the README's table. It also took Claude's 4 optional suggestions.
  • Round 2 (d82d654) fixed Copilot's 2 findings: the orientation's lease summary now separates the locked claim/takeover path from the unlocked holder fast path, and walk row 12 is limited to cmd.Start() failures, pointing at the providerCommand and failed-rewrite paths that leave the starting record.

The cycle on d82d654 was the third and final panel run. It raised no P1 or higher, so no further fix was made and no further panel request goes out. CI is green on d82d654.

Residuals: accepted or deferred

  • Accepted (P3, Claude round 2): the section 11 parenthetical "(on Windows, any session that dies that way)" stays as written. It is accurate, and section 5 already states that every Windows session is parent-unverified.
  • Deferred (out of scope): correcting the documents the drift log names. This includes cmd/fleet/README.md, OVERVIEW.md, headless.md, model/README.md, CLAIMS.md, SOURCE_MAP.md, the org–fleet boundary spec and the code comments, among them Claude's suggested CLAIMS.md note on noSilentResourceTakeover. This PR adds a guide and deliberately changes no other doc's claims. Each row names the file and line to fix.
  • Deferred (own task): the watch --once lost-request bug (section 11) and the flaky delivery-scenario.py:121 check it probably explains. The fix is split into a separate task and is not in this PR.
  • Deferred (by design): the stuck starting launch record stays unfixed as the first target of the planned crash-point test harness.
  • Panel coverage: Codex did not review any cycle because of its usage limit. The required reviewer (Claude) and Copilot reviewed every cycle.

@claude

claude Bot commented Sep 12, 2026

Copy link
Copy Markdown

Claude finished @itsHabib's task in 3m 4s —— View job


Review: docs(fleet): Fleet 101 (round 3)

  • Load tools and post initial status
  • Read fleet-101.md at current head (d82d654)
  • Verify round-2 copilot fixes (orientation locking, row 12 start error)
  • Independent review pass
  • Post full review

Reviewed commit: d82d654 (fold of round-2 copilot fixes)

Round-2 Copilot Fixes: Both Correctly Applied

Fix 1 — Orientation locking behavior

Copilot flagged that the orientation overstated CheckLease as always using a lock. The current text (lines 48–53) now correctly distinguishes both paths:

"When a key is free or held by someone else, CheckLease reads, decides and writes the lease inside a kernel file lock; when the lease already names the caller, it allows the write through an unlocked read that changes nothing."

This matches policy.go:385-391 (fast path) and policy.go:392-440 (locked path). ✅

Fix 2 — Row 12 start error table

Copilot flagged that row 12 claimed every start error rewrites the record to failed, but providerCommand errors exit before that rewrite. The current text (line 693) now reads:

"if cmd.Start() fails: launch record rewritten status: "failed", then launch gives the stamps back. A providerCommand error returns before this rewrite, and a failed rewrite returns too; both leave the starting record from step 10 (section 11)"

This is accurate and consistent with runtime.go:134-146. The three distinct failure paths (normal cmd.Start() failure → rewritten to failed; providerCommand error → stuck starting; failed rewrite → stuck starting) are all now represented. ✅


Independent Review Pass

No new blocking issues. Two observations:

1. Windows parenthetical (optional, carried from round 2)

Section 11 (line 1208–1209) reads:

"A parent-unverified record left by a session that died without SessionEnd (on Windows, any session that dies that way)"

The deeper point — that Windows sessions are always parent-unverified, not just when they die without SessionEnd — is established in section 4 (lines 345–347) but the parenthetical here is ambiguous for a reader who hasn't internalized that yet. This was noted as optional in round 2 and remains minor; not blocking.

2. Self-test Q1 alignment with preToolUse order (no issue)

Self-test Q1 asks what happens when a session runs cd ~/dev/repo-author-1 && git commit. The answer says "PreToolUse denies it before the lease check: the directory guard runs first." This matches section 5's description of preWriteVerdicts ordering (stop flag → directory guard → CheckLease). The guide is internally consistent. ✅


Review cycle position

Per CLAUDE.md review-cycle discipline: this is round 3 of the panel — the cap is two fix-rounds. Both fix rounds were warranted (round 1 addressed 8 genuine copilot findings; round 2 addressed 2 more). The remaining optional observation above is not P1-or-higher and does not justify a fourth fix round. It should be recorded as a deferred residual and accepted by the judge as-is.


Summary

Both round-2 copilot findings are correctly and precisely fixed. The guide remains internally consistent across all three rounds of review. No blocking issues at this head. Ready for operator read per PR body.

@github-actions

Copy link
Copy Markdown

Reviewer: claude
Reviewed commit: d82d65464de303abdebf15ece1095e78a10a83c1

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Unresolved documentation accuracy clarifications must be addressed before approval.

Review details
  • Files reviewed: 2/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@itsHabib

Copy link
Copy Markdown
Owner Author

Independent design review: Fleet 101 (#329)

Recommendation: SIMPLIFY. Keep the material, change its job. It is a useful engineering reference and incident investigation, but I would not publish this version as the primary first-reader introduction. Put a short, task-first explanation ahead of it, reconcile the operational guidance, and correct the concrete errors below. Preserve the operator's explicit read/merge hold.

Fleet's direction is sound: editable responsibilities, one accountable lead for work, ordinary peer questions, independent occupancy/work/mail, optional Org, and provider adapters beneath one Go watcher. The evidence supports continuing that small design. It does not justify another hierarchy, compulsory lifecycle, policy engine, or general formal-verification platform.

Subject and evidence boundary

Reviewed 2026-09-12. Remote verified as git@github.com:itsHabib/workbench. The isolated review checkout was switched to the exact PR head; tracked source remained unchanged.

Subject Verified revision/status
#329 OPEN, non-draft, held for operator read, d82d65464de303abdebf15ece1095e78a10a83c1
Guide's declared source snapshot ea76b6230210d42802cb2967938341090e73dd3d (#328)
Current remote main 3c3ced644cc165a4192e92b310bdf23daf9bb851
#330 MERGED at 2026-09-12 17:40:47 UTC; source 6db1b0e65b74bb4aea6f00a3c7bf6d8812916887, merge 3c3ced644cc165a4192e92b310bdf23daf9bb851
#324 OPEN; latest update 2026-09-12 05:42:14 UTC; Windows rerun outstanding

These heads/statuses were refreshed before writing this report and rechecked unchanged immediately before publication. API snapshots, test logs and overlay probes are retained with the local review artifact; they are not attached to this comment. Read all 1,331 lines of the proposed guide, both index changes, repository charter/instructions, relevant runtime/model source, installation/onboarding/run/headless/validation guides, and prior review dispositions. There is no scoped cmd/fleet/AGENTS.md or cmd/fleet/docs/DESIGN.md at this head despite the root map's general description; the root guidance and Fleet-specific documents supplied the context.

#329 has already used its three panel cycles. All three inline threads are resolved/outdated; I verified the ten recorded corrections and do not reopen them. Claude reviewed the final head; Codex's bot review was unavailable due to quota. This report is the requested independent design review, not another panel request or merge authorization.

Ranked findings and changes

1. High design priority: replace the first-reader path, retain the reference

Text: “learn the whole thing from one document. Read it top to bottom” (guide lines 3–7). The orientation says an agent can stop there and return to leases and watcher details (lines 29–30).

Demonstrated structure: 13,159 whitespace-delimited words; a complete storage table starts at line 147, while mail starts at 515. There is no opening worked example of a lead answering a worker's routine question, no compact operational kickoff, and no direct initial path through install → run → inspect. A 34-step launch walk is excellent maintainer material. It is a poor prerequisite for dispatching one task.

Judgment, not a measured learning result: the proposed reading order optimizes for reconstructing the implementation, not learning to coordinate work. No first-reader trial is retained. The README links it as the first guide, which makes that distinction consequential.

Better alternative: reuse run-a-fleet.md as the operational spine. Make Fleet 101 a short conceptual front door with one worked task, then link to installation and that run guide. Retain sections 3–11 as an optional engineering reference; put formal models beside their existing claims ledger. Do not create another competing command manual. If one physical document matters, use a short front section with explicitly optional deep chapters instead of “read top to bottom.”

Replacement opening:

Fleet helps a lead coordinate agents working in separate directories. The lead assigns a result, answers routine questions and checks evidence. Workers can ask relevant peers directly. The top lead normally brings scope, spend and authority decisions to the operator. Fleet records sessions, work and mail; it does not decide the team's organization. Role instructions are editable prose, and Org is an optional way to register and read them. Start with one lead and one worker. Add another lead only when the work needs one.

To run that team, follow the installation guide and run-a-fleet guide. The engineering reference below explains how the records and checks work; it is optional for first use.

2. P2 accuracy/clarity: state the actual hook protection boundary before claiming ownership enforcement

Text: Fleet “refuses the writes that would break ownership” (lines 82–87); “A lease gates the next tool call the hook sees” (line 429). The orientation correctly warns that MCP writes bypass checks, and section 5 correctly describes the Git classifier. It never gives the equally important plain-language consequence for normal shell writes.

Demonstrated: IsWrite recognizes named file-edit tools and a bounded set of Git/gh Bash patterns (policy.go:135–140). preWriteVerdicts invokes CheckLease only when writes is true (hook.go:334–338). My retained overlay probe confirms printf x > tracked.txt, python3 repair.py, sed -i.bak ..., and make generate all classify false. No shell command was executed by that probe. Other hook checks, such as stop and directory guards, may still apply.

Consequence: reaching a hook does not establish branch-lease coverage. This is broader than the already-fixed MCP omission, and independent of the correctly documented surviving-child limitation. It is a misleading summary, not a newly discovered runtime regression.

Replacement:

Fleet checks cooperative tool admissions, not filesystem writes. Registered file-edit tools and recognized Git/gh shell operations receive branch-lease checks. Ordinary shell generators, redirections and scripts can write without that branch-lease check; tools outside the hook matcher bypass the hook altogether. The kernel lock serializes Fleet's lease records. It does not lock the worktree, terminate children, or isolate one process from another. Keep separate worktrees and respect existing occupants; a dead session alone does not establish that its effects have stopped.

3. P2 current-main drift: refresh the request handoff and #324's actual open question

Text: row 11 says the request is copied to stdin by a goroutine (line 692); section 11 calls request loss “open” (1155); provider.Command “can fail only” on executable lookup or JSON encoding (1142–1144); #324 is “the programmatic-hook follow-up” (1223).

The source-snapshot disclaimer is honest: stdin was correct at ea76b62. It is no longer an explanation of current main. #330 now creates an exclusive private .request.json before starting Node and passes its path in argv (current provider.go). This removes the launcher-copy race, adds request-file I/O failures, and makes the new guide's file inventory and self-test Q13 historical. #330's open review thread and disposition explicitly leave request-file failure's starting record unresolved. Merge does not mean that residual was fixed.

#324's latest evidence identifies the extensionless Windows binary as a sufficient explanation and asks for a rerun after #325. Its unconditional programmatic-callback patch was not adopted. Real macOS settings-file hooks fired, including SessionEnd. Current official SDK documentation explicitly includes settings-file command hooks when their setting source is enabled. That supports investigating setup first; it does not qualify Windows.

Replacement status:

Current main includes #330: Fleet writes the complete request to a private per-attempt file before launching the bridge. The earlier stdin-copy failure is retained below as historical evidence. Request-file creation/write failures can still leave the launch record starting; that recovery gap remains open.

#324 remains open for a real Windows headless rerun after reinstalling fleet.exe and re-projecting hooks. macOS SDK sessions have produced actual Fleet hook records. Add programmatic callbacks only if the Windows rerun demonstrates a remaining mechanism failure and duplicate firing is addressed.

Refresh the current-operation section and immutable source links together. Keep the old race as a case study, with its fix and residual, rather than deleting useful history or silently repinning old line numbers.

4. P2 clarity: define the operational reply/escalation route without implying Org hierarchy

Text: unacked questions are mailed “to the addressee's parent” (lines 561–567), after Org was introduced as a registry with an optional parent.

Demonstrated: parentOf selects the newest matching work row's for role, otherwise the address's configured late_to; it never consults Org (late.go:272–293). With no destination it records late-no-recipient (252–256). Acknowledgement suppresses this report; an answer is not required by that condition. The prose can make a reader expect org charter -parent to wire an unattended escalation that does not exist.

Replacement:

Workers send routine questions to the assignment's reply_to mailbox or a relevant peer; leads answer and unblock authorized work. A late unacknowledged question is reported to the newest matching dispatch row's accountable role, or the address's configured late_to. Without either, Fleet records that no recipient exists. Org's parent field does not configure this route. An acknowledgement means handled mail, not proof that the dependency was solved.

Teach “only the top lead routinely asks the operator” as the run's working agreement. Do not enforce mandatory hops or make workers wait for charter traversal. Approval/refusal boundaries still apply wherever the provider or repository requires them.

5. P2 accuracy: self-test Q7 teaches the wrong release rule

Text: “The bridge exited 0 but provider_terminal is false. Is the directory free? (No. A bridge exit alone never releases it; watch status shows provider_cleanup_pending.)” (lines 1250–1251).

The question is underdetermined. providerTerminal also accepts matching provider_started: false or the strict pre-turn quiescence proof (runtime.go:215–242). The guide itself explains these alternatives at 776–785. My overlay test supplies exactly exit 0, false terminal, and matching never-started evidence; launchPresent returns false. This is a function-level counterexample to the quiz, not a claim that a normal production never-started failure exits 0. Existing tests also cover quiescence and watcher-loss release.

Replacement:

Not enough information. A collected bridge exit plus matching provider-terminal, never-started, or qualifying pre-turn quiescence evidence releases the launch reservation. Otherwise it stays reserved. A separate present session or stop flag can still prevent the next launch. None of these facts establishes task completion.

Existing documentation conflicts that affect adoption

These are disposition work, not fresh runtime bugs. #329 already records much of the drift; do not inflate that into a second finding list. Merely appending another drift ledger leaves several active instructions in disagreement.

Guide Assessment and smallest correction
install.md Best current setup source: optional example roles, environment consistency, .exe, actual hook verification. Link here first.
run-a-fleet.md Best current operational source: optional Org, direct peer contact, no quotas, checkpoints distinct from messages, exact-head evidence. Reuse it.
ONBOARDING.md:16–20 “One lead per bucket” leads with hierarchy. Start with one lead; add a bucket lead only for an observed coordination bottleneck.
ONBOARDING.md:46–48 “Existing Org-held work and terms keep their original owner until an explicit migration” conflicts with the current clean-cutover instructions and removal of Org claims. Replace the lifecycle sentence with preserving actual live work/dirty files while removing old callers.
README.md:12–19 “workers talk to their lead, never to the operator” is too absolute beside the direct-peer/no-hop run guide. State the normal escalation convention and actual authority exceptions.
README.md:456–458, boundary spec's “Next runtime step” Provider sessions are implemented; annotate the historical design or update the entry point. Do not leave an apparently pending replacement that another agent may implement again.
headless.md Most precise runtime guide. Keep its reservation/identity distinctions, correct the already-recorded post-release resume wording, and add a direct link to the unresolved starting recovery limitation.
model README/CLAIMS/SOURCE_MAP Reconcile TLC depth wording and label-invariant claim with Fleet 101. A claims ledger contradicted only in another document is not a reliable contract.

The headless guide also calls all of app-server “experimental.” Today's official reference distinguishes a stable surface from opt-in experimental methods. Qualify the particular installed methods/version instead. This is a small external-documentation refresh, not evidence that Fleet's platform integration is qualified.

Formal methods and reliability direction

Proceed with the existing bounded examples. The guide correctly distinguishes model safety, finite-state exploration, synthetic session records, real helper processes, the hand-projected resource trace, and lack of Go refinement. I reran judge.sh: all checks passed, including the branch B\nA\n / resource A\n replay. TLC found the same 325 reference-branch states at --max-steps=3 and 12, queue empty. An independently copied silent-takeover mutant with only silentFact(s) removed passed all four invariants, 349 states. These support the guide's strongest claims. The copies/logs are retained as review evidence.

Do not use a bigger model as the next deliverable. The unlabelled-takeover result means the property needs an independent observation of ownership transitions and explicit takeover authorization if that model is extended. Do not add more labels that only the implementation of the mutant can emit. Replay frozen witnesses through ordinary Go paths; add model checking when a specific interleaving question warrants it. Whole-Fleet proof is neither offered nor earned here.

The proposed progress property at lines 1084–1091 needs one qualification: “every state has a path back to free” proves recoverability, not that the system will take that path. Eventual recovery needs stated assumptions about watcher restart, scheduling and successful I/O. An indefinitely uncertain effect may correctly require operator action. A bounded claim such as “on the next healthy fold after a proven pre-start failure, this address can retry” is smaller and directly testable. Preserve safety when the provider may have started.

Simplify the planned crash-point work into one retained regression and repair. The starting gap remains visible in current main; #330 added an ordinary request-file I/O trigger. I disagree with preserving the deployed defect merely to have a target for a future harness. Preserve the bad revision or fixture, write the regression, then fix it in its separately authorized task. This review does not implement or take over that task.

The first repair should handle a proven no-start error: retain the failure reason, settle the launch as failed, and keep the request eligible. A crash after spawn but before PID publication is a different, uncertain case; do not make it retryable through age or fabricated terminal evidence. Define one supported inspection/recovery action before generalizing a crash injector. The existing manual deletion discussion honestly warns about duplicate launches, but a newcomer should leave with “preserve evidence and escalate an unresolved start,” not a quiz answer encouraging an invented exit file.

Keep the existing macOS no-fork observer narrowly scoped. It earned some value through a reproduced pre-turn failure, but it is not a descendant container or a platform-neutral termination guarantee. More OS machinery should follow a measured failure in the intended workload, with an ordinary process-supervision baseline, rather than a desire to make the architecture look complete.

What the workload evidence actually supports

The retained run 7 scorecard reports two workers, one lead, one verifier, a contended bench, 16 Claude turns, two draft PRs, two verify receipts and zero post-start operator interventions. I read its contract and scorecard and checked the retained /Users/mh/dev/r5/state aggregate: 16 attempt summaries, all terminal; five provider session IDs; five session records; two verify files. I did not independently recalculate its spend, elapsed time, or intervention count from every raw event.

That is useful real evidence for question/answer, dirty-work continuation, contention relay and lead closure. It is not a current-main full-workflow repeat or controlled evidence of cost superiority. The earlier provider ledger and #318's final comments preserve both failed attempts and version boundaries. Codex has real fresh/resume/cancellation and bounded sandbox evidence; the macOS pre-turn failure probe is controlled injection with explicit recovery, not unattended success. Windows packaging CI and cross-builds do not establish Windows headless hooks or process cleanup. #324 still needs that machine's evidence.

Smallest valuable slices and falsifiable criteria

  1. Teaching slice: one short front door, one lead/worker question → answer → continue → evidence example, explicit hook/isolation boundary, current setup links, optional internals/models. Give it to two fresh readers with one task and no verbal coaching. Accept if both identify who answers routine questions, operate without compulsory Org, explain stamp versus ack versus receipt, and identify the safe stop on unresolved runtime state. If either invents an Org lifecycle or assumes leases stop shell/child writes, revise the teaching before adding chapters. This trial is proposed, not performed.
  2. Reliability slice: one disposable store and fake provider; inject request preparation failure after the starting write, and separately kill after spawn before publication. Retain the cut point, files, child/effect evidence and next fold. Accept the fix only if proven no-start recovers on the next healthy fold with the request still eligible, while uncertain-start never duplicates execution and has a supported recorded recovery path. The old source must fail the new acceptance. Stop expanding the harness if a few direct tests expose the behavior; no generic event engine is required.
  3. Platform slice: the owner of fleet: settings-file hooks do not execute in SDK-spawned sessions, so headless sessions have no identity #324 reruns one fresh Windows headless turn using fleet.exe and projected hooks, then verifies actual SDK identity, mail, receipt and orderly session end. Accept only with correlated retained records. A generated hook file or go test on macOS is insufficient. If the extension fix works, close the hook-mechanism hypothesis; do not add callbacks. If it fails, investigate duplicate-safe integration in that task.

The next broader run should keep tasks, model settings and setup comparable to a desktop baseline and count completed accepted work, operator rescues, idle assigned time, coordination spend and recovery failures. If extra leads or recurring ticks do not reduce an observed bottleneck, remove them.

Fresh-agent kickoff using current supported behavior

Verify the repository remote, exact revision, actual worktree and run brief. Read Fleet's install.md and run-a-fleet.md; use headless.md only for a configured headless run. Keep the main checkout unbound and clean. Work in your assigned directory; preserve dirty work and respect live holders and exclusive resources.

Roles are editable prose. Org registration and parent links are optional and grant no authority. The assignment names the result and accountable lead; its reply_to is the routine reply address. Ask relevant peers directly within the tenant. Leads answer and unblock routine work; the top lead normally takes scope, spend and authority decisions to the operator. There is no compulsory Org lifecycle, message quota or one-action-per-turn rule.

Confirm a real startup hook record in this provider home before relying on Fleet identity. Read fleet work, fleet status --all, fleet watch status, and fresh mail. Acknowledge handled messages. Continue useful authorized work; when waiting, leave a useful handoff and end the turn. An existing Go watcher supplies configured headless wakes; desktop native loops remain supported. Do not create a second runtime owner.

Match receipts to the exact clean head and task acceptance; use fleet done <sha> --kind <kind>. A pass is the recorded verdict, not automatic proof of independence or value. Respect the requested delivery boundary and Gate. Stop configured addresses at the agreed outcome when the run authorizes you to do so.

Hooks are partial admission checks, not OS isolation. Current main includes #330's request-file handoff, but unresolved starting records and silent unverified sessions can still block delivery. Inspect and preserve evidence; do not fabricate session/exit records, treat silence as death, or blindly replay stamped work. Windows headless identity remains unqualified pending #324; Linux/Windows do not have the macOS Codex no-fork recovery proof. Do not claim general descendant cleanup on any platform. #329 remains held for the operator's read.

Validation and handoff

At d82d654: gofmt -l . produced no paths; go vet ./..., golangci-lint run ./..., go test ./..., go test ./cmd/fleet/..., and cmd/fleet/model/judge.sh passed. PR CI shows check/fuzz/hygiene and macOS/Windows seat jobs successful; skipped signal/Claude workflow entries are not additional reviews. I did not rerun the full race suite locally. Two overlay probes passed against unchanged implementation source; they demonstrate the misleading quiz and classifier boundary, not production qualification. The scratch Quint checks passed as described above. During verification, no live provider, installed configuration change, live runtime-state mutation, Gate call, merge, deployment, timer, or implementation edit occurred. This comment is published with the operator’s subsequent authorization.

Only genuinely operator-owned choices remain: whether this should remain a single physical reading artifact; whether the next deployment target is Windows (making #324 qualification the immediate prerequisite); and the authorized boundary/budget for the separate recovery repair and real workload. My defaults are a short front door plus reference, the platform you actually intend to use, and the smallest direct recovery test/repair. More hierarchy and formal tooling need no decision now because they have not earned a place.

This comment is the operator-authorized publication of the independent review handoff. No Fleet handoff/receipt was written because the review instruction explicitly forbids runtime-state changes. Next action: operator reads this review and #329; a separately authorized documentation pass can apply the replacements without reopening the exhausted panel cycle. Preserve the merge hold until the operator explicitly releases it.

@itsHabib

Copy link
Copy Markdown
Owner Author

Independent review: Fleet 101 and Fleet's direction

Reviewed 2026-09-12. Verdict: SIMPLIFY. Proceed with editable role prose, optional Org, Fleet-owned occupancy/work/mail, direct peer coordination and one headless launch owner. Preserve Fleet 101's forensic detail as an implementation reference. Replace the compulsory top-to-bottom introduction with a short path through one useful task. Stop treating a future crash harness as a reason to retain a known recoverable runtime defect.

#329 remains held for the operator's own read. This review neither supplies merge authority nor requests another review-panel round. Verification used isolated checkouts. This comment publishes the completed review with the operator’s subsequent authorization; it does not authorize implementation or release the merge hold. Writer #5 and Rooms were excluded.

Revisions and review state

Subject Exact revision / current state
Confirmed local remote git@github.com:itsHabib/workbench (confirmed from the local repository)
PR #329 OPEN, d82d65464de303abdebf15ece1095e78a10a83c1
Source baseline used by the guide ea76b6230210d42802cb2967938341090e73dd3d
Current fetched main 3c3ced644cc165a4192e92b310bdf23daf9bb851
PR #330 MERGED at 2026-09-12 17:40:47 UTC; reviewed branch head 6db1b0e65b74bb4aea6f00a3c7bf6d8812916887; merge commit is current main above
Issue #324 OPEN; awaits a Windows run after the executable-name fix, not an unconditional programmatic-hook implementation
Sandbox documents inspected itsHabib/fleet-demo-sandbox at 75c5ae9ef3fdb55a9f7eb2b07ef58cd441c26e55, run-7 contract and scorecard

#329 has three Claude attestations, including its current head. Copilot's three inline threads are resolved and outdated; its latest summary generated zero new comments. Codex review was unavailable through usage limits. Required-looking check entries returned success; that is neither independent factual completeness nor permission to merge. I read the reviews and checked the repaired locking/start-error passages rather than reopening them. The repository's two-fix-round cap has already been reached.

The shared checkout contained an unrelated change to cmd/triage/labels/mismatches.jsonl. All verification used an independent local clone and a worktree owned by that clone. Final remote observations retained the revisions above.

Ranked findings and exact corrections

The order below is practical importance. R1 is an existing reliability/design problem, not a runtime regression introduced by this documentation PR. The remaining findings distinguish #329 text from the wider documentation set. Their corrections are recommendations for the operator's read and a bounded follow-up, not an instruction to restart the exhausted panel.

R1 — High design priority: a future experiment should not require keeping a known pre-spawn failure in service

Exact text: “It is being kept unfixed as the first target of the planned crash-point test harness” — Fleet 101, section 11, lines 1152–1153. #330's body likewise says the starting gap is “deliberately untouched.”

The guide accurately identifies the problem. The direction is what I challenge. run publishes starting before constructing the provider command. A constructor error returns without changing it. Delivery returns the mail stamps, but every subsequent fold refuses this directory; cancel and release also refuse its unknown process state. #330 adds request-file creation errors to this path.

Independent reproduction: a review-only constructor fixture returned an error before spawning anything. On both #329 and current main, the record remained starting, the message became eligible again, the next fold deferred, and both recovery verbs refused. The local probe output is summarized here. This is deterministic mechanism evidence; it is not a new observed production incident or a measurement of incidence.

Better alternative: freeze the reproducer against the old SHA and make a focused pre-spawn repair. Prepare the command and its durable request before publishing starting; publish starting immediately before attempting Start. Verify that a constructor failure permits the next fold, preserving any previous completed attempt and returned mail. Keep genuine possibly-started uncertainty conservative. A failed durable publication must remain an error.

Proposed correction: “Known constructor failures occur before any process exists and should be recoverable in a focused fix. Preserve the old behavior as a regression fixture. Crashes spanning process start remain a separate, unresolved recovery boundary.”

Do not respond by teaching routine JSON deletion. The guide already explains that deletion can permit a second launch while an unrecorded bridge survives. Retained mail bytes also do not establish eventual delivery: stamped unacked mail needs a later session, and an indefinitely blocked address may never get one.

R2 — P2, #329: the opening ownership guarantee omits ordinary shell writes

Exact text: “it refuses the writes that would break ownership” — lines 85–87. The orientation explicitly warns about MCP writes, but not ordinary Bash file writes.

IsWrite recognizes named file tools plus selected Git/GitHub commands. printf x > file.txt, python3 script.py, and go generate ./... all return false. In a private-store probe with a live rival holding the branch, those Bash calls received no branch-admission refusal; an Edit control was refused. The local probe output is summarized here. This exercised the real admission function, not a real harness or actual shell write.

Section 5 lists the recognition rule, but leaves the reader to infer this consequential limitation. “The hook sees it” and “the branch lease checks it” are different questions. Lane resource requirements and provider permissions can still refuse a call for separate reasons.

Proposed correction: “Fleet keeps one recorded holder and gates recognized future tool admissions. Ordinary shell programs and redirections can write without a branch-lease check; tools outside the matcher bypass the hook entirely. It does not isolate files or stop already-running effects.” Put this beside the orientation's lease rule, with the printf example.

Do not try to make shell regexes into a security boundary. Keep separate worktrees as the baseline. A workload requiring effect isolation needs an enforced filesystem/resource boundary or process containment; adding more role permissions does not provide it.

R3 — P2, #329: refresh the teaching path for merged #330, not just its “open gap” heading

Exact text: “with the request on stdin” — delivery step 11, line 692; “A one-shot fold can lose the bridge's request (open; found while writing this guide)” — section 11.

The historical snapshot marker is honest. It does not make an obsolete default learning path useful after #330 merged. Current provider.Command creates a new private .request.json, and the bridge reads its path from argv. The request-file and one-shot regression tests passed locally at current main.

Proposed correction: “Fixed on main by #330 (3c3ced6). The watcher writes a private, exclusive request file before creating the bridge; argv contains its path and stdin is unused. The starting recovery gap remains, including request-file creation failure.”

Update the orientation, store table, delivery step 11, failure triggers, unmodeled-boundary list, section 11, self-test 13 and attempt glossary together. Retain the old stdin schedule as dated historical evidence in the reference. The statement that command construction “can fail only” on executable lookup or encoding is no longer true. Re-pin changed source citations after the update. #330's own body already requests this reconciliation; do not take over or reopen its fix.

R4 — P2, teaching direction: make the simple path primary and hierarchy optional in the actual examples

Exact text: “Read it top to bottom” — Fleet 101 lines 3–7. “Decide the tree” and “One overall lead. One lead per bucket of work” — ONBOARDING lines 14–17.

Fleet 101 is 1,331 lines. Its first substantive journey is through rules, storage and leases, followed by a 34-step delivery trace and formal methods. This is a valuable maintainer reference, but a costly first encounter. Onboarding's prose calls roles optional while its commands still seed multiple workers and bucket leads. The README's “workers talk to their lead, never to the operator” also presents one workflow as the universal shape.

The newer boundary spec and run-a-fleet.md already contain the better design: one named accountable lead, direct peer questions, editable cards, optional Org. The run-7 scorecard uses one lead, two workers and a verifier; it provides no evidence that a lead-of-leads is needed. Nor does its different build/contract justify a causal cost comparison with earlier runs.

Proposed correction: “Start with one task owner in one worktree and one accountable lead. Add a separate verifier when acceptance requires independent checks. Add a second worker, resource lease or second lead only when the work requires it.”

Split the reading paths rather than duplicating their content. Keep a short Fleet 101 with one task, one question, one handoff, one exact-head verification and one failure example. Move or relabel the existing detailed sections as Fleet runtime and model reference. Reuse install.md and run-a-fleet.md for commands. Keep the 34-step trace and drift log available by link. Org and parent links should not be prerequisites in the starter example.

R5 — P2, #329: distinguish runtime observations from an agent's evidence claim at the point of introduction

Exact text: “Facts come from hooks, never from an agent” — line 122; “Each is enforced somewhere specific” — lines 106–107; “Done is evidence” — lines 142–145.

The detailed receipt section correctly says independence is not checked. The opening still invites the inference that Fleet establishes that a task really passed. receiptArgs checks verdict/ID shape and nonempty observable text; the surrounding command checks live session, head and clean tree. It does not execute the check, authenticate its conclusion, or establish the verifier's independence. An agent-authored receipt changes the derived row to done. Authored handoffs and messages remain necessary too.

Proposed correction: “Runtime activity is observed from hooks and process evidence. Assignments, handoffs and receipts are authored records. done means the latest required receipt says pass at this head; the verifier and lead establish whether that receipt supports acceptance.”

Keep receipts lightweight and authority-free. Do not repair the wording by adding an Org permission chain or a mandatory second receipt product. The same distinction belongs in the one-screen orientation; readers are expressly told they can stop there.

R6 — P2, #329: “parent” conflates Org discovery with Fleet lateness routing

Exact text: an overdue question goes “to the addressee's parent” — lines 560–566. Earlier the guide introduces Org's optional parent reference, but never mentions LATE_TO.

parentOf chooses the accountable for field of the newest matching seat row, otherwise LATE_TO from the delivery entry. It does not read Org. With neither, it logs late-no-recipient. A reader registering an Org parent would not thereby configure escalation delivery.

Proposed correction: “An overdue unacknowledged question is reported to the seat's latest accountable --for role, or the address's configured LATE_TO. Org's parent field is not consulted. With no recipient, the watcher records late-no-recipient.” Call this the lateness recipient, preserving parent as descriptive prose rather than creating another hierarchy mechanism. Acknowledgement measures receipt, not whether the question was answered.

R7 — P2, #329: self-test 7 teaches the wrong release condition

Exact text: “The bridge exited 0 but provider_terminal is false. Is the directory free? (No. A bridge exit alone never releases it; watch status shows provider_cleanup_pending.)” — lines 1250–1251.

The answer does not follow from the premise. providerTerminal also accepts matching never-started evidence or the bounded Codex quiescence proof. The guide explains both in section 8, then tests the opposite simplification. An isolated state-file probe confirms exit zero plus provider_started:false, provider_terminal:false frees the directory on both reviewed versions.

Proposed answer: “Not enough information. Exit alone is insufficient; matching never-started evidence or the supported quiescence proof can also free it. Otherwise it stays reserved.” Alternatively add “and neither other release proof exists” to the question. This is a factual correction, not a demand to change release behavior.

R8 — P2, documentation set: platform qualification statements lag the evidence, and #324's current next step is misstated

Exact text: “Everything measured so far was measured on a Mac with Claude” — ONBOARDING line 5; “nothing so far is Windows or Codex acceptance” — e2e line 124; “#324, the programmatic-hook follow-up” — Fleet 101 line 1223.

provider-runtime-validation.md records completed bounded Codex and Claude workflows, including frozen-build PRs #18/#19, along with explicit final-head limitations and intervention counts. Those do not establish Windows readiness, but they invalidate the blanket no-Codex wording. The larger multi-lead/resource Codex variant can remain explicitly unqualified.

Issue #324's current update reports that macOS SDK-launched settings hooks do fire, including SessionEnd. It points to the extensionless Windows binary as a sufficient explanation and requests a Windows rerun using fleet.exe. It warns unconditional programmatic callbacks would double-fire on macOS. The run-7 follow-up reinforces this.

Proposed correction: “Bounded macOS workflows have been exercised with both providers; this larger scenario and Windows headless behavior require their own evidence. #324 awaits a Windows rerun after #325. Additional hook registration is conditional on that result.” Link the existing evidence ledger instead of duplicating a timeless qualification claim. Do not implement the obsolete patch from the issue body.

What the code and evidence support

Proceed with the role boundary. Org's actual cards.go/cards_cli.go implements three operations over a file-reference registry. Boot reads the Markdown afresh; a parent is stored/displayed, not traversed for authorization. Fleet resolves directory bindings separately. This is appropriately small. Keep role card changes as prose edits, not schema migrations, incarnations or enrollment ceremonies.

Proceed with provider-neutral coordination, with bounded adapter claims. Go owns scheduling/reservations; the embedded bridge translates Claude SDK or Codex app-server lifecycle. Neither adapter should own another scheduler, mailbox or organizational policy. Protocol fixtures and recorded workflows establish useful mechanism behavior. They do not establish equal operating cost, identical platform guarantees, or effortless headless setup. Codex hook trust, Claude settings, and the macOS-only process observer remain explicit adapter differences.

Keep leads useful rather than hierarchical. Their job is to answer routine scope/fact questions, reconcile evidence, unblock actual contention and retain accountability. Peers can answer one another directly. Decisions outside their scope still belong with the operator. A child-to-parent-only route or an extra lead for every repository would add delays without demonstrated benefit.

Hooks are admission guards; OS observation is narrower than OS isolation. The existing no-fork observer is a defensible bounded cleanup mechanism: it arms before exec and requires matching attempt evidence. It does not contain descendants, and any fork disqualifies the no-fork proof. Normal provider-terminal evidence is not an all-descendant quiescence theorem. Keep the existing conservative resource rule. Choose a process-scope/fencing experiment only for a concrete workload with surviving effects; do not start a generic OS or sandbox project from this review.

The formal work is useful, but its strongest contribution is a counterexample. I independently ran judge.sh successfully, including the real-process trace: branch effects B\nA\n, resource effects A\n. TLC with --max-steps=1 still explored all 325 ReferenceBranch states, depth 12, queue empty. Removing only the silent-takeover label from the mutant passed all four invariants over 349 states. The guide is right about both points.

The models assume atomic admission and omit the watcher launch protocol, arbitrary shell writes, real hooks, restart/PID reuse, multi-key behavior and filesystem failures. QuiescentBranch's guard is an oracle. No safety-model result proves implementation refinement or eventual progress. Process-kill tests also do not qualify power-loss durability of multi-file state. Keep these scope boundaries with the model output.

A later repair of noSilentResourceTakeover should check the ownership transition and authorized takeover event, with a mutant that changes behavior without self-labeling the violation. That can stay a small model change. It is less urgent than deterministic runtime recovery. Nor should a valid alternative SMT counterexample become a release failure merely because its bytes differ; use a fixed witness for a frozen replay and a separate unrestricted search where appropriate.

Smallest valuable next step and experiment

Next reliability change: the pre-spawn constructor-failure repair in R1, with one regression that fails on current main and passes after the change. Keep one adjacent control for a possibly-started attempt remaining reserved. This needs neither a generic crash injection framework nor a new launch state machine. The proposed fresh-agent kickoff appended below names the exact starting head, checks and exclusions.

Next teaching change: after the operator reads #329, promote one short workflow and preserve the long text as its linked reference. Make R2/R3/R5–R8 concrete residual corrections, not another broad review exercise.

Next usefulness trial, when separately authorized: use a real small repository bug with one owner and one independent verifier. Compare the existing desktop/native coordination baseline with Fleet mail plus watcher on matched tasks and acceptance. Record operator rescues, coordination turns/cost, idle assigned time, preserved dirty work and time to an independently checked draft. Use both providers only if parity is the question being tested.

Run 7 is positive coordination evidence, but its actual deliverable is a four-line Markdown file with prescribed unit questions and a bench script. It is not evidence of better software delivery. Its reported 16 turns, $3.44 and zero rescues are scorecard observations, not independently recomputed raw telemetry in this review. The exact frozen runtime SHA is also not supplied by “3c04b8b plus #326.”

For the next small trial, require no lost work, no unsafe overlap and no manual JSON repair. Predeclare the maximum spend and compare completed acceptance, not token counts alone. If Fleet does not reduce operator intervention or meaningful waiting enough to justify its setup and coordination cost, retain the desktop baseline and stop expanding the runtime. Add neither hierarchy nor formal machinery to rescue a failed usefulness hypothesis.

Verification record and limitations

At pristine #329 head:

  • go test -race -count=1 ./cmd/fleet/...: all eight packages passed.
  • gofmt -l cmd/fleet cmd/org: empty; go vet ./cmd/fleet/... ./cmd/org/...: passed; golangci-lint run ./cmd/fleet/... ./cmd/org/...: zero issues; Org tests passed.
  • Both bash cmd/fleet/testdata/run-suite.sh variants: all scenarios passed. They use fixtures and disable automatic watchers/GitHub writes; these are not paid-provider acceptance runs.
  • (cd cmd/fleet/model && ./judge.sh): all checks passed, including four frozen counterexamples and the real-process replay.
  • git diff --check: passed.

At pristine current main: go test -race -count=1 ./cmd/fleet/internal/provider ./cmd/fleet/internal/watch passed, including #330's one-shot/request-file checks. Review-only probes were then added in scratch trees and passed separately; they are not changes to the reviewed commit. They exercised shell admission, the pre-spawn stuck state, never-started release, and stale unverified presence. The latter three were repeated on current main.

Tools: Go 1.26.5, darwin/arm64; Node v26.5.1; Quint 0.32.0; Codex CLI 0.153.4. Logs and probe source are retained in the originating Codex task; local-only paths are not presented as GitHub-accessible evidence. The two review-only path mistakes were corrected before the recorded model/probe runs; no failing substantive check was discarded or converted to a pass. Full repository-wide tests, live provider calls, Windows execution, raw private rehearsal telemetry and real-harness shell admission were not rerun. Existing CI and rehearsal observations are identified as retrieved evidence above.

Read scope: repository AGENTS/CLAUDE and design charter; the complete #329 guide and diff; Fleet README, install, overview, minimum, onboarding, run-a-fleet, headless, e2e and provider validation guides; Org guide and actual card implementation; the full Org/Fleet boundary spec; relevant followups; Go admission/lease/session/role/receipt/mail/delivery/recovery code; the complete Node bridge and macOS observer; model ledgers, source map, reference/crash models, mutants and process replay; both PRs' reviews and #324 updates; run-7 contract and scorecard. Citations point to the examined revisions, not an assumption that the shared installed Fleet binary matches them.

Proposed fresh-agent kickoff (separate implementation scope)

Proposed fresh-agent kickoff: Fleet pre-spawn recovery

This is a proposed task for the operator to adopt after reading the review. The current review did not authorize implementation, remove #329's hold, or supersede the operator's earlier choice to retain the gap. Do not execute this packet merely because it exists.

If adopted, use this prompt:

Implement the smallest safe repair for Fleet command-construction failures that strand a directory in starting. Workbench remote is git@github.com:itsHabib/workbench; last reviewed main is 3c3ced644cc165a4192e92b310bdf23daf9bb851, containing merged #330. Refresh main and existing work ownership first, read root AGENTS.md/CLAUDE.md and docs/DESIGN.md, and use your own isolated worktree. Do not change a shared or live Fleet home.

Read cmd/fleet/internal/watch/runtime.go, deliver.go, status.go, runtime_test.go, and cmd/fleet/internal/provider/provider.go. At the reviewed head, run writes the launch record as starting, then calls providerCommand. If that call errors, no process exists but the record stays starting. Delivery returns the mail stamps; the next fold defers forever, and cancel/release refuse pid 0 as unknown. A local probe demonstrated this on both examined revisions; at current main the in-package injected constructor takes (string, map[string]any) and returns (*exec.Cmd, error).

First preserve a deterministic regression using the existing in-package constructor fixture. Prefer preparing the command and private request before publishing starting, then publishing starting immediately before Start. This separates a known no-spawn error from a potentially-started attempt without inventing another protocol. Preserve the previous launch/resume identity if preparation fails. Keep the existing exclusive/private request-file contract from #330.

Acceptance: (1) an injected constructor failure leaves no new unresolved starting reservation, and the same returned mail can start exactly one later attempt when construction succeeds; (2) a prior completed attempt and its resume target survive preparation failure; (3) if an attempt may have started but has no conclusive process/provider evidence, another fold still refuses a competing launch; (4) no manual record deletion, false exit record, timeout-as-death or mail replay after a successful start is introduced. Assert these against real store behavior; no paid provider is needed. Preserve the old failing result before the fix and show that it discriminates the new behavior.

Run focused tests, go test -race ./cmd/fleet/..., Fleet build/vet/lint and both documented CLI/adapter suites. Run model regeneration only if model files change; none should be needed. Document exactly which failure class became recoverable and which remains unknown. Update the relevant headless recovery text in the same proposed change, with a pointer for the separate Fleet 101 refresh.

Scope exclusions: no fix takeover of #330 or #324; no unconditional programmatic SDK hooks; no new scheduler, generic chaos framework, Org hierarchy, lease-epoch scheme, process supervisor, OS isolation project or broad state migration. Do not change stale-session policy in this slice. Do not launch real providers, install/replace live binaries, create timers, modify Gate state, merge or deploy. Return a locally tested, reviewable diff and evidence; publication requires the operator's requested delivery scope. #329 remains held for the operator's own read, and its exhausted panel is not to be retriggered.

Separate follow-up only after this slice: characterize the genuine watcher-death window before and after process start, keeping an ambiguous surviving provider reserved. Extend recovery only when the implementation can distinguish no process from possibly surviving effects. A generic harness is justified only if repeated concrete cases need the shared mechanism.

@itsHabib

Copy link
Copy Markdown
Owner Author

Workbench queue ownership is now tracked in the local Fleet improvement task. Preserving the operator-read/no-Gate/no-merge hold at head d82d65464de303abdebf15ece1095e78a10a83c1 and the two-fix-round cap. Both independent SIMPLIFY reviews are part of the disposition: retain the forensic reference, lead with a short task-first introduction, and reconcile concrete residual claims when the operator chooses the teaching direction. CI is green; no new review cycle or merge is being initiated.

@itsHabib

Copy link
Copy Markdown
Owner Author

The operator requested concrete teaching improvements and left the Workbench/Fleet document structure to us. Follow-up #335 now reconciles both independent SIMPLIFY reviews at 4350bbedf13f2b383c651c9949359a1cb50f4d78.

Workbench 101 is the short entry/reading map; Fleet 101 stays a focused linked chapter beside install/run guides. This PR's detailed text is retained as a dated runtime/model reference. The new path covers one assignment, a peer question, handoff and exact-head completion, and corrects the shell-write boundary, authored receipt claims, lateness routing, historical transport and qualification wording. A separate independent verifier passed the final documentation head; validation and per-finding dispositions are in #335.

This replaces the proposed first-reader structure, not your review history. #329 stays open and held for operator read; no merge, closure, Gate call or fourth reviewer-panel cycle is being performed. Runtime defects remain with the existing Fleet reliability owner.

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

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants