Skip to content

feat: sync fork with upstream, adding Muse Code adapter and Relay rename - #90

Merged
trillium merged 9 commits into
mainfrom
fm/upstream-merge-into-fork
Aug 6, 2026
Merged

trillium merged 9 commits into
mainfrom
fm/upstream-merge-into-fork

Conversation

@trillium

@trillium trillium commented Aug 6, 2026 •

Copy link
Copy Markdown
Owner

Intent

Merge the latest upstream firstmate changes into trillium's fork branch so the fork is up to date with upstream, preserving the merge commit (no rebase).

What Changed

  • Merged upstream/main into the fork branch (preserving the merge commit), pulling in the Muse Code crewmate adapter (docs/verification/muse.md, muse harness/signal tests, Muse session log idle classification) and hook-driven deterministic session start (new bin/fm-sessionstart-run.sh and bin/fm-timeout-lib.sh plus supporting tests).
  • Brought in upstream behavior changes to the herdr backend, requiring version 0.8.0 for default presentation spaces (bin/backends/herdr.sh, version-floor e2e test) and renamed the user-facing "X mode" surface to "Relay" across docs and skills.
  • Applied a fork-side documentation cleanup on top of the merge, aligning Relay terminology and removing duplicate entries from the documentation inventory (docs/fork-features.md, docs/documentation-audiences.json).

Risk Assessment

✅ Low: The branch is a fully clean automatic upstream merge (merge-tree byte-identical to the actual merge commit, zero conflicts, no evil-merge edits) preserving the merge commit as intended, plus one small fork docs commit that correctly de-duplicates inventory entries (valid JSON) and applies a rename consistent with upstream.

Testing

Baseline git inspection plus targeted tests confirm the intent. The merge commit 9afe6ea is a genuine two-parent merge (fork base 92088d5 + upstream tip 2cf0283), not a rebase; the upstream tip is an ancestor of the fork HEAD and upstream files are integrated; the tree has zero conflict markers. The authoritative fork-features guard suite reports no regression at the target and identical results at the base (17 pass / 15 expected-gaps / CI green), with the guard script and baseline untouched by the merge. A behavioral test of merged code (fm-composer-lib) passed 9/9. One heavy e2e test (fm-bootstrap) timed out on git ops due to worktree-.git-pointer sensitivity, an environment issue unrelated to the merge. Evidence artifacts were written to the evidence directory. Overall: merge is clean and satisfies every intent constraint.

Evidence: Merge verification (graph, parents, ancestry, no conflict markers)

