feat: register external workspace routes - #13
Merged
Merged
Conversation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
codeant-ai
Bot
added
the
size:XXL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| Scaffold the task with matching `--workspace` and `--member` arguments to `bin/fm-brief.sh`, then spawn it with the same arguments to `bin/fm-spawn.sh`. | ||
| The brief receives the validated instruction snapshot, and spawn resolves only the selected member Git root into the existing isolated-worktree lifecycle. | ||
| The outer root is never a task cwd, primary checkout, or worktree source. | ||
| The registry and route are backend-neutral: every supported primary harness receives the same brief, tmux, Herdr, Zellij, and cmux continue through Treehouse, and Orca continues through its own isolated worktree provider. |
There was a problem hiding this comment.
Suggestion: Replace the enumerated backend list with a stable reference to the backend-neutral spawn contract and its owning documentation, so adding or removing a backend does not require updating this skill. [custom_rule]
Severity Level: Major
Why it matters? ⭐
This policy embeds a concrete list of harnesses and provider routing details despite stating that the contract is backend-neutral. Adding or changing a backend would require editing this skill, creating avoidable coupling and documentation drift; a stable reference to the owning contract would improve maintainability.
Rule source 📖
CodeAnt dashboard (rule "agent-persona-maintainability")
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** .agents/skills/project-management/SKILL.md
**Line:** 57:57
**Comment:**
*Custom Rule: Replace the enumerated backend list with a stable reference to the backend-neutral spawn contract and its owning documentation, so adding or removing a backend does not require updating this skill.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| 6. **Network checks** - after the fleet-state digest, the deferred stage's result, or an explicit statement of what it has not confirmed yet. | ||
| A read-only session runs no network checks at all and says so. | ||
| 7. **Context digest and next step** - last of the bulk sections, the full contents of `data/projects.md`, `data/secondmates.md`, `data/captain.md`, `data/captain-shared.md`, and `data/learnings.md`, each clearly delimited, followed by the closing reminder. | ||
| 7. **Context digest and next step** - last of the bulk sections, the full contents of `data/projects.md`, a validated summary of `data/workspaces/`, and the full contents of `data/secondmates.md`, `data/captain.md`, `data/captain-shared.md`, and `data/learnings.md`, each clearly delimited, followed by the closing reminder. |
There was a problem hiding this comment.
Suggestion: Replace the requirement to emit the full contents of every context file with bounded summaries or capped excerpts, retaining a targeted-read path for complete contents when needed. [custom_rule]
Severity Level: Major
Why it matters? ⭐
Captain, this requires every session-start digest to load and emit several potentially growing private files in full, including learnings and captain preferences. That creates avoidable repeated I/O and payload or memory growth on a commonly executed startup path; bounded summaries with targeted full reads would preserve correctness while keeping the digest fast and bounded.
Rule source 📖
CodeAnt dashboard (rule "agent-persona-performance")
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** AGENTS.md
**Line:** 179:179
**Comment:**
*Custom Rule: Replace the requirement to emit the full contents of every context file with bounded summaries or capped excerpts, retaining a targeted-read path for complete contents when needed.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| printf 'ABSENT\n' | ||
| return 0 | ||
| fi | ||
| if out=$("$SCRIPT_DIR/fm-workspace.sh" list 2>&1); then |
There was a problem hiding this comment.
Suggestion: Refactor the workspace-list command so registry validation and record loading occur only once per session-start invocation; the current list implementation validates and hashes every record and then reloads every record to render it, doubling filesystem scans, parsing, and instruction-file hashing on this blocking hot path. [custom_rule]
Severity Level: Major
Why it matters? ⭐
The invoked list implementation first runs validate_registry, which loads and validates every workspace record, including hashing instruction files, and then loads every record again while rendering the listing. This duplicates filesystem scans, parsing, and instruction-file hashing on the blocking session-start path, so the performance concern is real for registries with multiple workspaces.
Rule source 📖
CodeAnt dashboard (rule "agent-persona-performance")
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** bin/fm-session-start.sh
**Line:** 390:390
**Comment:**
*Custom Rule: Refactor the workspace-list command so registry validation and record loading occur only once per session-start invocation; the current `list` implementation validates and hashes every record and then reloads every record to render it, doubling filesystem scans, parsing, and instruction-file hashing on this blocking hot path.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+568
to
+587
| while IFS= read -r target; do | ||
| [ -n "$target" ] || continue | ||
| load_record "$target" || exit 1 | ||
| [ "$REC_ROOT" != "$root_real" ] || die "workspace root is already registered by '$REC_ID': $root_real" | ||
| idx=0 | ||
| while [ "$idx" -lt "${#REC_MEMBER_PATHS[@]}" ]; do | ||
| other_idx=0 | ||
| while [ "$other_idx" -lt "${#member_specs[@]}" ]; do | ||
| spec=${member_specs[$other_idx]} | ||
| member_path=${spec#*=} | ||
| member_real=$(canonical_directory "$member_path") || die "member path drifted during registration: $member_path" | ||
| [ "${REC_MEMBER_PATHS[$idx]}" != "$member_real" ] \ | ||
| || die "member path is already registered by '$REC_ID/${REC_MEMBER_IDS[$idx]}': $member_real" | ||
| other_idx=$((other_idx + 1)) | ||
| done | ||
| idx=$((idx + 1)) | ||
| done | ||
| done <<EOF | ||
| $(registry_files) | ||
| EOF |
There was a problem hiding this comment.
Suggestion: Canonicalize the proposed member paths once before entering the cross-record ownership check and compare those cached values, rather than repeating canonicalization inside every existing-member comparison. [custom_rule]
Severity Level: Major
Why it matters? ⭐
The candidate member paths were already canonicalized and stored in REC_MEMBER_PATHS during the earlier input-validation loop, but this nested loop calls canonical_directory again for every candidate member for every existing record member comparison. The repeated filesystem-based canonicalization is avoidable work that grows with the number of registry records and members, so caching the canonical paths is a valid medium-impact performance improvement.
Rule source 📖
CodeAnt dashboard (rule "agent-persona-performance")
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** bin/fm-workspace.sh
**Line:** 568:587
**Comment:**
*Custom Rule: Canonicalize the proposed member paths once before entering the cross-record ownership check and compare those cached values, rather than repeating canonicalization inside every existing-member comparison.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+589
to
+616
| # Restore the new record values after inspecting existing records above. | ||
| REC_ID=$id | ||
| REC_ROOT=$root_real | ||
| REC_SCOPE=$scope | ||
| REC_INSTRUCTION_PATHS=() | ||
| REC_INSTRUCTION_HASHES=() | ||
| idx=0 | ||
| while [ "$idx" -lt "${#instruction_inputs[@]}" ]; do | ||
| instruction_real=$(canonical_directory "${instruction_inputs[$idx]}") || die "instruction root drifted during registration" | ||
| instruction_hash=$(sha256_file "$instruction_real/AGENTS.md") || die "cannot hash instruction file" | ||
| REC_INSTRUCTION_PATHS+=("$instruction_real") | ||
| REC_INSTRUCTION_HASHES+=("$instruction_hash") | ||
| idx=$((idx + 1)) | ||
| done | ||
| REC_MEMBER_IDS=() | ||
| REC_MEMBER_RELATIONS=() | ||
| REC_MEMBER_PATHS=() | ||
| idx=0 | ||
| while [ "$idx" -lt "${#member_specs[@]}" ]; do | ||
| spec=${member_specs[$idx]} | ||
| member_id=${spec%%=*} | ||
| member_path=${spec#*=} | ||
| member_real=$(canonical_directory "$member_path") || die "member path drifted during registration" | ||
| REC_MEMBER_IDS+=("$member_id") | ||
| REC_MEMBER_RELATIONS+=("${member_relations[$idx]}") | ||
| REC_MEMBER_PATHS+=("$member_real") | ||
| idx=$((idx + 1)) | ||
| done |
There was a problem hiding this comment.
Suggestion: Keep the candidate record in local variables or pass it through a dedicated helper while inspecting existing records, instead of restoring all global record arrays and recomputing every path and hash afterward. [custom_rule]
Severity Level: Major
Why it matters? ⭐
command_add uses global REC_* arrays both for the candidate being built and for records loaded during the ownership scan. Because load_record overwrites those globals, the candidate must be reconstructed afterward, including repeated canonical_directory and SHA-256 operations. This state coupling makes the function harder to read and change safely; a local candidate representation or dedicated helper would remove the implicit save-and-restore protocol while preserving behavior.
Rule source 📖
CodeAnt dashboard (rule "agent-persona-maintainability")
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** bin/fm-workspace.sh
**Line:** 589:616
**Comment:**
*Custom Rule: Keep the candidate record in local variables or pass it through a dedicated helper while inspecting existing records, instead of restoring all global record arrays and recomputing every path and hash afterward.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+647
to
+650
| validate_registry || exit 1 | ||
| while IFS= read -r file; do | ||
| [ -n "$file" ] || continue | ||
| load_record "$file" || exit 1 |
There was a problem hiding this comment.
Suggestion: Reuse the records already loaded during registry validation instead of calling load_record again for every file; retain validated metadata or combine validation and output in one pass. [custom_rule]
Severity Level: Major
Why it matters? ⭐
The list command first traverses and fully loads every record through validate_registry, then traverses and fully loads every record again before printing it. This duplicates file parsing, path validation, Git-root checks, and instruction hashing on a commonly used listing path, creating measurable extra I/O and computation as the registry grows. Reusing validated metadata or combining validation and output into one pass addresses the performance rule.
Rule source 📖
CodeAnt dashboard (rule "agent-persona-performance")
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** bin/fm-workspace.sh
**Line:** 647:650
**Comment:**
*Custom Rule: Reuse the records already loaded during registry validation instead of calling `load_record` again for every file; retain validated metadata or combine validation and output in one pass.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| # Task | ||
| {TASK} | ||
|
|
||
| $WORKSPACE_SECTION |
There was a problem hiding this comment.
Suggestion: The inserted external instruction context is copied into the same brief that fm-spawn.sh scans globally for Workspace route: and Workspace repository: lines. A registered AGENTS.md can legitimately contain either of those strings, causing the spawn-time count to exceed one and rejecting an otherwise valid workspace task. Delimit the generated metadata in a parser-safe section or make the spawn parser read only that section. [api mismatch]
Severity Level: Major ⚠️
- ❌ Valid external workspace tasks fail before backend launch.
- ⚠️ Workspace instruction wording can invalidate routing metadata.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** bin/fm-brief.sh
**Line:** 350:350
**Comment:**
*Api Mismatch: The inserted external instruction context is copied into the same brief that `fm-spawn.sh` scans globally for `Workspace route:` and `Workspace repository:` lines. A registered `AGENTS.md` can legitimately contain either of those strings, causing the spawn-time count to exceed one and rejecting an otherwise valid workspace task. Delimit the generated metadata in a parser-safe section or make the spawn parser read only that section.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| # Task | ||
| {TASK} | ||
|
|
||
| $WORKSPACE_SECTION |
There was a problem hiding this comment.
Suggestion: The workspace context is emitted before the ship definition-of-done section, while fm-spawn.sh obtains BRIEF_MODE by selecting the first matching Delivery contract: mode= line in the entire brief. If any registered outer AGENTS.md contains such a line, the outer instruction's value becomes the task's parsed mode and the spawn can reject the task or apply the wrong delivery-posture check. Use uniquely delimited metadata or scope the parser to the generated definition-of-done block. [api mismatch]
Severity Level: Major ⚠️
- ❌ Workspace ship tasks can be rejected before backend launch.
- ⚠️ Outer instructions influence delivery-contract validation.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** bin/fm-brief.sh
**Line:** 463:463
**Comment:**
*Api Mismatch: The workspace context is emitted before the ship definition-of-done section, while `fm-spawn.sh` obtains `BRIEF_MODE` by selecting the first matching `Delivery contract: mode=` line in the entire brief. If any registered outer `AGENTS.md` contains such a line, the outer instruction's value becomes the task's parsed mode and the spawn can reject the task or apply the wrong delivery-posture check. Use uniquely delimited metadata or scope the parser to the generated definition-of-done block.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+146
to
+149
| if ! mkdir -- "$LOCK" 2>/dev/null; then | ||
| die "external workspace registry is locked by another operation: $LOCK" | ||
| fi | ||
| LOCK_HELD=1 |
There was a problem hiding this comment.
Suggestion: The lock is represented only by a directory and is removed solely by the EXIT trap, so termination by SIGKILL, power loss, or another abrupt process failure leaves "$LOCK" permanently present. Because every command, including remove/unregister, must acquire this lock, the entire registry becomes unusable until an operator manually deletes the lock directory, defeating the intended recovery path. [missing cleanup]
Severity Level: Major ⚠️
- ❌ Abandoned locks block workspace registration and removal.
- ⚠️ Manual filesystem cleanup is required for recovery.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** bin/fm-workspace.sh
**Line:** 146:149
**Comment:**
*Missing Cleanup: The lock is represented only by a directory and is removed solely by the EXIT trap, so termination by SIGKILL, power loss, or another abrupt process failure leaves `"$LOCK"` permanently present. Because every command, including `remove`/`unregister`, must acquire this lock, the entire registry becomes unusable until an operator manually deletes the lock directory, defeating the intended recovery path.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Vinosaamaa
added a commit
that referenced
this pull request
Aug 25, 2026
Co-authored-by: wenkxu <v-wenkxu@expediagroup.com>
Vinosaamaa
added a commit
that referenced
this pull request
Aug 25, 2026
* fix(composer): stop a blocked pi pane from proving an empty composer (kunchenguid#2811) A pi worker parked on an interactive prompt - a permission dialog, a question menu, a trust dialog - reports agent_status=blocked, because it is waiting on a human keystroke. Pi draws that menu above its separator pair, so the composer region between the rules is blank and structure alone looks like a free composer. _fm_composer_pi_verdict admitted blocked alongside idle and done, so the shared classifier reported an affirmatively empty composer for exactly the pane where typing is unsafe. Every "is it safe to type here?" consumer reads that verdict and proceeds only on an affirmative empty, so both are told yes on a parked prompt: the away-mode injection guard in bin/fm-supervise-daemon.sh, and fm-send's pre-type refusal. The keys then answer the menu instead of composing a message - the highlighted default is selected, the text is discarded, and the record attributes a decision to a human who never made it. blocked now defers to unknown, which every consumer already treats as fail-closed. idle and done still prove an empty composer, so ordinary steering is unchanged, and Cursor is unaffected because its always-blocked panes never reach this pi-only branch. Regression coverage lands first at both levels: the verdict owner (a blocked pi defers) and the herdr adapter (a parked pi prompt is not an empty composer). * fix(bin): require project clone roots during fleet sync (kunchenguid#2849) * fix(bin): require a clone root before fleet-sync touches a project Git repository discovery walks upward, so `git -C projects/<dir>` on a plain directory nested under projects/ resolves to the enclosing repository - in a firstmate home, the firstmate checkout itself. fm-fleet-sync.sh guarded its candidates with `rev-parse --is-inside-work-tree`, which such a directory passes, so every later git call read, pruned and fast-forwarded firstmate's own default branch and reported it under the project directory's label. A running session's AGENTS.md changed underneath it, and the report named a project that had nothing to do with the change. Require each candidate to be the root of its own work tree before any other git command: compare `rev-parse --show-toplevel` against the directory's own physical path. Both sides are physical, so a symlinked clone still compares equal. Anything else is skipped by name, naming the repository that would have been touched, and bootstrap relays that as a FLEET_SYNC line. Regression coverage reproduces the wrong-repo fast-forward against a home nested inside another repository, in both the whole-fleet and single-project forms, and pins that a symlinked clone dir still syncs. * no-mistakes(review): Keep enclosing fixture clean during clone-root regression * fix(bin): retry transient Lavish poll interruptions (kunchenguid#2846) * fix(procevent): retry a transient Lavish poll interruption quietly A live Lavish listener can be cut short by the server with exactly error: Lavish Editor poll response was interrupted code: SERVER_ERROR while the session's marks remain available. Firstmate registered raw `lavish-axi poll` output, so the generic process-event runner captured that transient response as a result and woke the whole fleet over what is really an internal retry. The Lavish adapter now registers its own listener command, which reruns the published blocking poll up to 12 times at 5 second intervals for that one exact two-line response. The match is deliberately narrow: real feedback, ended and missing sessions, any other SERVER_ERROR, and the same interruption still standing once the bound is spent all pass straight through and are captured and announced as before. The retry is a Lavish fact, so the generic runner stays adapter-agnostic. `FM_LAVISH_POLL_RETRY_DELAY` is a bounded 0 to 60 second override for the interval only, refused rather than rounded when malformed, so a test can exercise the real bound without waiting it out. * no-mistakes(review): Harden Lavish retry matching, validation, and cleanup * no-mistakes(review): Bound Lavish retry staging and stabilize regression * no-mistakes(document): docs: explain Lavish retry adoption * no-mistakes(lint): Restore Lavish trap ShellCheck suppression * fix(brief): stop the documented {TASK} fill from corrupting the Herdr gate (kunchenguid#2838) The unguarded Herdr declaration quoted `{TASK}` in its own prose while the scaffold instructs firstmate to replace every `{TASK}` placeholder. The documented global replace therefore spliced the whole task body into the middle of the safety gate's sentence, silently destroying the one contract that exists precisely because the scaffold cannot inspect the task text. Reword the gate to refer to the task text filled in above, leaving the placeholder only at its genuine fill site. Rewording rather than renaming the token keeps the unfilled-charter guards in fm-home-seed.sh and fm-remote-home-seed.sh working unchanged. Add a regression test that performs the documented global fill on ship and scout scaffolds and asserts the body lands once and the gate survives. * fix(bin): resolve the busy-state lock mtime with the platform's own stat form (kunchenguid#2837) The writer lock's stale-lock branch read the lock's mtime with `stat -f %m ... || stat -c %Y ...`. On GNU coreutils `-f` is filesystem stat, so it consumed the format string as a path, complained on stderr, printed a partial filesystem dump (" File: ...") on stdout, and still exited 0. The GNU form in the fallback therefore never ran, and the following arithmetic evaluated the word `File`, aborting the writer under `set -u` with "File: unbound variable". fm-teardown.sh died there after returning the worktree, leaving state/<id>.meta, .status, .busy-gen, .busy-state, .busy-state.lock/ and .turn-ended behind. The surviving metadata kept the watcher monitoring an endpoint whose agent was gone, so a finished task produced stale wakes forever, and every re-run died identically because the abandoned lock was never broken. Detect the platform once and pick the right stat form, the pattern bin/fm-watch.sh already documents, and treat any non-numeric result as "just created" so a future portability surprise degrades to a lock-timeout refusal rather than killing teardown mid-way. * fix(stow): add opt-in pass horizon for memory decay (kunchenguid#2850) * fix(stow): give memory decay a per-pass horizon so the clock fires The tiered decay clocks were wall-clock only, while admission is per-pass: each /stow admits the findings that pass produced. In a home that stows daily those two rates diverge by the stow cadence, an entry the fleet keeps exercising never reaches 30 days unreinforced, and memory only grows while the pass reports decay evaluated. Give each dated marker an optional unreinforced-pass counter and make both tiers stale at whichever horizon comes first: 10 passes or 30 days for aging, 3 passes or 7 days for perishable. Reinforcement clears the counter and nothing else does, so the existing evidence-based restamp rule stays the only way an entry renews its lease. An absent /N means zero, so entries that stay exercised carry no extra marker bytes, and a rarely stowed home keeps its current behaviour through the unchanged date horizon. * no-mistakes(document): Align stow workflow with dual decay clocks * fix(stow): make the per-pass decay horizon opt-in The unreinforced-pass horizon shipped as a new default archival cadence, which is a product default rather than a restoration of the existing wall-clock contract. Keep the 30-day and 7-day horizons as the only default clock, and put the 10-pass and 3-pass horizons behind an explicit opt-in: config/stow-pass-horizon for the firstmate home, and the file's own header pointer for the public skill. With the opt-in absent no counter is written and no counter is read, so a home that does not ask for it decays exactly as it does today. * no-mistakes(review): Preserve frozen counters and correct archive provenance * test(watcher): stop fixture confirmation budgets racing real child startup (kunchenguid#2876) tests/fm-watcher-lock.test.sh passed in isolation but failed intermittently under full-suite and ambient concurrent load. bin/fm-watch-arm.sh computes its confirmation deadline immediately after forking the real child watcher, so the child's entire fork, exec, lock acquisition and beacon publication has to land inside that wall clock. Two cases shrank that budget to one second, leaving a two-second window for work measured at 3.1-4.9s under CPU oversubscription, so the arm honestly reported "FAILED - no live watcher with a fresh beacon" and their premises collapsed. A third case ran on the production budget, but its child must also execute a registered check before exiting: measured at 1.9-2.3s idle and 9.1-13.1s under load, against an 11s budget. The two cases that must confirm a real child now hold the arm to production's own budget instead of a shrunken fixture one, the immediate-wake case gets an explicit budget with headroom over its measured loaded cost, and the two waits for the arm's typed failure are sized off the largest production default rather than a fixed eight seconds. No bin/ change and no default behavior change: the lock's fail-closed semantics, SIGSTOP handling, stale-heartbeat detection and the arm's typed failures are untouched. Verified 4/4 green at 3x CPU oversubscription (loadavg 75-80) after 3/3 red before the change, and CONTRIBUTING.md records the convention. * fix(bin): deterministically order remote tool paths (kunchenguid#2870) * fix(bin): order discovered tool installs by the shell's own expansion fm_remote_job_compose_operator_path built the asdf and mise install directories with `compgen -G`, which does not sort. Bash sorts glob matches in pathexp.c, on the shell's own pathname-expansion path only; `compgen -G` reaches the same glob_filename through pcomplete.c, which sorts nothing. On bash 3.2 (macOS /bin/bash) and every bash before 5.3 that handed the composition raw readdir order, so which install of a multi-version tool a remote job resolved was decided by directory order on disk rather than by this composition. Expand the globs at the call sites and let the function take the matches, so the composition and the documented portable-PATH contract are the same operation. Quoting the account home at the call site also stops a home whose name contains glob metacharacters from being reinterpreted. The colocated regression pins both the order and the mechanism: bash 5.3 moved sorting into the glob library, so an order-only assertion cannot see the defect there. * no-mistakes(review): Remove source-reading PATH regression guard * fix(bin): prevent routed secondmate work from stranding (kunchenguid#2848) * fix: surface stalled secondmate queues and wake handoffs * no-mistakes(review): Make handoff wakes retryable and stall alerts crash-safe * no-mistakes(review): Prevent duplicate handoff wakes and cover remote delivery * no-mistakes(review): Serialize local handoffs and preserve pre-move wake intent * no-mistakes(review): Serialize teardown with handoffs and retain remote wake confirmation * no-mistakes(review): Reconcile correlated handoff wake delivery after crashes * no-mistakes(review): Keep failed wakes retryable and isolate stall receipts * no-mistakes(review): Reset known-undelivered wake attempts for durable retries * no-mistakes(review): Refuse duplicate sends for unresolved delivery attempts * no-mistakes(review): Atomically restore retryability after reconciled send failures * no-mistakes(review): Serialize delivery confirmation with reconciliation * no-mistakes(document): Document routed wake and stall supervision * no-mistakes(lint): Fix ShellCheck expansion and subshell warnings * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes(review): Retire stale wake state and defer pre-move wakes * no-mistakes(review): Secure markers, bind batches, and preserve teardown routes * no-mistakes(review): Preserve unresolved prepared wakes across unrelated handoffs * no-mistakes(review): Preserve prepared wakes before unrelated moving handoffs * no-mistakes(document): Document prepared wake batch ownership * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes(review): Make local wake retirement recoverable * no-mistakes(document): Clarify handoff recovery and teardown documentation * fix: make macOS inbox test path portable (kunchenguid#2857) * feat(bin): deliver local steers through durable task inboxes (kunchenguid#2856) * feat(bin): steer local tasks by durable inbox record plus constant doorbell Stage 1 (local steers) of the captain-adopted reframe in data/fm-send-reliability-reframe-s1/report.md: an ordinary fm-send text steer to a task recorded in this home is appended as a sequenced durable record under state/<id>.inbox/ and the terminal receives only one constant self-describing doorbell line, best-effort. The worker acknowledges by moving the record into handled/; the watcher re-rings an unacknowledged message on an idle pane and escalates once as an ordinary stale wake. --resolve-key closes decisions at enqueue time, because the durable enqueue IS delivery to the task's record. bin/fm-task-inbox-lib.sh owns the record format, doorbell line, and re-ring ladder. The typed plane remains for what must reach the terminal itself: lifecycle keys, harness-native slash and codex $-skill invocations, explicit backend targets, and the remote secondmate leg (unchanged until the remote inbox leg ships separately). The composer classifier is demoted from delivery proof to an advisory ring guard that skips only on a proven pending verdict. Verified live against claude, codex, opencode, pi, grok, and muse: each real worker read its record, acted, and acked with the mv (docs/verification/runtime-backends.md "Steering-inbox doorbell"). * docs(verification): flag the grok 1.0.5 composer-matrix staleness observed by the doorbell run * test(captain-hold): read the chat-channel answer from the durable inbox record * test: migrate fm-control's marker contrast to the inbox record and fix macOS wc padding in the tool-update suite * no-mistakes(review): Harden inbox locking, teardown races, and acknowledgements * no-mistakes(review): Serialize watcher actions with inbox acknowledgements * no-mistakes(review): Bound metadata locking and tighten acknowledgement rechecks * no-mistakes(review): Preserve exact inbox bytes and harden delivery recovery * no-mistakes(review): Harden watcher bookkeeping against concurrent inbox teardown * no-mistakes(document): Update inbox and typed-plane documentation * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * revert(pipeline): keep parser-native secondmate marking and the both-failed exit out of stage 1 The CI monitor's fix changed the secondmate marking contract for parser-native invocations (appending the marker after the text) and softened the both-commit-and-marker-failed branch to exit 0. The merge authority ruled the marking question out of scope for this stage-1 transport PR (follow-up: fm-send-secondmate-harness-invocation-r1) and ruled the both-failed case a loud nonzero local failure. Restore both, keeping the monitor's legitimate migrations and hardening. * no-mistakes(document): Document inbox and typed-plane boundaries * no-mistakes(document): Scope backend transport docs to typed plane * no-mistakes(document): Clarify inbox attempt-budget documentation * no-mistakes: apply CI fixes * fix(send): the durable record alone governs the inbox exit status Captain-refined ruling on the F2/Greptile finding: the durable inbox record is what delivers the steer, so pending-reply bookkeeping trouble after a successful enqueue never exits nonzero - a resend-inviting status would make automated callers enqueue the delivered instruction again under a new sequence. With the recovery marker stored the watcher reconciles silently; with the commit and marker both lost the send surfaces a distinct reply-tracking-degraded do-not-resend warning and still exits 0. Nonzero remains only where nothing was delivered (or a decision close needs its manual command). Regression: record durable + both bookkeeping writes lost -> exit 0, one record, no duplicate. * no-mistakes(review): Preserve inbox ordering with drain-all doorbells * no-mistakes(review): Surface unwritable inbox ladder bookkeeping * no-mistakes(review): Silence ladder failures after inbox acknowledgement * no-mistakes(document): Update steering inbox documentation * no-mistakes: apply CI fixes * feat(bin): add fast local lint mode (kunchenguid#2891) * feat: add fast local lint mode * fix: preserve complete fm-lint help * fix: isolate fast lint mode * no-mistakes(document): Clarify lint mode documentation ownership * no-mistakes: apply CI fixes * feat(bin): deliver remote steers through durable inboxes (kunchenguid#2901) * feat(bin): deliver remote secondmate steers through durable task inboxes Stage 2 of the inbox+doorbell steer channel (stage 1: kunchenguid#2856). A remote secondmate steer now crosses fm-on.sh as a durable record written idempotently into the remote home's steering inbox plus a best-effort remote doorbell, and the last typed-payload steer transport is deleted: - fm-remote-secondmate-control.sh cmd_send writes the record via the new fm_task_inbox_write_idempotent and rings the doorbell; it no longer types the payload through an inner fm-send at an explicit pane target. - fm-send.sh routes every remote text steer (harness-native included, which marking already reduced to chat) onto the remote inbox leg, retries the identical leg once on ssh 255, closes --resolve-key decisions at enqueue for remote too, and preserves a marked request's reply expectation when completion stays unknown. The exit-3-as- delivered remap, the 255 do-not-resend trap, and the remote typed submit block are removed. - fm-task-inbox-lib.sh owns the idempotent enqueue: an exact-body re-run lands on the existing record, handled or not, so an ambiguous transport can always be safely re-run. - Tests pin the new contract end to end (record + doorbell + no typed payload across ssh, one-record idempotence under an ambiguous transport, enqueue-time decision close, loud real failures, and the deleted typed-payload behaviors gone), and AGENTS.md plus docs/remote-secondmates.md describe the remote leg's new semantics. * no-mistakes(review): Harden remote inbox delivery against lifecycle races * no-mistakes(review): Enable correlation-preserving remote steer resends * no-mistakes(review): Fail closed on stale correlation resends * no-mistakes(review): Include home context in remote resend commands * no-mistakes(review): Lock and revalidate remote parent routes * no-mistakes(document): Clarify remote steer retry documentation * no-mistakes: apply CI fixes * feat: add persistent Pi supervision branch (kunchenguid#2858) * wip: forked supervision on Pi (checkpoint before docs) * fix(pi-branch): harden mirror delivery, fallback encoding, and session replacement Peek-then-shift mirror flush so a failed append retries instead of dropping; durable mirror cursor commits only after delivery into the branch; the main fallback wake is operational-encoded like every watcher injection; session_shutdown quiesces the generation and session_start re-arms, so /new and /resume no longer kill the branch permanently. Registers the extension in the strict typecheck, adds the dispatch handshake test, the branch extension suite, the bash-level regression suite, the session-start replay test, and the opt-in real-SDK live guard. * test(fixtures): carry the branch-dispatch lib and lease lib into isolated fixtures The watcher extension now imports lib/fm-branch-dispatch.ts and fm-teardown sources fm-lease-lib.sh, so every fixture that copies or symlinks those files in isolation gains the new sibling. * no-mistakes(review): Prevent shutdown wake loss and serialize lease claims * no-mistakes(review): Durably hand off wakes and retain portable leases * no-mistakes(review): Require durable reports and clear disposed branch leases * no-mistakes(review): Enforce per-wake outcomes and quiescent lease cleanup * no-mistakes(review): Require wake acknowledgements and tighten branch lifecycle boundaries * no-mistakes(review): Require complete acknowledgements and replay cleanup failures * no-mistakes(review): Bind supervision to lock ownership and durable delivery * no-mistakes(review): Activate branch lazily after session lock acquisition * no-mistakes(review): Preserve undelivered mirror context across extension rebinds * no-mistakes(review): Acknowledge startup replay only after main delivery * no-mistakes(review): Isolate replay metadata from untrusted digest content * no-mistakes(review): Reject duplicate reports for active wake sequences * no-mistakes(review): Retain failed fallbacks and deduplicate outcome replay * no-mistakes(review): Deduplicate durable outcomes and cache delivery receipts * no-mistakes(review): Anchor wake sequence matching to outcome fields * no-mistakes(document): Clarify Pi supervision durability contracts * no-mistakes(lint): Fix ShellCheck issues in branch supervision scripts * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * refactor(pi-branch): collapse to confused-agent-grade guards per captain decision Captain decision A: the lease/actor guards target the CONFUSED-AGENT threat model bin/fm-gate-refuse-lib.sh already documents; adversarial-grade separation is impossible in the shared-process design and is filed as separate follow-up work. Rip out the machinery that chased it: the generation fence and shell-provenance markers, the wrapper-tagged ancestry walks, guard auto-claim with per-script release traps, the pending-wake files and ack-receipt correlation (the durable wake queue already re-presents anything unacknowledged), the delivery-receipt store with contiguous cursor advancement, the session-start replay-metadata channel, and the branch tool quiescence counters. Keep the behaviors the board requires, each on its simplest implementation: lazy per-action session-lock ownership (cold start activates after the lock lands; a secondary session stays inert), mirror durability across extension rebinds via the durable cursor, replay-exactly-once from the one read cursor, the awaited operational-encoded fallback, per-generation stray-lease cleanup, session-lock-bound lease liveness (a recycled pid or a non-Pi home never honors a leftover lease), the loud accidental-override guards (readonly actor prelude, cross-actor claim refusal), and the role-partition refinements (no forced teardown, no direct relaunch for the branch). Default-on-for-Pi is unchanged. * no-mistakes(review): Enforce lock ownership and serialize lease mutations * no-mistakes(review): Synchronize guard cleanup and bind leases to lock owner * no-mistakes(review): Report outcomes before acknowledging durable wakes * no-mistakes(review): Restrict leases to Pi and instruct main claims * no-mistakes(review): Reject malformed lease locks and torn outcome tails * no-mistakes(review): Validate complete outcome tails before appending * no-mistakes(review): Guard branch side effects across session replacements * no-mistakes(document): Update Pi supervision durability and lease documentation * no-mistakes(lint): Suppress intentional nested-shell expansion warning * no-mistakes: apply CI fixes * fix(pi-branch): authorize lease releases by caller * fix(lint): break redundant source-analysis path in fm-lease-lib.sh fm-lease-lib.sh's lazy fallback source of fm-wake-lib.sh gave ShellCheck's --external-sources traversal a second path into an already 1540-line file that fm-send.sh and fm-teardown.sh also source directly, blowing up the recursive analysis past CI's lint timeout. Mark it a source=/dev/null analysis boundary, matching the existing fm-task-inbox-lib.sh convention. Also restores bin/fm-lint.sh and tests/fm-lint.test.sh to the shared serial-lint definition (dropping an unrelated parallel-sharding change that was itself hanging and masked this root cause). * no-mistakes(document): Correct lease caller-authorization documentation * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * fix(bin): parallelize startup network sweeps (kunchenguid#2927) * feat(bin): parallelize session-start remote secondmate network sweeps Run per-secondmate liveness and convergence probes concurrently and overlap clone refresh, while replaying each mate's fail-closed diagnostic in original order. Ignore scratchpad* so untracked scratch no longer blocks remote sync. Co-authored-by: Cursor <cursoragent@cursor.com> * no-mistakes(document): Document parallel startup network sweeps * no-mistakes(lint): Fix empty environment assignment lint warning * no-mistakes: apply CI fixes --------- Co-authored-by: Cursor <cursoragent@cursor.com> * test: handle absent watcher wake queues (kunchenguid#2845) * fix(tests): count declared-pause wakes without crashing on an absent queue The exited-declared-pause case counts queued stale wakes by handing state/.wake-queue straight to awk. A watcher that queues nothing never creates that file, and awk aborts on a missing path before its END rule runs, so the count collapses to the empty string. The next comparison then fails as an integer-expression error and surfaces as a wake flood with no number, hiding the real contract breach the following grep names. Read the queue the way the drain-count assertion at the end of this file already does: silence awk's open error and default an absent queue to zero. Applied to all four counts in this case, including the live external-decision gate pair whose queue an acknowledged drain can also leave behind. An absent queue now reports "did not use the bounded paused recheck", while a genuine flood still fails with its real count. Fixes kunchenguid#2628 * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * style(pi): distinguish routine and captain supervision merge notes by icon (kunchenguid#2934) * style(pi): restyle supervision merge notes with a sailboat and matching pad Secondary-session notes were flush against the TUI edge and fully tinted. Use the sailboat prefix, Pi's default outputPad, boat-only color, and dim remainder so they sit like real messages. * style(pi): distinguish routine and captain merge notes by icon only Visible notes now lead with a sailboat or anchor, then only the dim outcome. Drop the branch-merged wording and verdict brackets so the icon is the only kind signal. * docs(pi): add the approved multi-brain architecture poster (kunchenguid#2938) The markdown contract stays the owner; the still is only the visual of the idea. * feat(pi): default branch supervision and route heartbeats (kunchenguid#2939) * fix(bin): bound remote job worker supervisor restarts (kunchenguid#2942) * fix(bin): bound remote worker supervisors * no-mistakes(review): release incumbent supervisor before starting its replacement * no-mistakes(review): wait out a healthy same-root supervisor instead of replacing it * no-mistakes(review): narrow remote worker change to restart accounting only * no-mistakes(document): clarify supervisor restart guard is a lifetime total * fix: safely split supervision wake handling by actor (kunchenguid#2953) * feat(bin,pi): per-actor wake consume, silent success gating, merge-poll dedup Three related fixes to the shared wake-drain and Pi supervision-branch dispatch machinery so a routine success is never main-blocking and a mixed queue can safely split between actors. 1. Successful routine results no longer create main-blocking wake rows. fm-startup-network.sh only enqueues a check: startup-network wake when the deferred result is actionable (state is not "done", or the report carries a bootstrap-diagnostics actionable prefix); a clean success stays durable in the report file without ever waking the agent. 2. Per-actor wake-drain consume contract. bin/fm-wake-drain.sh now scopes presentation and --ack-through to the current actor (bin/fm-lease-lib.sh's fm_lease_actor): main keeps the original whole-queue cutoff behavior, unaffected. A branch actor (FM_SUPERVISION_ACTOR=branch, set only inside the Pi supervision branch's own bash tool calls) is scoped to an explicit eligible-row snapshot instead of a cutoff comparison, so it can never remove a row it was not granted - the fix for the swallow risk that used to force an all-or-nothing whole-queue fallback to main. .pi/extensions/lib/fm-branch-dispatch.ts's scopeForUnreadWake is the single owner of eligibility: a check-kind row (merge-confirmation polls, Relay mentions, credential/auth failures) is now excluded rather than vetoing the whole scan for a non-heartbeat wake, while a heartbeat review keeps its original all-or-nothing rule unchanged. writeEligibleRowsSnapshot publishes the exact eligible sequence numbers before every branch prompt; fm-primary-pi-watch.ts's offer still refuses a check-kind trigger outright so a main-only close is never itself routed to the branch. 3. A repeat identical merged-PR-poll result for an already-notified task is absorbed instead of enqueued again. A poll's own retirement state is scoped to one registration and cannot see a prior registration's outcome, so a task re-registered after its merge was already surfaced would otherwise wake main a second time for the same event. bin/fm-pr-lib.sh's new per-task pr-poll-merge-notified marker survives across re-registrations to catch that case; the first notification for a task still reaches main unchanged. Regression tests colocated in tests/fm-startup-network.test.sh, tests/fm-wake-queue.test.sh (including the mixed-queue no-swallow property), tests/fm-pi-branch-extension.test.sh, and tests/fm-pr-check-security.test.sh. docs/watcher-continuity.md and docs/pi-supervision-branch.md updated for the new contracts. * no-mistakes(review): Bind merge deduplication to canonical PR identity * no-mistakes(review): Serialize wake row ownership across main and branch * no-mistakes(review): Bind branch grants and deduplicate within actor claims * no-mistakes(review): Fallback main-owned wake claims to main delivery * no-mistakes(review): Clarify silent startup success guidance * no-mistakes(review): Release residual branch grants after settled prompts * no-mistakes(review): Reject truncated wake rows as corrupted * no-mistakes(document): Document per-actor routing and silent startup success * no-mistakes(lint): Fix ShellCheck findings in wake grant and startup test * no-mistakes: apply CI fixes * fix(pi): hide branch outcomes tool rows in Calm (kunchenguid#3024) * Hide branch outcome tool in Pi Calm * no-mistakes(review): Preserve stock outcomes rendering and document tool audit * no-mistakes(review): Document branch read tool audit disposition * no-mistakes(review): Match stock outcomes output sanitization * no-mistakes(document): Document Calm custom-tool visibility * no-mistakes(review): Secure Pi wake batches and complete orchestrator documentation * no-mistakes(document): Correct current orchestrator comparison facts * Add opt-in resumable Codex crewmates (#4) * Add opt-in resumable Codex crewmates * fix(bin): restore parked Codex binding after crash --------- Co-authored-by: wenkxu <v-wenkxu@expediagroup.com> * Add persistent human names and names-first routing (#6) * Add persistent human callsigns * Close persistent callsign safety gaps --------- Co-authored-by: wenkxu <v-wenkxu@expediagroup.com> * Bind tmux task routing to stable pane identity (#7) * Bind tmux task routes to stable pane identity * Handle failed tmux identity binding --------- Co-authored-by: wenkxu <v-wenkxu@expediagroup.com> * fix: gate Codex checkpoints on genuine idle (#12) Co-authored-by: wenkxu <v-wenkxu@expediagroup.com> * feat: register external workspace routes (#13) Co-authored-by: wenkxu <v-wenkxu@expediagroup.com> * feat(codex): propagate assigned display titles (#14) * feat(codex): propagate assigned display titles * test(codex): keep title resume guard isolated --------- Co-authored-by: wenkxu <v-wenkxu@expediagroup.com> * no-mistakes(review): Secure Pi wake batches and complete orchestrator documentation * no-mistakes(document): Correct current orchestrator comparison facts --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: stanzhang <thinking.chang@gmail.com> Co-authored-by: wenkxu <v-wenkxu@expediagroup.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
Scope boundaries
Validation
bin/fm-test-run.sh tests/fm-workspace.test.shbin/fm-test-run.sh tests/fm-brief.test.sh tests/fm-spawn-batch.test.shtests/fm-session-start.test.sh; that broader suite later hit its pre-existing synthetic secondmate-home failure (not a git repo) outside this diffbin/fm-test-run.sh --check-coveragebin/fm-doc-audience-check.shgit diff --checkRefs #9
CodeAnt-AI Description
Add safe routing for external workspaces and their member repositories
What Changed
Impact
✅ Explicit external workspace routing✅ Fewer accidental primary-checkout edits✅ Fail-closed handling of workspace and instruction drift💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.