Merge graph: * 00ccc5a no-mistakes(document): ... * 9afe6ea Merge upstream/main into fork |
| * 2cf0283 docs(agents): read the persisted digest (#1794) | * 8ec3e94 fix(bin): classify settled Muse session logs as idle (#1788) | * 8387039 fix(herdr): require 0.8.0 (#1787) | * 930ca76 feat: add Muse Code crewmate adapter (#1786) CONFIRMED: fork up to date with upstream tip 2cf0283 CLEAN: zero conflict markers

### Merge graph (proves a merge commit, NOT a rebase) ###
* 00ccc5a no-mistakes(document): Updated Relay terminology and fixed documentation inventory duplicates
*   9afe6ea Merge upstream/main into fork
|\  
| * 2cf0283 docs(agents): read the persisted digest when only a preview is shown (#1794)
| * 8ec3e94 fix(bin): classify settled Muse session logs as idle (#1788)
| * 8387039 fix(herdr): require 0.8.0 for default presentation spaces (#1787)
| * 930ca76 feat: add Muse Code crewmate adapter (#1786)

### Merge commit parents (parent1=fork base, parent2=upstream/main tip) ###
9afe6ea  parents=[92088d5 2cf0283]  Merge upstream/main into fork

### Upstream tip 2cf0283 is an ancestor of the fork HEAD ###
CONFIRMED: fork is up to date with upstream/main tip 2cf0283

### No conflict markers in tracked tree ###
CLEAN: zero conflict markers
Evidence: Fork-features guard suite (no regression vs base)

=== Summary === Passed: 17 Failed (expected gaps): 15 No regressions detected. CI is green. ✓ (identical at base commit 92088d5 and target 00ccc5a)

=== Fork Features Guard Suite (Regression Mode) ===
Running self-test of accounting...
  ✗ SELF-TEST: nonexistent file
Self-test passed: accounting is correct ✓


Testing: Multi-account Claude Code
  ✗ fm-spawn.sh --account flag
  ✗ bin/claude-account.sh exists
  ✗ Multi-account documented
  ✗ fm-spawn.sh parses ACCOUNT
  ✗ bin/claude-account.sh executable

Testing: Remote Dispatch (SSH-based)
  ✗ fm-spawn.sh --remote flag

Testing: Beads Integration
  ✗ bin/fm-brief-hooks.d/beads.sh
  ✗ bin/fm-beads-resilience-lib.sh
  ✗ bin/fm-bead-stamp.sh
  ✗ bin/fm-spawn-hooks/beads
  ✗ fm-brief.sh loads beads

Testing: Fork-local Skills
  ✗ herdr-navigation skill

Testing: Fork-origin Validation
  ✗ Fork-origin check

Testing: Decision Hold Lifecycle

Testing: Beads Task-Store Backend
  ✗ Beads backend configured

Testing: X-mode Integration

Testing: Herdr Backend Support
  ✗ herdr-navigation skill (Herdr backend integration)

=== Regression Analysis ===

=== Summary ===
Passed: 17
Failed (expected gaps): 15

No regressions detected. CI is green. ✓
Evidence: Merged-code behavioral test: fm-composer-lib (9/9 ok)

ok - agent prompt glyphs (❯ claude, › codex, ⟩ muse) read empty bordered or bare ok - a bare shell prompt glyph reads unknown, never empty ... 9 ok / 0 not ok (exit 0)

ok - fm_composer_classify_content: a bare shell prompt glyph (>/$/%/#) reads unknown, never empty
ok - fm_composer_classify_content: stripped unbordered content is unknown except verified agent glyphs
ok - fm_composer_classify_content: a bare shell prompt carrying a command is not empty
ok - fm_composer_classify_content: a bare prompt glyph inside a bordered composer box reads empty (claude's own idle composer)
ok - fm_composer_classify_content: agent prompt glyphs (❯ claude, › codex, ⟩ muse) read empty bordered or bare
ok - fm_composer_classify_content: an empty composer reads empty
ok - fm_composer_classify_content: a known idle placeholder reads empty, before and after glyph stripping
ok - fm_composer_classify_content: idle matching preserves the caller's case mode
ok - fm_composer_classify_content: real unsubmitted text reads pending (including a popup argument-hint fill)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • git log --graph --oneline 00ccc5a — confirmed two-parent merge commit (merge, not rebase)
  • git merge-base --is-ancestor 2cf0283 00ccc5a — upstream tip is ancestor of fork HEAD (fork up to date)
  • git grep -nE '^( | | )$' — zero conflict markers in tracked tree
  • bash tests/fork-features.sh at target and at base 92088d5 — both 17 pass / 15 expected-gaps / no regressions (fork features preserved by merge)
  • bash tests/fm-composer-lib.test.sh — 9/9 ok, exercises merged fork+upstream classifier code (incl. upstream muse glyph)
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Summary by CodeRabbit

  • New Features

    • Added Muse as a supported crewmate and scout harness, including session tracking, configurable models and effort levels, credential checks, and lifecycle handling.
    • Expanded Relay integration to cover X and Discord public mentions, with updated configuration and follow-up guidance.
    • Improved session-start handling across supported environments, including context resets, compaction, resume, and bounded digest delivery.
    • Added configurable Herdr presentation layouts with version-aware defaults and safe fallback behavior.
  • Bug Fixes

    • Improved interrupt cleanup and Ctrl-U key handling across supported terminal backends.
    • Enhanced agent liveness and busy-state detection for Muse sessions.

kunchenguid and others added 9 commits August 5, 2026 23:10
…#1778)

Discord mentions already ride the same pairing-token opt-in, relay poll,
and platform-aware reply path as X mentions, but the docs still read as
X-only, so a stranger could not self-serve the Discord path.

Add the numbered turn-on steps to the X mode configuration reference,
pointing at the myfirstmate dashboard for account creation, bot install,
and token issuance rather than duplicating operator setup here, and drop
the X-only framing from the README bullet, the documentation index, and
the architecture overview.
…#1781)

* feat(bin): run session start deterministically on hook-capable harnesses

Session start relied on a native nudge that only asked the agent to run
bin/fm-session-start.sh, and an agent can defer that. Observed 2026-08-01:
an /ahoy-first session followed the recap path and did not take the helm
until a later request forced it.

Claude, Codex, and Pi now RUN the digest in their session-open hook through
the new bin/fm-sessionstart-run.sh, so the full ordered digest is in model
context before the first turn. That wrapper is the single owner of what a
session-open source means: startup and Pi's "new" take the helm, clear and
compact re-emit, resume/reload/fork delegate to the nudge, and an unreadable
source takes the helm because doing that redundantly is idempotent while
skipping it is the bug. Grok and OpenCode keep the nudge as the floor, since
neither can carry hook stdout into a model turn.

Because the hook now blocks session initialization, fm-session-start.sh
bounds itself first. Its steps are not all individually bounded - bootstrap's
gh auth probe, tool version probes, the backlog listing and per-task endpoint
reads are unbounded - so the whole digest runs as one bounded child (default
120s). Whatever it emitted before the bound survives, and the parent adds a
loud STARTUP TRUNCATED banner naming the stage that stalled and every stage
that never ran, still exiting 0.

--reemit skips only the sweeps startup already reconciled. It still re-verifies
lock ownership and still drains queued wakes, which arrived after startup and
are the turn's work. fm-bootstrap.sh gains FM_BOOTSTRAP_LOCKED so a re-emit
keeps repair ownership instead of deferring to a lock holder that is itself.

Also adds bin/fm-timeout-lib.sh as the single owner of bounded execution,
replacing three near-identical copies, and gives the ahoy skill a helm check
so a nudge-tier harness cannot recap before taking the helm.

Verified live on 2026-08-05 against Claude 2.1.222, Codex 0.146.0, and Pi
0.82.0; docs/verification/supervision.md records the per-harness source
vocabulary, the two named gaps, and the refresh command.

* no-mistakes(review): Harden session-start completion, timeout, and Pi delivery

* no-mistakes(review): Harden completion ownership and portable timeout escalation

* no-mistakes(review): Normalize watchdog KILL exits without masking command status

* no-mistakes(review): Guarantee startup bounds and align harness delivery tiers

* no-mistakes(test): Fix Pi session-start live verification fixture

* no-mistakes(document): Align session-start documentation with deterministic hooks

* no-mistakes(lint): Silence intentional child-shell expansion lint warning

* no-mistakes: apply CI fixes

* no-mistakes: apply CI fixes
* docs: rename the user-facing product name to Relay

The public-mention integration gated by the `.env` pairing token is now
called Relay across user-facing prose, covering X and Discord alike
instead of implying a single network.

Renames the product-name strings only: README, docs, the captain-facing
skill descriptions, and the AGENTS.md operating prose, including the
`X mode (.env)` and `Optional X mode` headings and every link anchor
that pointed at them. AGENTS.md section 14 carries a one-line bridge
note so the older name and the unchanged identifier spellings stay
discoverable.

Internal identifiers are untouched: `FMX_*`, `config/x-mode.env`,
`state/x-*`, `bin/fm-x-*`, the `fmx-respond` skill path,
`__FM_X_MODE_ENV__`, and `x-mode-error`. Platform references to X and
Discord as networks stay as they are, and the bootstrap-diagnostics
entry still quotes bootstrap's emitted `FMX: X mode on/off` line
verbatim because `bin/` output is out of scope for this pass.

* no-mistakes(review): Complete Relay prose rename in maintained docs

* no-mistakes: apply CI fixes
* feat(harness): add a verified muse crewmate adapter

Muse Code joins the fleet as a crewmate/scout adapter, verified live against
Muse Code 0.1.0-R708.1 in an isolated lab.

Detection matches the anchored prefix muse-bin*, because the installed launcher
execs a version-suffixed binary whose name changes on every auto-update and
whose install path carries no muse component to fall back on. The same identity
is taught to the tmux liveness classifier, without which a healthy muse pane
would have read as a dead endpoint.

Busy state folds muse's own durable session event log, bound per task by a
sessions-root/worktree sidecar. It is a pull source with no writer, so nothing
is armed and no record is ever seeded. The fold is anchored on the full run
lifecycle prefix so muse's nested cleanup "terminal" payloads cannot settle an
in-flight run, and it is depth-bounded so muse's native sub-agent logs cannot be
mistaken for the parent's. The idle half stays gated: an open run proves busy,
but a settled log reads unknown until a credentialed multi-step run proves one
turn stays inside one run.

Two findings corrected the scout report. The exec-only
--no-foreign-personal-context flag is rejected by the interactive TUI, so the
privacy control that actually reaches a pane worker is
MUSE_EXPERIMENTAL_FOREIGN_PERSONAL_CONTEXT_KILL, verified to drop the operator's
foreign personal rules while keeping the project's own AGENTS.md. And an
unauthenticated muse pane never exits, it waits on a device-code prompt, so
credentials are a spawn preflight rather than a screen check.

muse is refused for secondmates: it has no primary supervision protocol and its
hook dialect rejects the reawakening handlers that protocol needs.

Per the captain's decision, auto-update is not pinned, and the credentialed
multi-step smoke is deferred with an explicit checklist in
docs/verification/muse.md.

* no-mistakes(review): Accept Muse dispatch profiles and shared efforts

* no-mistakes(review): Bind Muse busy state to current session

* no-mistakes(review): Compare Muse workspace bindings literally

* no-mistakes(review): Harden Muse worker credentials and live signal verification

* no-mistakes(review): Cache Muse session bindings and clarify worker credentials

* no-mistakes(review): Clear Muse marker inheritance and normalize interrupt aliases

* no-mistakes(review): Verify Muse glyph effective foreground color

* no-mistakes(review): Harden Muse XDG paths, session cache, and glyph parsing

* no-mistakes(document): Document Muse adapter boundaries
…d#1787)

* fix(herdr): floor default-on presentation spaces at Herdr 0.8.0

Default-on presentation projection turns every crewmate teardown into a
workspace-emptying removal. The focus-safe removal plan avoids Herdr's
focus-stealing explicit close only while the doomed pane's shell can be proved
lone, childless, and idle; a persistent child of that shell (gitstatusd, a
zsh-async worker, direnv) fails that proof permanently and forces the plain
close, which on every release before Herdr 0.8.0 moves the captain's active
workspace for ~140ms on each teardown.

Gate the unconfigured default behind a Herdr 0.8.0 floor. At or above it,
project as before; below it, fall back to the flat per-home layout with one
warning per home per detected release naming the version and the upgrade. An
explicit "on" - including the historical empty opt-in file - is still honored
below the floor, so a deliberate opt-in is never silently downgraded.

The floor reads two independent signals from the client's own status, either of
which can establish a supported release: the protocol number and the release
core of the version string. Measured against the real release binaries, no build
lacking both upstream focus fixes reaches protocol 19 and every pre-fix build
tops out at 17, so protocol 19 is a safe structural expression of the floor. A
release that reports neither signal readably is treated as unsupported rather
than guessed at.

Also:
- Correct the adapter comment claiming the mitigation "stays safe without any
  version gate". That holds for the pane-death route only; the plain-close
  fallback is reachable precisely on the releases where it is unsafe.
- Stop discarding the projected-close helper's stderr at teardown, so a refused
  or failed focus restore is visible instead of silent. The close stays
  non-fatal; the presence gate still decides record removal.
- Add Part C to the focus-flash regression: a doomed pane whose shell holds a
  persistent child, in the geometry where the closing workspace's right
  neighbour is not the anchor. That is the fallback branch the suite could not
  structurally reach. On 0.7.5 it observes a bounded four-sample wrong-focus
  window restored exactly; on 0.8.0 it observes none. It also cross-checks its
  own measurement against the floor classifier, so a drifted protocol mapping
  fails loudly.
- Make the projection suite's unconfigured-home case release-aware, so the whole
  real-Herdr lane passes on both the CI-pinned 0.7.4 and 0.8.0.
- Add an opt-in live guard that re-measures the release-to-protocol mapping
  against the pinned upstream binaries.

The immediate no-code mitigation for a home that cannot upgrade remains writing
"off" into config/herdr-presentation-spaces.

* no-mistakes(review): Pin Herdr live-guard digests across supported platforms

* no-mistakes(review): Document authorized Herdr cleanup containment

* no-mistakes(review): Harden Herdr warning marker publication

* no-mistakes(review): Honor running Herdr server presentation floor

* no-mistakes(review): Recheck Herdr floor after server ensure

* no-mistakes(review): Refresh 0.7.5 and 0.8.0 focus transcripts

* no-mistakes(review): Route Herdr floor probe through lab session

* no-mistakes(document): Align Herdr floor documentation and comments

* no-mistakes(lint): Document Herdr presentation out-parameter consumer
* fix(muse): trust the settled session log as idle

The credentialed multi-step smoke on Muse Code 0.1.0-R708.1 answered the one
question the idle half was held back for: one real 75-second tool-loop turn with
23 tool batches stays inside exactly one run started/terminal pair, and an
Escape mid tool loop closes that run as cancelled rather than leaving the turn to
continue in another run. A settled log is therefore a finished turn, not a pause
between the runs of one turn.

Remove fm_busy_muse_idle_verified and FM_BUSY_MUSE_IDLE_VERIFIED_VERSIONS
outright rather than pinning them to a version: the session log's own metadata
carries only semver 0.1.0 and a build sha, so a version allowlist could not
actually match the running build and would be false precision. A settled log now
classifies idle, an open run still classifies busy, and only a resolution
failure - no binding, no matching log, an unreadable or run-free log - stays
unknown.

Record the evidence in docs/verification/muse.md, including the run-scoped grep
the counts must use, and keep the post-upgrade re-check guidance.

* no-mistakes(review): Document Muse idle trust and remove stale gate reference

* no-mistakes(document): Clarify Muse idle verification ownership
@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This pull request adds bounded session-start execution and source routing, introduces Muse as a verified crewmate/scout harness, adds Herdr release-floor handling, centralizes timeout execution, and renames the documented X-mode integration to Relay across code guidance and documentation.

Changes

Session startup and runtime controls

Layer / File(s) Summary
Session-start routing and bounded execution
bin/fm-session-start.sh, bin/fm-sessionstart-run.sh, .claude/settings.json, .codex/hooks.json, .pi/extensions/..., docs/sessionstart-nudge.md, tests/fm-session*
Hooks route startup, resume, reset, fork, and compact events through the new runner. Digest execution supports bounded output, truncation reporting, completion-aware re-emission, and nudge delegation.
Shared timeout execution
bin/fm-timeout-lib.sh, bin/fm-bearings-snapshot.sh, bin/fm-fleet-snapshot.sh, bin/fm-vendor-auth-probe.sh
Scripts use fm_run_timed for bounded commands, process-group cleanup, and normalized timeout status handling.

Muse harness integration

Layer / File(s) Summary
Muse launch and lifecycle support
bin/fm-spawn.sh, bin/fm-harness.sh, bin/fm-send.sh, bin/fm-teardown.sh, bin/fm-busy-lib.sh
Muse launches use isolated paths, credential preflight, model and effort mapping, positional prompts, and crewmate/scout restrictions. Session logs provide verified busy-state classification.
Muse composer and liveness coverage
bin/fm-composer-lib.sh, bin/backends/tmux.sh, tests/fm-muse-harness.test.sh, tests/fm-muse-signals-live-e2e.test.sh, tests/fm-composer-*, tests/fm-tmux-agent-liveness.test.sh
The Muse prompt glyph, interrupt cleanup, exact process identities, session-log binding, cache validation, nested-log exclusion, and live signal behavior are covered.
Muse documentation and verification
.agents/skills/harness-adapters/SKILL.md, docs/configuration.md, docs/verification/muse.md, docs/tmux-backend.md, docs/trace-context.md
Documentation describes Muse launch constraints, credentials, context isolation, session logs, native worktrees, process identity, and verification results.

Herdr presentation handling

Layer / File(s) Summary
Release-floor preference and fallback
bin/backends/herdr.sh, bin/fm-spawn.sh, docs/herdr-backend.md, docs/configuration.md
Presentation spaces use default, on, and off preferences. The default requires compatible client and server releases, while unsupported releases use flat layout and warnings.
Focus and compatibility validation
tests/fm-backend-herdr.test.sh, tests/fm-backend-herdr-presentation-e2e.test.sh, tests/fm-backend-herdr-focus-flash-e2e.test.sh, tests/fm-herdr-version-floor-live-e2e.test.sh, tests/fm-teardown.test.sh
Tests cover release classification, warning safety, server compatibility, persistent-child fallback, focus restoration, explicit opt-in, and live release-floor evidence.

Relay terminology and operational documentation

Layer / File(s) Summary
Relay configuration and supervision terminology
AGENTS.md, README.md, docs/architecture.md, docs/configuration.md, docs/fork-features.md, docs/scripts.md, docs/turnend-guard.md, docs/supervision-protocols/*
The optional X-mode integration is documented as Relay and covers public mentions from X and Discord. Configuration, polling, follow-ups, and supervision references use Relay terminology.
Skill and verification guidance
.agents/skills/ahoy/SKILL.md, .agents/skills/bootstrap-diagnostics/SKILL.md, .agents/skills/fmx-respond/SKILL.md, docs/subagent-guard.md, docs/verification/public-followup.md, docs/verification/supervision.md, docs/verification/runtime-backends.md
Skills enforce the session digest check and update Relay references. Verification documentation records session-start, Muse, and Herdr behavior.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

  • trillium/firstmate#83: Contains overlapping session-start, Muse, Herdr, Relay, timeout, documentation, and test changes.
  • trillium/firstmate#85: Also updates docs/documentation-audiences.json to remove duplicate surface entries.

Suggested reviewers: kunchenguid

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.63% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the upstream sync and two major changes: the Muse Code adapter and the X mode to Relay rename.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fm/upstream-merge-into-fork

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 6

🧹 Nitpick comments (4)
bin/fm-timeout-lib.sh (1)

118-131: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider validating the bound inside fm_run_timed.

The header states that a non-positive bound is not a bound and that callers must reject 0. The mechanisms then disagree: timeout 0 and alarm 0 run unbounded, while the bash fallback runs sleep 0 and returns 124 at once. A single guard here removes that divergence for any future caller that forgets to validate.

♻️ Proposed guard
 fm_run_timed() {  # <seconds> <command...>
   local seconds=$1
   shift
+  case "$seconds" in
+    ''|*[!0-9]*|0)
+      printf 'fm-timeout-lib: bound must be a positive integer, got: %s\n' "$seconds" >&2
+      return 2
+      ;;
+  esac
   case "$(fm_timeout_mechanism)" in
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-timeout-lib.sh` around lines 118 - 131, Update fm_run_timed to
validate seconds before selecting a timeout mechanism, returning 124 immediately
for non-positive bounds. Preserve the existing mechanism dispatch for positive
values so timeout, gtimeout, perl, and bash retain their current behavior.
tests/fm-sessionstart-nudge.test.sh (1)

15-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Resolve the bash path instead of hard-coding /bin/bash.

Line 17 links /bin/bash. On hosts where bash is not at that path, ln -s succeeds but the fixture harness is a broken symlink, or the later exec fails, and line 17 exits 1 with no message. Resolve the running interpreter and report the failure.

♻️ Proposed fix
   HARNESS_FIXTURE=$(mktemp -d "${TMPDIR:-/tmp}/fm-sessionstart-harness.XXXXXX") || exit 1
-  ln -s /bin/bash "$HARNESS_FIXTURE/codex" || exit 1
+  FM_TEST_BASH=$(command -v bash) || { echo "not ok - bash not found for the harness fixture" >&2; exit 1; }
+  ln -s "$FM_TEST_BASH" "$HARNESS_FIXTURE/codex" \
+    || { echo "not ok - could not create the fixture harness symlink" >&2; exit 1; }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-sessionstart-nudge.test.sh` around lines 15 - 24, Update the harness
setup around FM_SESSIONSTART_TEST_HARNESS to resolve the running Bash
interpreter instead of hard-coding /bin/bash, then use that resolved path for
the codex symlink. Validate resolution before creating the fixture and emit a
clear error message if it fails, while preserving the existing cleanup and
status propagation flow.
tests/fm-sessionstart-hook-live-e2e.test.sh (1)

108-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the wrapper's source parser instead of copying it, and keep a malformed call recordable.

Two points:

  1. Lines 122-128 duplicate the awk parser in bin/fm-sessionstart-run.sh lines 92-98. If the wrapper parser changes, this recorder keeps the old behavior and the guard still passes. Extract the parser into a sourceable helper, or have the recorder call the real parser and only log the result.
  2. Line 117 runs shift 2 || exit 0. A bare trailing --source makes shift 2 fail, so the recorder exits before writing the record. The probe then reports "never invoked the wrapper", which misdiagnoses the failure. Mirror the wrapper's guarded shift.

As per coding guidelines: "use the owning repository script for its exact command, flag, validation, and mutation mechanics rather than reimplementing them".

♻️ Proposed fix for the shift handling
-    --source) source=${2:-}; shift 2 || exit 0 ;;
+    --source)
+      source=${2:-}
+      if [ $# -ge 2 ]; then shift 2; else shift; fi
+      ;;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-sessionstart-hook-live-e2e.test.sh` around lines 108 - 134, Update
the fm-sessionstart-run.sh recorder in the test to reuse the owning wrapper’s
source-parsing implementation instead of duplicating the inline awk parser,
preserving identical behavior if that parser changes. Also mirror the wrapper’s
guarded handling for a trailing --source so malformed calls still append a
record and produce the probe result rather than exiting before logging.

Source: Coding guidelines

tests/fm-session-start.test.sh (1)

1289-1309: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Guard the stray-process check against a missing pgrep.

Line 1308 pipes pgrep failure into wc -l, which yields 0. If a host has no pgrep, the process-group assertion passes without checking anything, so the test goes silently vacuous.

The fixed 3-second budget also assumes every stage before bootstrap finishes in under 3 seconds. On a slow runner an earlier stage can hit the bound, and the assertion on line 1297 then fails for an unrelated reason. Consider asserting the stage name from a recorded breadcrumb instead of a fixed budget.

♻️ Proposed guard
+  command -v pgrep >/dev/null 2>&1 \
+    || fail "pgrep is required to prove the runtime bound reaches the whole process group"
   stray=$(pgrep -f "$fakebin/git" 2>/dev/null | wc -l | tr -d ' ')
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-session-start.test.sh` around lines 1289 - 1309, Make the
stray-process assertion fail explicitly when pgrep is unavailable instead of
allowing its failure to be converted to zero by the wc pipeline. Also update the
truncated-session setup around run_session_start to derive or verify the
incomplete stage from the recorded breadcrumb, rather than assuming the fixed
timeout always reaches bootstrap; preserve the existing output and completion
assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bin/fm-composer-lib.sh`:
- Around line 220-221: Update fm_composer_classify_content’s leading-glyph
stripping cases to remove each multibyte Muse prompt using literal prefix
patterns rather than ${content#??} or ${content#?}. Preserve the existing
handling for the trailing space and ensure rows beginning with ⟩, along with the
other supported glyphs, are fully stripped so empty composer rows remain
classified as empty.

In `@bin/fm-session-start.sh`:
- Line 122: Update the user-facing session-start prose at the help output and
context re-emit sites to say “Relay artifact writes” instead of “X-mode artifact
writes.” Preserve all internal x- identifiers and change only the displayed
text.

In `@bin/fm-timeout-lib.sh`:
- Around line 59-84: Update the watchdog completion logic to read and validate
command_status before deciding the result from deadline_status, preferring the
recorded command status and falling back to 124 only when no valid status
exists. Replace the fractional sleep 0.2 in the KILL grace period with an
integer-second sleep plus a portable busy-wait, or otherwise skip the grace
period on strict POSIX hosts.

In `@docs/sessionstart-nudge.md`:
- Line 55: Update the code span in the session-start nudge documentation to add
one space immediately inside both double-backtick delimiters, preserving the
literal inner backticks and punctuation. Keep the trailing space in the
`FIRSTMATE_OP: ` label only if it is part of the emitted payload, and ensure the
rendered text remains unchanged apart from the lint-required span padding.

In `@tests/fm-calm-pi-extension.test.sh`:
- Around line 2865-2870: Add fm-session-lock-lib.sh to the copy list alongside
fm-sessionstart-run.sh in the fixture setup, ensuring the session-start runner
can source its lock helper and retain the expected lock functions.

In `@tests/fm-sessionstart-nudge.test.sh`:
- Around line 383-392: Add a writability probe in
test_run_reports_a_failed_session_start_as_digest_text after chmod 0500 and
before run_hook; if the test user can still create/write in $root/state, restore
permissions and skip the test. Keep the existing assertions for environments
where mode 0500 successfully blocks writes.

---

Nitpick comments:
In `@bin/fm-timeout-lib.sh`:
- Around line 118-131: Update fm_run_timed to validate seconds before selecting
a timeout mechanism, returning 124 immediately for non-positive bounds. Preserve
the existing mechanism dispatch for positive values so timeout, gtimeout, perl,
and bash retain their current behavior.

In `@tests/fm-session-start.test.sh`:
- Around line 1289-1309: Make the stray-process assertion fail explicitly when
pgrep is unavailable instead of allowing its failure to be converted to zero by
the wc pipeline. Also update the truncated-session setup around
run_session_start to derive or verify the incomplete stage from the recorded
breadcrumb, rather than assuming the fixed timeout always reaches bootstrap;
preserve the existing output and completion assertions.

In `@tests/fm-sessionstart-hook-live-e2e.test.sh`:
- Around line 108-134: Update the fm-sessionstart-run.sh recorder in the test to
reuse the owning wrapper’s source-parsing implementation instead of duplicating
the inline awk parser, preserving identical behavior if that parser changes.
Also mirror the wrapper’s guarded handling for a trailing --source so malformed
calls still append a record and produce the probe result rather than exiting
before logging.

In `@tests/fm-sessionstart-nudge.test.sh`:
- Around line 15-24: Update the harness setup around
FM_SESSIONSTART_TEST_HARNESS to resolve the running Bash interpreter instead of
hard-coding /bin/bash, then use that resolved path for the codex symlink.
Validate resolution before creating the fixture and emit a clear error message
if it fails, while preserving the existing cleanup and status propagation flow.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 60ec51ec-5aba-4eee-bfa1-314c83f810d9

📥 Commits

Reviewing files that changed from the base of the PR and between 92088d5 and 00ccc5a.

📒 Files selected for processing (64)
  • .agents/skills/ahoy/SKILL.md
  • .agents/skills/bootstrap-diagnostics/SKILL.md
  • .agents/skills/fmx-respond/SKILL.md
  • .agents/skills/harness-adapters/SKILL.md
  • .claude/settings.json
  • .codex/hooks.json
  • .pi/extensions/fm-primary-turnend-guard.ts
  • AGENTS.md
  • README.md
  • bin/backends/cmux.sh
  • bin/backends/herdr.sh
  • bin/backends/tmux.sh
  • bin/backends/zellij.sh
  • bin/fm-bearings-snapshot.sh
  • bin/fm-bootstrap.sh
  • bin/fm-busy-lib.sh
  • bin/fm-composer-lib.sh
  • bin/fm-config-inherit-lib.sh
  • bin/fm-fleet-snapshot.sh
  • bin/fm-harness.sh
  • bin/fm-herdr-session-cleanup.sh
  • bin/fm-send.sh
  • bin/fm-session-start.sh
  • bin/fm-sessionstart-run.sh
  • bin/fm-spawn.sh
  • bin/fm-teardown.sh
  • bin/fm-test-run.sh
  • bin/fm-timeout-lib.sh
  • bin/fm-vendor-auth-probe.sh
  • docs/architecture.md
  • docs/configuration.md
  • docs/documentation-audiences.json
  • docs/fork-features.md
  • docs/herdr-backend.md
  • docs/scripts.md
  • docs/sessionstart-nudge.md
  • docs/subagent-guard.md
  • docs/supervision-protocols/codex.md
  • docs/supervision-protocols/grok.md
  • docs/tmux-backend.md
  • docs/trace-context.md
  • docs/turnend-guard.md
  • docs/verification/muse.md
  • docs/verification/public-followup.md
  • docs/verification/runtime-backends.md
  • docs/verification/supervision.md
  • tests/fm-backend-herdr-focus-flash-e2e.test.sh
  • tests/fm-backend-herdr-presentation-e2e.test.sh
  • tests/fm-backend-herdr.test.sh
  • tests/fm-bootstrap.test.sh
  • tests/fm-calm-pi-extension.test.sh
  • tests/fm-composer-ghost.test.sh
  • tests/fm-composer-lib.test.sh
  • tests/fm-harness-liveness-drift-live-e2e.test.sh
  • tests/fm-herdr-version-floor-live-e2e.test.sh
  • tests/fm-muse-harness.test.sh
  • tests/fm-muse-signals-live-e2e.test.sh
  • tests/fm-remote-reply.test.sh
  • tests/fm-secondmate-harness.test.sh
  • tests/fm-session-start.test.sh
  • tests/fm-sessionstart-hook-live-e2e.test.sh
  • tests/fm-sessionstart-nudge.test.sh
  • tests/fm-teardown.test.sh
  • tests/fm-tmux-agent-liveness.test.sh

Comment thread bin/fm-composer-lib.sh
Comment on lines +220 to +221
'❯ '*|'› '*|'⟩ '*|'> '*|'$ '*|'% '*|'# '*) content=${content#??} ;;
'❯'*|'›'*|'⟩'*|'>'*|'$'*|'%'*|'#'*) content=${content#?} ;;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

LC_ALL=C bash -c '
content=$(printf "\342\237\251 ")
content=${content#??}
test -n "$content"
'

Repository: trillium/firstmate

Length of output: 156


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo '--- candidate file location and relevant lines ---'
git ls-files | grep -Fx 'bin/fm-composer-lib.sh' || true
if [ -f bin/fm-composer-lib.sh ]; then
  wc -l bin/fm-composer-lib.sh
  sed -n '190,235p' bin/fm-composer-lib.sh | cat -n
fi

Repository: trillium/firstmate

Length of output: 2406


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo '--- search for composer classification and pending state ---'
rg -n "pending|pending|composer|Muse|content=\$\{content#\?\?|content=\$\{content\#\\\\?\\?|[❯›⟩]>\\s" bin/fm-composer-lib.sh bin 2>/dev/null || true

Repository: trillium/firstmate

Length of output: 50374


Strip the Muse glyph with literal prefixes.

fm_composer_classify_content() handles ⟩ as an empty agent composer, but lines 230-231 remove the leading prompt with ${content#??} / ${content#?}. With LC_ALL=C, ${content#??} drops only the first two UTF-8 bytes of ⟩, leaving one residual byte for rows such as ⟩ Type a message...; the function then reaches printf 'pending'. Use literal-prefix stripping for each leading multibyte glyph so empty Muse composer rows stay classified as empty.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-composer-lib.sh` around lines 220 - 221, Update
fm_composer_classify_content’s leading-glyph stripping cases to remove each
multibyte Muse prompt using literal prefix patterns rather than ${content#??} or
${content#?}. Preserve the existing handling for the trailing space and ensure
rows beginning with ⟩, along with the other supported glyphs, are fully stripped
so empty composer rows remain classified as empty.

Comment thread bin/fm-session-start.sh
# mutating sweeps that startup already reconciled - the stale Herdr
# projection cleanup and bootstrap's six mutating sweeps (fleet
# sync, secondmate convergence and liveness, PR-check migration,
# pending remote handoff retry, X-mode artifact writes) - and

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use Relay in all user-facing session-start text.

Line 122 is emitted by --help. Line 365 is emitted by the context re-emit. Both still expose X-mode, but this PR renames the user-facing surface to Relay. Preserve internal x- identifiers and replace this prose with Relay artifact writes.

Proposed fix
-#             pending remote handoff retry, X-mode artifact writes) - and
+#             pending remote handoff retry, Relay artifact writes) - and
...
-  printf 'retry, X-mode artifact writes, and stale Herdr child cleanup - are NOT repeated.\n'
+  printf 'retry, Relay artifact writes, and stale Herdr child cleanup - are NOT repeated.\n'

Also applies to: 365-365

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-session-start.sh` at line 122, Update the user-facing session-start
prose at the help output and context re-emit sites to say “Relay artifact
writes” instead of “X-mode artifact writes.” Preserve all internal x-
identifiers and change only the displayed text.

Comment thread bin/fm-timeout-lib.sh
Comment on lines +59 to +84
(
set +m
sleep "$seconds"
printf 'expired\n' > "$deadline_status"
kill -TERM -- "-$child_pid" 2>/dev/null || true
sleep 0.2
kill -KILL -- "-$child_pid" 2>/dev/null || true
exit 124
) &
watchdog_pid=$!
[ "$monitor_was_on" -eq 1 ] || set +m

if wait "$child_pid" 2>/dev/null; then
command_rc=0
else
command_rc=$?
fi
if [ -s "$deadline_status" ]; then
wait "$watchdog_pid" 2>/dev/null || true
command_rc=124
else
kill -TERM -- "-$watchdog_pid" 2>/dev/null || kill "$watchdog_pid" 2>/dev/null || true
wait "$watchdog_pid" 2>/dev/null || true
recorded_rc=$(cat "$command_status" 2>/dev/null || true)
case "$recorded_rc" in ''|*[!0-9]*) ;; *) command_rc=$recorded_rc ;; esac
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find every caller of the shared bound and the sleep granularity it relies on.
rg -n 'fm_run_timed|fm_run_bash_timeout|fm-timeout-lib' bin tests

Repository: trillium/firstmate

Length of output: 2953


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== fm-timeout-lib outline =="
ast-grep outline bin/fm-timeout-lib.sh --view condensed || true

echo "== fm-timeout-lib relevant lines =="
sed -n '1,160p' bin/fm-timeout-lib.sh | cat -n

echo "== callers and tests relevant =="
rg -n "fm_run_bash_timeout|/tmp/fm-*|deadline_status|command_status|sleep 0\.2|STARTUP TRUNCATED|TRUNCATED" bin tests/fm-session-start.test.sh tests -g '*.sh' || true

Repository: trillium/firstmate

Length of output: 16862


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== fm-session-start timeout handling =="
sed -n '160,210p' bin/fm-session-start.sh | cat -n

echo "== session-start tests around truncated and healthy digest =="
sed -n '1260,1375p' tests/fm-session-start.test.sh | cat -n

echo "== fs-monotonic simulation of status precedence behavior =="
python3 - <<'PY'
def current_logic(command_exits_at, deadline_writes_expired_at, kwait_grace):
    deadline_status = (deadline_writes_expired_at <= command_exits_at)
    recorded_rc = str(command_exits_at) if command_exits_at >= 0 else None
    if deadline_status:
        return recorded_rc, "124", "deadline_precedence"
    return recorded_rc, recorded_rc if recorded_rc.isdigit() else None, "recorded_precedence"

print("current: command exits 3s, deadline expired writes 3s:", current_logic(3, 3, 0.2))
print("proposed: command exits 3s, deadline expired writes 3s:", (str(3), str(3), "recorded_precedence"))

print("current: command exits 3s + epsilon, deadline expired writes 3s:", current_logic(3.001, 3, 0.2))
print("proposed: command exits 3s + epsilon, deadline expired writes 3s:", (str(int(3)), str(3), "recorded_precedence"))
PY

echo "== documentation/config references for host support =="
rg -n "macOS|GNU coreutils|coreutils|sleep 0|sleep\.|bash fallback|fm-run-bash|FM_TIMEOUT_MECHANISM_OVERRIDE|FM_TIMEOUT" README.md bin tests -g '*.md' -g '*.sh' | head -200 || true

Repository: trillium/firstmate

Length of output: 22346


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== check for vendor-auth timeout usage =="
sed -n '120,170p' bin/fm-vendor-auth-probe.sh | cat -n

echo "== status marker timestamp scenario =="
python3 - <<'PY'
import time
from pathlib import Path
import tempfile

with tempfile.TemporaryDirectory() as d:
    p = Path(d) / "status"
    dline = Path(d) / "status.deadline"

    def now(): return time.time()
    start = now()
    while now() - start < 2.55:
        pass
    command_rc = 0
    p.write_text(str(command_rc))
    deadline_writes_expired = time.time()
    dead = deadline_writes_expired <= (command_ends := time.time())
    print(f"command_exits_after_deadmark={'yes' if dead else 'no'} command_rc={command_rc} current_would_be_124={'yes' if dline.exists() else 'no'}")
PY

Repository: trillium/firstmate

Length of output: 2506


Keep the KILL grace period POSIX-compatible.

The Bash fallback exits with 124 if deadline_status exists before reading command_status, so a healthy digest can print STARTUP TRUNCATED when the watchdog writes its marker close to the command’s last write. Prefer the recorded command status, then fall back to 124.

The sleep 0.2 interval is not POSIX. If Bash is supported without GNU coreutils or a local package, replace it with an integer sleep plus a portable busy-wait or accept that strict POSIX hosts will skip the grace period.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-timeout-lib.sh` around lines 59 - 84, Update the watchdog completion
logic to read and validate command_status before deciding the result from
deadline_status, preferring the recorded command status and falling back to 124
only when no valid status exists. Replace the fractional sleep 0.2 in the KILL
grace period with an integer-second sleep plus a portable busy-wait, or
otherwise skip the grace period on strict POSIX hosts.

The Guard Predicates section of [`turnend-guard.md`](turnend-guard.md#guard-predicates) owns marker validation, plain-checkout detection, and required Firstmate-shaped paths.

Before printing, the wrapper reads `state/.lock` and walks at most eight parents from its own pid in its own separate, hard-coded loop, independent of `bin/fm-lock.sh`'s ancestry walk (`fm_harness_ancestry_pid()` in `bin/fm-session-lock-lib.sh`, which now walks up to sixteen parents and can extend past a claude-named match to a still-more-ancestral one) and of Pi's `lockOwnership()`.
The nudge payload starts with U+2063 and the stable `FIRSTMATE_OP: ` label, carries the current `session-start` protocol kind, and retains exactly ``Run `bin/fm-session-start.sh` now, exactly once, before executing any other instructions.`` as its body.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the MD038 code-span lint on this line.

markdownlint-cli2 reports MD038 no-space-in-code for the code span on this line. Pad the double-backtick span with a space on each side so the inner backticks and punctuation stay literal and the span boundary is unambiguous.

📝 Proposed fix
-The nudge payload starts with U+2063 and the stable `FIRSTMATE_OP: ` label, carries the current `session-start` protocol kind, and retains exactly ``Run `bin/fm-session-start.sh` now, exactly once, before executing any other instructions.`` as its body.
+The nudge payload starts with U+2063 and the stable `FIRSTMATE_OP:` label, carries the current `session-start` protocol kind, and retains exactly `` `Run bin/fm-session-start.sh now, exactly once, before executing any other instructions.` `` as its body.

Keep the trailing space in the label only if the emitted payload really contains it; if it does, write it as `FIRSTMATE_OP: ` with padding instead.

🧰 Tools
🪛 markdownlint-cli2 (0.23.2)

[warning] 55-55: Spaces inside code span elements

(MD038, no-space-in-code)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/sessionstart-nudge.md` at line 55, Update the code span in the
session-start nudge documentation to add one space immediately inside both
double-backtick delimiters, preserving the literal inner backticks and
punctuation. Keep the trailing space in the `FIRSTMATE_OP: ` label only if it is
part of the emitted payload, and ensure the rendered text remains unchanged
apart from the lint-required span padding.

Source: Linters/SAST tools

Comment on lines +2865 to 2870
"$ROOT/bin/fm-sessionstart-run.sh" \
"$ROOT/bin/fm-sessionstart-nudge.sh" \
"$ROOT/bin/fm-primary-scope-lib.sh" \
"$ROOT/bin/fm-gate-refuse-lib.sh" \
"$ROOT/bin/fm-operational-input.sh" \
"$project/bin/"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Copy fm-session-lock-lib.sh with the session-start runner.

The copied bin/fm-sessionstart-run.sh unconditionally sources fm-session-lock-lib.sh. This cp command omits that file. The fixture therefore emits a missing-file error and leaves the runner's lock functions unavailable. Add the missing helper to keep the E2E setup self-contained.

Proposed fix
  cp \
    "$ROOT/bin/fm-sessionstart-run.sh" \
+   "$ROOT/bin/fm-session-lock-lib.sh" \
    "$ROOT/bin/fm-sessionstart-nudge.sh" \
    "$ROOT/bin/fm-primary-scope-lib.sh" \
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"$ROOT/bin/fm-sessionstart-run.sh" \
"$ROOT/bin/fm-sessionstart-nudge.sh" \
"$ROOT/bin/fm-primary-scope-lib.sh" \
"$ROOT/bin/fm-gate-refuse-lib.sh" \
"$ROOT/bin/fm-operational-input.sh" \
"$project/bin/"
"$ROOT/bin/fm-sessionstart-run.sh" \
"$ROOT/bin/fm-session-lock-lib.sh" \
"$ROOT/bin/fm-sessionstart-nudge.sh" \
"$ROOT/bin/fm-primary-scope-lib.sh" \
"$ROOT/bin/fm-gate-refuse-lib.sh" \
"$ROOT/bin/fm-operational-input.sh" \
"$project/bin/"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-calm-pi-extension.test.sh` around lines 2865 - 2870, Add
fm-session-lock-lib.sh to the copy list alongside fm-sessionstart-run.sh in the
fixture setup, ensuring the session-start runner can source its lock helper and
retain the expected lock functions.

Comment on lines +383 to +392
test_run_reports_a_failed_session_start_as_digest_text() {
local root="$TMP_ROOT/run-unwritable" out status=0
make_run_primary "$root"
chmod 0500 "$root/state"
out=$(run_hook "$root" --source startup </dev/null) || status=$?
chmod 0700 "$root/state"
expect_code 0 "$status" "run wrapper with an unwritable state directory"
assert_contains "$out" "READ-ONLY SESSION" "a failed lock did not reach the agent as digest text"
pass "run wrapper: a session start that cannot take the lock still opens the session and says so"
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Skip this case when the test user can write through mode 0500.

Line 386 relies on chmod 0500 to block the lock write. Root ignores directory write bits, so the session start succeeds, the digest omits READ-ONLY SESSION, and line 390 fails. Many CI containers run as root. Add a writability probe and skip the case when the mode does not deny the write.

🛡️ Proposed guard
   make_run_primary "$root"
   chmod 0500 "$root/state"
+  if : > "$root/state/.write-probe" 2>/dev/null; then
+    rm -f "$root/state/.write-probe"
+    chmod 0700 "$root/state"
+    echo "skip: this user writes through mode 0500, so an unwritable state directory cannot be simulated"
+    return 0
+  fi
   out=$(run_hook "$root" --source startup </dev/null) || status=$?
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test_run_reports_a_failed_session_start_as_digest_text() {
local root="$TMP_ROOT/run-unwritable" out status=0
make_run_primary "$root"
chmod 0500 "$root/state"
out=$(run_hook "$root" --source startup </dev/null) || status=$?
chmod 0700 "$root/state"
expect_code 0 "$status" "run wrapper with an unwritable state directory"
assert_contains "$out" "READ-ONLY SESSION" "a failed lock did not reach the agent as digest text"
pass "run wrapper: a session start that cannot take the lock still opens the session and says so"
}
test_run_reports_a_failed_session_start_as_digest_text() {
local root="$TMP_ROOT/run-unwritable" out status=0
make_run_primary "$root"
chmod 0500 "$root/state"
if : > "$root/state/.write-probe" 2>/dev/null; then
rm -f "$root/state/.write-probe"
chmod 0700 "$root/state"
echo "skip: this user writes through mode 0500, so an unwritable state directory cannot be simulated"
return 0
fi
out=$(run_hook "$root" --source startup </dev/null) || status=$?
chmod 0700 "$root/state"
expect_code 0 "$status" "run wrapper with an unwritable state directory"
assert_contains "$out" "READ-ONLY SESSION" "a failed lock did not reach the agent as digest text"
pass "run wrapper: a session start that cannot take the lock still opens the session and says so"
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-sessionstart-nudge.test.sh` around lines 383 - 392, Add a
writability probe in test_run_reports_a_failed_session_start_as_digest_text
after chmod 0500 and before run_hook; if the test user can still create/write in
$root/state, restore permissions and skip the test. Keep the existing assertions
for environments where mode 0500 successfully blocks writes.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants