diff --git a/.gitignore b/.gitignore index edf05b424a..d9971300e4 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ __pycache__/ config/ .tools/ +.stow-notes.md diff --git a/.no-mistakes.yaml b/.no-mistakes.yaml index 10a1c9bef2..b20759c3d6 100644 --- a/.no-mistakes.yaml +++ b/.no-mistakes.yaml @@ -9,6 +9,9 @@ # HEAD-continuity guard; see docs/architecture.md "No-mistakes gate authority boundary." disable_project_settings: true +jev: + review_assist: true + # Trusted documentation placement policy for the Document step. # The audience inventory and coding guideline own the detail; keep this as a # pointer so gate instructions cannot become a second prose policy. diff --git a/.omo/evidence/security-privacy-gate-review.md b/.omo/evidence/security-privacy-gate-review.md new file mode 100644 index 0000000000..58c577a17c --- /dev/null +++ b/.omo/evidence/security-privacy-gate-review.md @@ -0,0 +1,35 @@ +# Security and privacy gate review + +- recommendation: REJECT +- originalIntent: Review the branch range `1bb72cc5f88014c86e3d03244efa0bb26c22d001..e9e9ec8e772fcc588d59ca751e9441a49d147d5e` for reachable auth, secret, protected-data, command/path, or disclosure defects in the Jev, Discord relay, wake/state, and external-command boundaries. +- desiredOutcome: Only the operator/captain can supply authority-bearing Discord instructions; Jev and persisted state do not disclose protected data; external command paths cannot be redirected or injected. +- userOutcomeReview: The self-hosted Discord path violates its downstream owner-only trust contract. With no channel allowlist configured, the poller enumerates accessible guild channels and DMs, accepts any non-bot author who mentions or DMs the bot, and emits the same `x-mention` payload consumed by `fmx-respond`. That consumer explicitly treats every direct author as the captain and autonomously performs normal lifecycle work. No author-id or role check exists in the introduced poller. + +## Blockers + +1. violatedCriterion: SEC-AUTH-1 — identify an exact reachable authorization bypass and consequence + - severity: HIGH + - evidencePointer: `bin/fm-discord-poll.js:66-98,116-149,182-192`; `.agents/skills/fmx-respond/SKILL.md:26-44,70-80`; `docs/configuration.md:689-699` + - observation: Any Discord user able to DM the bot or mention it in an accessible channel is converted into a captain-authorized request. The default configuration scans guild channels and enables DMs, while the poller checks only `author.bot`, mention/DM status, and channel exclusion. The shared response skill then treats the direct author as the captain and may file work, dispatch agents, investigate, or ship gated changes. + - bypass: Send a DM to the bot, or mention it in any channel visible to the bot, without being the operator/captain. + - consequence: An untrusted Discord user gains authority to trigger autonomous replies and normal reversible lifecycle actions on the operator's machine; the system may also expose public-safe operational outcomes to that user. + +## Notes + +- Jev: tracked `.no-mistakes.yaml` sets `jev.review_assist`, but the branch documentation states this key is global-only and ignored from repository config. No reachable new Jev disclosure path was established from this repository change. +- Wake/state and external command paths: no additional source-backed security or privacy finding was established in the reviewed changes. +- remove-ai-slops/programming direct pass: The Discord boundary uses no author authentication and relies on a downstream hosted-Relay invariant that the self-hosted adapter does not establish. Tests cover channel selection, DM defaults, and delivery, but do not prove an owner identity boundary. No slop-only concern is promoted as a blocker. + +## Checked artifacts + +- Diff and history: `1bb72cc5f88014c86e3d03244efa0bb26c22d001..e9e9ec8e772fcc588d59ca751e9441a49d147d5e`, especially commits `5879ec3` and `6f2a79b` +- Source: `bin/fm-discord-{lib,poll,reply}.{sh,js}`, `bin/fm-{bootstrap,watch,x-reply,x-dismiss,spawn,control,crew-state,wake-lib,session-lock-lib}.sh`, `.pi/extensions/*.ts` +- Policy/call contract: `.agents/skills/fmx-respond/SKILL.md`, `AGENTS.md`, `docs/configuration.md` +- Tests read: `tests/fm-discord-selfhosted.test.sh`, relevant `tests/fm-x-mode.test.sh` request-id and reply cases +- Existing evidence read: `.omo/evidence/*gate-review*.md` + +## Exact evidence gaps + +- Tests were not run, as explicitly prohibited by the assignment. +- No live Discord API call was made. Reachability is established from the poller's source and the downstream instruction contract. +- No ulw-loop plan exists; `omo ulw-loop status --json` returned `ULW_LOOP_PLAN_MISSING`, so this fallback report path is used. diff --git a/AGENTS.md b/AGENTS.md index 1dfdf6214b..6da664f15b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,7 +59,7 @@ Tracked files hold shared instructions and tooling; `data/` holds durable privat Load `firstmate-layout` before reasoning about any more specific path. ``` -AGENTS.md this file (CLAUDE.md is a real @AGENTS.md pointer to it) +AGENTS.md this file CONTRIBUTING.md contributor workflow and repo conventions README.md public overview and development notes .github/workflows/ shared CI and PR enforcement, committed @@ -458,7 +458,7 @@ Each skill owns its own daemon procedure, which is otherwise identical; these sa - Every current daemon injection uses the `away-supervisor` kind from `bin/fm-operational-input.sh` after `FM_OPERATIONAL_PREFIX` (U+2063 INVISIBLE SEPARATOR followed by `FIRSTMATE_OP: `), while the `/afk` skill owns legacy bare-marker compatibility. - `state/.afk-contract` is the away posture, written only after the captain confirms the read-back of their away words; entry announces hold-for-return only, and the record's clauses are recorded, not executed, in this release. -- While `state/.afk` exists, the daemon owns supervision; do not arm a separate watcher. +- On harnesses that launch the away daemon, while `state/.afk` exists, the daemon owns supervision; do not arm a separate watcher. The daemon is never launched on Pi, where the ordinary supervision session continues under the record with main parked: the branch takes every safe actionable wake it can, and only a declined wake (including a broken branch or unsafe scan) or a watcher failure wakes main. - A marked message while away or quiet mode is active is internal escalation and does not exit that mode. - A message beginning `/afk` refreshes away mode; a message beginning `/quiet` refreshes quiet mode. diff --git a/bin/backends/cmux.sh b/bin/backends/cmux.sh index 747d3fc2cc..836d0ac8e5 100644 --- a/bin/backends/cmux.sh +++ b/bin/backends/cmux.sh @@ -632,6 +632,21 @@ fm_backend_cmux_kill() { # [unused] [expected-label] fm_backend_cmux_cli close-workspace --workspace "$wsid" >/dev/null 2>&1 || true } +fm_backend_cmux_endpoint_confirmed_gone() { + local target=$1 expected_label=${3:-} workspaces workspace_count expected_title + [ -n "$expected_label" ] || return 1 + fm_backend_cmux_parse_target "$target" || return 1 + workspaces=$(fm_backend_cmux_cli workspace list --json --id-format uuids 2>/dev/null) || return 1 + expected_title=$(fm_backend_cmux_scoped_title "$expected_label") + workspace_count=$(printf '%s' "$workspaces" | jq -er --arg title "$expected_title" \ + '[.workspaces[]? | select(.title == $title)] | length' 2>/dev/null) || return 1 + [ "$workspace_count" -eq 0 ] || return 1 + workspace_count=$(printf '%s' "$workspaces" | jq -er --arg w "$FM_BACKEND_CMUX_WORKSPACE" \ + '[.workspaces[]? | select(.id == $w)] | length' 2>/dev/null) || return 1 + [ "$workspace_count" -eq 0 ] && return 0 + return 1 +} + # fm_backend_cmux_list_live: recovery/orphan discovery. Lists every workspace # whose title is scoped to this firstmate home, by TITLE - never by trusting a # stored uuid, since workspace ids do NOT survive an app relaunch (finding #5). diff --git a/bin/backends/herdr.sh b/bin/backends/herdr.sh index b836b77201..10608d33ab 100644 --- a/bin/backends/herdr.sh +++ b/bin/backends/herdr.sh @@ -3392,11 +3392,12 @@ fm_backend_herdr_kill() { # done fi if [ "$lock_held" = 1 ]; then - fm_backend_herdr_kill_serialized "$session" "$pane" + fm_backend_herdr_kill_serialized "$session" "$pane" || true fm_lock_release "$lock_path" || true else echo "warning: herdr task kill could not acquire its session presentation lock; refusing an unlocked pane close" >&2 fi + return 0 } # fm_backend_herdr_endpoint_confirmed_gone: gate durable-record removal on diff --git a/bin/backends/orca.sh b/bin/backends/orca.sh index ffea7bdfad..26b0baff2e 100644 --- a/bin/backends/orca.sh +++ b/bin/backends/orca.sh @@ -284,15 +284,19 @@ fm_backend_orca_send_text_submit() { # fm_backend_orca_tool_check || return 1 orca terminal close --terminal "$1" --json >/dev/null 2>&1 || true } + +fm_backend_orca_endpoint_confirmed_gone() { + local output + fm_backend_orca_tool_check || return 1 + output=$(orca terminal read --terminal "$1" --limit 1 --json 2>&1) || true + printf '%s' "$output" | node -e ' +const fs = require("fs"); +let value; +try { value = JSON.parse(fs.readFileSync(0, "utf8")); } catch (_) { process.exit(1); } +process.exit(value && value.ok === false && value.error && value.error.code === "terminal_handle_stale" ? 0 : 1); +' +} diff --git a/bin/backends/tmux.sh b/bin/backends/tmux.sh index 2bc1c8aa0d..9634c0978d 100644 --- a/bin/backends/tmux.sh +++ b/bin/backends/tmux.sh @@ -203,6 +203,20 @@ fm_backend_tmux_kill() { # return 1 } +fm_backend_tmux_endpoint_confirmed_gone() { + local target=$1 session window windows inventory_status + case "$target" in + *:*) session=${target%%:*}; window=${target#*:} ;; + *) return 1 ;; + esac + case "$session:$window" in :*|*:|*:*:*) return 1 ;; esac + windows=$(fm_backend_tmux_window_inventory "=$session") + inventory_status=$? + [ "$inventory_status" -eq 2 ] && return 0 + [ "$inventory_status" -eq 0 ] || return 1 + ! printf '%s\n' "$windows" | grep -qxF -- "$window" +} + # fm_backend_tmux_current_command: 's live foreground process name - # tmux's own `#{pane_current_command}`, already resolved from the pty's # foreground process group (verified empirically with real tmux 3.6a: a diff --git a/bin/backends/zellij.sh b/bin/backends/zellij.sh index a90247899f..f9e7d0293d 100644 --- a/bin/backends/zellij.sh +++ b/bin/backends/zellij.sh @@ -621,6 +621,23 @@ fm_backend_zellij_kill() { # [tab_id] [expected_label] fi } +fm_backend_zellij_endpoint_confirmed_gone() { + local target=$1 expected_label=${3:-} sessions panes count tabs scoped + fm_backend_zellij_parse_target "$target" || return 1 + sessions=$(zellij list-sessions --short --no-formatting 2>/dev/null) || return 1 + printf '%s\n' "$sessions" | grep -qxF -- "$FM_BACKEND_ZELLIJ_SESSION" || return 0 + [ -n "$expected_label" ] || return 1 + scoped=$(fm_backend_zellij_scoped_title "$expected_label") + tabs=$(fm_backend_zellij_cli "$FM_BACKEND_ZELLIJ_SESSION" action list-tabs --json 2>/dev/null) || return 1 + count=$(printf '%s' "$tabs" | jq -er --arg scoped "$scoped" --arg bare "$expected_label" \ + '[.[]? | select(.name == $scoped or .name == $bare)] | length' 2>/dev/null) || return 1 + [ "$count" -eq 0 ] || return 1 + panes=$(fm_backend_zellij_cli "$FM_BACKEND_ZELLIJ_SESSION" action list-panes --json 2>/dev/null) || return 1 + count=$(printf '%s' "$panes" | jq -er --argjson p "$FM_BACKEND_ZELLIJ_PANE" \ + '[.[]? | select(.id == $p and .is_plugin == false)] | length' 2>/dev/null) || return 1 + [ "$count" -eq 0 ] +} + # fm_backend_zellij_list_live: recovery/orphan discovery. Lists every tab in # whose title carries THIS firstmate home's own tag # (fm--, fm_backend_zellij_home_label) - never any other home's diff --git a/bin/fm-backend.sh b/bin/fm-backend.sh index 5d34e8bb15..42e590522a 100644 --- a/bin/fm-backend.sh +++ b/bin/fm-backend.sh @@ -795,14 +795,10 @@ fm_backend_send_text_submit() { # - local backend=$1 + local backend=$1 kill_status shift [ -n "${1:-}" ] || { echo "error: refusing empty backend kill target" >&2; return 1; } fm_backend_source "$backend" || return 1 @@ -814,6 +810,25 @@ fm_backend_kill() { # cmux) fm_backend_cmux_kill "$@" ;; *) echo "error: no kill implementation for backend '$backend'" >&2; return 1 ;; esac + kill_status=$? + [ "$kill_status" -eq 0 ] || return 1 + fm_backend_endpoint_confirmed_gone "$backend" "$@" && return 0 + return 1 +} + +fm_backend_endpoint_confirmed_gone() { + local backend=$1 + shift + local helper="fm_backend_${backend}_endpoint_confirmed_gone" + declare -F "$helper" >/dev/null 2>&1 || return 0 + case "$backend" in + tmux) fm_backend_tmux_endpoint_confirmed_gone "$@" ;; + herdr) fm_backend_herdr_endpoint_confirmed_gone "$@" ;; + zellij) fm_backend_zellij_endpoint_confirmed_gone "$@" ;; + orca) fm_backend_orca_endpoint_confirmed_gone "$@" ;; + cmux) fm_backend_cmux_endpoint_confirmed_gone "$@" ;; + *) return 1 ;; + esac } fm_backend_remove_worktree() { # diff --git a/bin/fm-bootstrap.sh b/bin/fm-bootstrap.sh index e6c8aaa7d5..39d71feaee 100755 --- a/bin/fm-bootstrap.sh +++ b/bin/fm-bootstrap.sh @@ -826,7 +826,20 @@ secondmate_liveness_one() { # dead|missing) if [ "$agent_state" = dead ]; then cause="confirmed agent absence on existing endpoint" - fm_backend_kill "$backend" "$target" 2>/dev/null || true + case "$backend" in + zellij) + fm_backend_kill "$backend" "$target" "$(fm_meta_get "$meta" zellij_tab_id)" "fm-$id" 2>/dev/null + ;; + cmux) + fm_backend_kill "$backend" "$target" '' "fm-$id" 2>/dev/null + ;; + *) + fm_backend_kill "$backend" "$target" 2>/dev/null + ;; + esac || { + echo "SECONDMATE_LIVENESS: secondmate $id: skipped: endpoint cleanup could not be confirmed (backend=$backend)" + return 0 + } else cause="recorded endpoint confidently missing" fi diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index 688e47d34c..2e97033179 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -1644,7 +1644,7 @@ families_for_changed_path() { tests/*) printf '%s\n' "__unmapped__:$path" ;; - README.md|LICENSE|assets/*|docs/*|.gitignore) + README.md|LICENSE|assets/*|docs/*|.omo/evidence/*|.gitignore) ;; *) if [ -e "$path" ]; then diff --git a/docs/architecture.md b/docs/architecture.md index 5edd2123d1..eb3c04477b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -59,7 +59,7 @@ A busy pane is otherwise exempt from staleness, but only until its last complete A crew that declared an external wait (`paused:`) or a verified captain-held transfer is the first exception to that bound: its busy verdict supplies liveness while identifying the long-running foreground call as the declared wait, so it takes the bounded `FM_PAUSE_RESURFACE_SECS` recheck instead of a wedge escalation, except that a captain-held transfer is not rechecked while the away-posture record exists. In a home that armed `config/wedge-defer-parked-gate`, a crew whose own validation gate awaits the supervisor's still-open decision for that run is the second, reached through the shared wedge timer rather than the declaration branch, because who owes that answer does not depend on what the pane is rendering; it takes the same bounded recheck, including while the away-posture record exists. Lifting the declaration restores the unchanged busy-pane wedge path, while a pane that is no longer busy returns to the existing idle declared-wait classification. -While the legacy daemon flag is active, a busy pane that crosses the bound under a declared external wait is handed to the daemon as the plain wake identity instead of taking that recheck in the watcher, because the daemon owns triage there and a wake already decorated as a possible wedge would override the daemon's own declared-wait verdict; an undeclared busy pane past the bound still takes the wedge escalation. +On a harness that runs the away daemon, while the legacy daemon flag is active, a busy pane that crosses the bound under a declared external wait is handed to the daemon as the plain wake identity instead of taking that recheck in the watcher, because the daemon owns triage there and a wake already decorated as a possible wedge would override the daemon's own declared-wait verdict; an undeclared busy pane past the bound still takes the wedge escalation. That handoff is keyed on the declaration itself (the status log's signature) rather than on the pane capture, so a harness footer that ticks on every poll wakes the daemon once per declaration instead of once per poll, and it clears the wedge timer, escalation count, and worktree-write deferral exactly as the normal-mode absorber does, so an undeclared busy phase's timer does not resume when the declaration lifts. Those actionable wakes are written to a durable local queue (`state/.wake-queue`) only after generation-bound recovery evidence is published, so an interrupted watcher or handling turn can be recovered without losing the queue record. Agent endpoint liveness and queue-consumption liveness are separate: on each poll, the primary watcher reads the oldest valid actionable row from every endpoint-recorded local secondmate home's durable wake queue without locking, consuming, or rewriting that foreign queue. diff --git a/docs/cmux-backend.md b/docs/cmux-backend.md index 51618de5b5..ad820a0ac3 100644 --- a/docs/cmux-backend.md +++ b/docs/cmux-backend.md @@ -108,6 +108,7 @@ The sibling never carries an `fm-` title and is ignored by recovery. The exact window membership is re-read before this operation. A selected workspace that is not last closes normally; selection itself is not the trigger. Firstmate does not attempt to close the macOS window because cmux's socket cannot close a window holding a live terminal. +The shared cleanup path also requires a post-close proof that the recorded workspace and scoped task workspace are absent; an unreadable or ambiguous proof fails closed and retains the task identity. Real tests share the captain's running app rather than creating an isolated cmux session. `tests/cmux-test-safety.sh` permits cleanup only for an exact currently listed `fm-test-` workspace and never enumerates and closes unrelated workspaces or relaunches the app. diff --git a/docs/configuration.md b/docs/configuration.md index 0e754c0d01..08db71e347 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -231,7 +231,8 @@ The flag is a home-local supervision-noise preference and is not inherited by se ## Gate defaults (.no-mistakes.yaml) -The tracked `.no-mistakes.yaml` sets `test.evidence.store_in_repo: true` and pins `commands.lint` to `bin/fm-lint.sh`, the same owner CI invokes. +The tracked `.no-mistakes.yaml` enables `jev.review_assist: true` for advisory pre-brief context in the review step only, sets `test.evidence.store_in_repo: true`, and pins `commands.lint` to `bin/fm-lint.sh`, the same owner CI invokes. +The existing review remains the merge-gate decision point; Jev does not gate delivery. Storing evidence in the repo publishes each run's test artifacts to the orphan `no-mistakes/evidence` branch and links them from the PR body, instead of keeping them on local disk under the no-mistakes home. That branch shares no history with code branches, so evidence never enters a pushed feature branch or the default branch; the worktree's `.no-mistakes/` stays local and CI rejects tracked entries under that path. The [`firstmate-coding-guidelines` skill](../.agents/skills/firstmate-coding-guidelines/SKILL.md#no-mistakes-test-configuration) owns why `commands.test` stays absent and targeted validation belongs to the evidence path. @@ -239,6 +240,19 @@ The [`firstmate-coding-guidelines` skill](../.agents/skills/firstmate-coding-gui See [CONTRIBUTING.md](../CONTRIBUTING.md) for the firstmate-specific local test policy and entry points. Portable shard evidence and coverage rules are in [fm-test-portable-shards.md](fm-test-portable-shards.md); [herdr-backend.md](herdr-backend.md#destructive-lab-safety) owns the real-Herdr lane's isolation boundary, and [runtime-backends.md](verification/runtime-backends.md#herdr) owns active evidence. +## Jev review assist (optional, per-operator) + +no-mistakes v1.79.0+ can send each review turn's diff to TypeSafe's Jev model to rank which surrounding files are worth reading first; the ranked list only enriches the review prompt, the ordinary cold complete review remains authoritative, and Jev is advisory only and never gates delivery. +This repository opts in through its tracked `.no-mistakes.yaml`; the operator still needs `TYPESAFE_API_KEY` set in the daemon's environment. Without a key the step log says so and review proceeds without a pre-brief, exactly as when the setting is off. +On every call failure, oversized reply, or missing key, no-mistakes falls back to the ordinary cold review with no pre-brief - this repo does not depend on Jev being reachable. + +**What is sent.** Per review turn, no-mistakes sends only the diff of reviewable files plus up to 40 candidate file paths - paths only, never their content - ranked by name rarity and directory proximity. No project name, PR body, or brief text is part of this call. + +**Data boundary.** The review step selects reviewable files according to its diff and ignore rules; paths matched by those rules are not sent to Jev. Gitignored status alone is not a guarantee, because tracked or force-added files can still be reviewable. +The remaining operator responsibility is ordinary git hygiene: keep secrets out of tracked files, since no-mistakes has no Jev-specific secret redaction beyond the review step's existing findings pipeline. + +**Audit.** The review step log already records whether a pre-brief was requested, whether it was used, and the reason for any fallback (`no-mistakes axi logs --step review --full`); no separate Jev-specific audit log exists in this repo, since the call itself happens inside the no-mistakes daemon process, outside firstmate's own scripts. + ## Captain Preferences (data/captain.md / data/captain-shared.md) Domain-local preferences for one captain's fleet live locally in each home's `data/captain.md`; it is gitignored and printed in the session-start context digest after `data/projects.md` and optional `data/secondmates.md`. diff --git a/docs/documentation-audiences.json b/docs/documentation-audiences.json index 3ce4890222..7957e5fa06 100644 --- a/docs/documentation-audiences.json +++ b/docs/documentation-audiences.json @@ -304,6 +304,10 @@ "path": ".omo/evidence/pre-push-gate-review.md", "audience": "maintainer-verification" }, + { + "path": ".omo/evidence/security-privacy-gate-review.md", + "audience": "maintainer-verification" + }, { "path": "AGENTS.md", "audience": "agent-runtime" diff --git a/docs/turnend-guard.md b/docs/turnend-guard.md index c932eacf6a..3eb835461c 100644 --- a/docs/turnend-guard.md +++ b/docs/turnend-guard.md @@ -15,7 +15,7 @@ Do not infer this guard's scope, loop safety, or compatibility tradeoffs for tho The turn-end guard closes the remaining gap at the primary's own turn boundary. When work, a process-event source, a registered custom check, or Relay polling needs supervision at that boundary and no identity-matched watcher has a fresh beacon, the harness integration must either block the turn end or force one bounded follow-up that uses the recovery instruction from the emitted session-start protocol. The mid-turn pull warning uses the model-aware supervision verdict described below, while the turn-end guard keeps the PID-strict watcher predicate. -Away and quiet mode are the one place the turn-end guard accepts a different supervisor: while `state/.afk` exists, in either mode (`bin/fm-wake-lib.sh`'s `fm_afk_mode`), the daemon owns supervision, so a live identity-matched daemon with a fresh beacon satisfies that boundary in place of a watcher process holding the lock. +Away and quiet mode are the one place the turn-end guard accepts a different supervisor: on harnesses that run the away daemon, while `state/.afk` exists in either mode (`bin/fm-wake-lib.sh`'s `fm_afk_mode`), the daemon owns supervision, so a live identity-matched daemon with a fresh beacon satisfies that boundary in place of a watcher process holding the lock. The guard remains a backstop; [`watcher-continuity.md`](watcher-continuity.md) owns normal continuity. ## Guard predicates @@ -52,7 +52,7 @@ Without that proof an unheld lock alarms exactly as it did before, so an unloade Under every persistent-watcher harness a live identity-matched watcher with a fresh beacon is still required, so the pull guard keeps the same strict semantics there. Its banner names the true failing condition, either a missing live watcher process or a genuinely stale beacon with its real age, and keys the once-per-episode dedup on that condition rather than the beacon mtime. -While `state/.afk` exists the daemon (`bin/fm-supervise-daemon.sh`) owns supervision and runs the watcher one-shot, in either away or quiet mode: the watcher exits on every wake and the daemon starts its replacement, so a turn boundary regularly lands in a hand-off where no watcher process holds the lock and nothing is wrong. +On harnesses that run the away daemon, while `state/.afk` exists the daemon (`bin/fm-supervise-daemon.sh`) owns supervision and runs the watcher one-shot, in either away or quiet mode: the watcher exits on every wake and the daemon starts its replacement, so a turn boundary regularly lands in a hand-off where no watcher process holds the lock and nothing is wrong. The turn-end guard therefore accepts `fm_afk_daemon_owns_supervision` from `bin/fm-wake-lib.sh` as proof of supervision on that path: `state/.afk` must exist (the predicate does not distinguish away from quiet mode), and this home's `state/.supervise-daemon.lock` must name a live pid whose current process identity still matches the identity the daemon recorded for itself. That is the same identity discipline the watcher lock uses, so a recycled pid, a lock left behind by a killed daemon, and a daemon that never recorded its identity all fail it. A daemon that cannot record its own identity at startup logs a warning and keeps running, because a supervisor must not refuse to run over an unreadable `ps`; that warning is what names the cause when the guard then keeps blocking away/quiet-mode turn boundaries for the rest of that daemon's life. diff --git a/docs/verification/process-event-sources.md b/docs/verification/process-event-sources.md index c88b1ffe6b..e54ca571f5 100644 --- a/docs/verification/process-event-sources.md +++ b/docs/verification/process-event-sources.md @@ -229,4 +229,4 @@ The optional `self-announcing` declaration changes ordering only for an adapter Proactive delivery is inside that same boundary. The watcher reports a queued process-event result through the one shared actionable-exit path (`wake` in `bin/fm-push-transition-lib.sh`) that every existing signal, stale, and check wake already uses, so it reads no pane, queries no backend, and names no harness. Both axes are therefore unaffected by construction rather than by assumption: every supported primary harness re-arms from that same exit, and every runtime backend supplies endpoint state only to the pane paths this change does not touch. -While `state/.afk` exists the watcher stays one-shot as before, because this delivery ends the cycle exactly like the existing check path and leaves classification to the daemon. +On harnesses that run the away daemon, while `state/.afk` exists the watcher stays one-shot as before, because this delivery ends the cycle exactly like the existing check path and leaves classification to the daemon. diff --git a/docs/verification/runtime-backends.md b/docs/verification/runtime-backends.md index 2ec6d91f5b..02e7b7aa4b 100644 --- a/docs/verification/runtime-backends.md +++ b/docs/verification/runtime-backends.md @@ -358,13 +358,16 @@ The refusal is reached only through a close that could not do its job, and each | Backend | already gone | a close that failed | | --- | --- | --- | | tmux | 0, silent | 1, resolved by re-reading the window's exact recorded identity; a read that itself could not run refuses rather than passing for absence | -| orca | 0, silent | 1 when a missing CLI means no close was attempted; 0 for a close command that failed after the CLI accepted it | -| zellij | 0, silent | 0, not yet distinguishable | -| cmux | 0, silent | 0, not yet distinguishable | -| herdr | 0, silent | 0 from this arm; `bin/fm-teardown.sh` gates every Herdr record removal on `fm_backend_herdr_endpoint_confirmed_gone` instead | - -The three arms that still report 0 need a presence re-read taken after their own close, and the close-then-read timing that re-read depends on cannot be established without the real Zellij, Orca, and cmux binaries. -Guessing it is what a refusal must never rest on: a gate that refused an already-exited session would break ordinary cleanup on every task, which is a worse failure than the stranded endpoint it would be trying to prevent. +| orca | 0, silent when the terminal handle is definitively stale | 1 when the CLI, close, or stale-handle re-read cannot prove absence | +| zellij | 0, silent when the session is absent | 1 when the close or scoped tab/pane proof cannot prove absence | +| cmux | 0, silent when the recorded workspace is absent | 1 when the close or scoped workspace proof cannot prove absence | +| herdr | 0, silent when the exact recorded pane is confirmed dead | 1 when the exact pane presence is unknown or live | + +Every arm now performs its backend-specific absence proof after close. +Zellij requires the recorded task label and confirms that both the scoped task tab and recorded pane are absent. +cmux requires the recorded task label and confirms that both the scoped task workspace and recorded workspace are absent. +Orca accepts only the documented stale-handle read result. +Any missing, unreadable, or ambiguous proof returns 1 rather than passing for absence. tmux's re-read is deliberately exact - `=session` plus a whole-line window-name match - because a prefix match would read a neighboring window as this window's survivor, which is the same exactness the cleanup identity boundary above already requires. It is also deliberately conservative about the read itself, sharing `fm_backend_tmux_window_inventory` with `fm_backend_tmux_agent_state` so both mean the same thing by an absent session: only a definitive missing-session, missing-server, or connect-error response proves the window gone. Any other read failure - a momentarily unresponsive server, or a teardown PATH without tmux on it - refuses, because a read that could not run is not evidence of absence. diff --git a/docs/zellij-backend.md b/docs/zellij-backend.md index fb58e52a62..326f8781f7 100644 --- a/docs/zellij-backend.md +++ b/docs/zellij-backend.md @@ -89,6 +89,7 @@ A short viewport may expose fewer lines than requested. Closing a pane leaves an empty tab. Cleanup resolves and verifies the owning tab, then uses `close-tab-by-id` so both the task pane and tab disappear. +The shared cleanup path also requires a post-close proof that the recorded pane and scoped task tab are absent; an unreadable or ambiguous proof fails closed and retains the task identity. Real test cleanup uses only an isolated non-`firstmate` session and the guard in `tests/zellij-test-safety.sh`; it never calls all-session deletion commands. ## Active limits diff --git a/tests/fm-backend-orca.test.sh b/tests/fm-backend-orca.test.sh index a62043a76f..c49a8db1fe 100755 --- a/tests/fm-backend-orca.test.sh +++ b/tests/fm-backend-orca.test.sh @@ -46,12 +46,19 @@ if [ "${1:-}" = status ] && [ "${FM_ORCA_STATUS_RESPONSE:-ready}" != sequence ]; printf '{"ok":true,"result":{"runtime":{"reachable":true,"state":"ready"}}}\n' exit 0 fi +if [ "${1:-}" = terminal ] && [ "${2:-}" = read ] \ + && [ ! -f "$RESP/$next.out" ] && [ ! -f "$RESP/$next.exit" ]; then + printf '{"ok":false,"error":{"code":"terminal_handle_stale","message":"terminal handle stale"}}\n' + exit 0 +fi n=$next echo "$n" > "$COUNT_FILE" if [ -f "$RESP/$n.exit" ]; then exit "$(cat "$RESP/$n.exit")" fi -[ -f "$RESP/$n.out" ] && cat "$RESP/$n.out" +if [ -f "$RESP/$n.out" ]; then + cat "$RESP/$n.out" +fi exit 0 SH chmod +x "$fb/orca" @@ -966,7 +973,8 @@ test_teardown_preserves_metadata_when_orca_remove_error_json() { "decisions_reviewed=1" "decision_keys=" orca_case remove-error-teardown printf '{"ok":true,"result":{}}\n' > "$RESP/1.out" - printf '{"ok":false,"error":{"code":"worktree_not_removed","message":"worktree not removed"}}\n' > "$RESP/2.out" + printf '{"ok":false,"error":{"code":"terminal_handle_stale","message":"terminal handle stale"}}\n' > "$RESP/2.out" + printf '{"ok":false,"error":{"code":"worktree_not_removed","message":"worktree not removed"}}\n' > "$RESP/3.out" neutral=$(neutral_fm_root "$CASE_DIR/neutral") set +e out=$( PATH="$FB:$PATH" FM_ORCA_LOG="$LOG" FM_ORCA_RESPONSES="$RESP" \ @@ -1239,7 +1247,8 @@ test_secondmate_force_teardown_removes_orca_child_via_orca() { printf '{"ok":true,"result":{"worktree":{"id":"wt-child-cleanup::/orca/wt-child-cleanup","path":"%s"}}}\n' "$childwt" > "$RESP/1.out" printf '{"ok":true,"result":{"worktree":{"id":"wt-child-cleanup::/orca/wt-child-cleanup","path":"%s"}}}\n' "$childwt" > "$RESP/2.out" printf '{"ok":true,"result":{}}\n' > "$RESP/3.out" - printf '{"ok":true,"result":{}}\n' > "$RESP/4.out" + printf '{"ok":false,"error":{"code":"terminal_handle_stale","message":"terminal handle stale"}}\n' > "$RESP/4.out" + printf '{"ok":true,"result":{}}\n' > "$RESP/5.out" add_tmux_fake "$FB" neutral=$(neutral_fm_root "$CASE_DIR/neutral") set +e diff --git a/tests/fm-backend-zellij.test.sh b/tests/fm-backend-zellij.test.sh index 4963b05131..0f418a82ab 100755 --- a/tests/fm-backend-zellij.test.sh +++ b/tests/fm-backend-zellij.test.sh @@ -850,6 +850,8 @@ test_teardown_passes_recorded_tab_id_to_zellij_kill() { "decision_keys=" printf '[]\n' > "$dir/responses/1.out" printf '[{"tab_id":3,"name":"fm-zghost"}]\n' > "$dir/responses/2.out" + printf '[]\n' > "$dir/responses/4.out" + printf '[]\n' > "$dir/responses/5.out" fb=$(make_zellij_fakebin "$dir") out=$( PATH="$fb:$PATH" FM_STATE_OVERRIDE="$state" FM_DATA_OVERRIDE="$data" FM_CONFIG_OVERRIDE="$config" \ FM_ZELLIJ_LOG="$dir/log" FM_ZELLIJ_RESPONSES="$dir/responses" FM_ZELLIJ_SESSION_LIST="firstmate" \ @@ -896,6 +898,8 @@ test_forced_secondmate_teardown_kills_zellij_children_with_child_home_tag() { zellij_pane_response "$dir" 1 7 4 zellij_tab_response "$dir" 2 4 "$child_title" printf '[]\n' > "$dir/responses/3.out" + printf '[]\n' > "$dir/responses/4.out" + printf '[]\n' > "$dir/responses/5.out" fb=$(make_zellij_fakebin "$dir") out=$( PATH="$fb:$PATH" FM_STATE_OVERRIDE="$state" FM_DATA_OVERRIDE="$data" FM_CONFIG_OVERRIDE="$config" \ FM_ROOT_OVERRIDE="$ROOT" \ diff --git a/tests/fm-backend.test.sh b/tests/fm-backend.test.sh index 0f8f4fb4e3..2f720198db 100755 --- a/tests/fm-backend.test.sh +++ b/tests/fm-backend.test.sh @@ -971,6 +971,8 @@ test_teardown_conformance_old_vs_new() { old_bin=$(build_old_bin teardown-old) git -C "$ROOT" show "$old_tmux_ref:bin/backends/tmux.sh" > "$old_bin/bin/backends/tmux.sh" \ || { BASE_REF=$saved_base_ref; fail "could not materialize historical tmux adapter from $old_tmux_ref"; } + cp "$ROOT/bin/fm-backend.sh" "$old_bin/bin/fm-backend.sh" \ + || { BASE_REF=$saved_base_ref; fail "could not overlay the current backend wrapper in the legacy adapter fixture"; } BASE_REF=$saved_base_ref proj="$TMP_ROOT/teardown-project"; wt="$TMP_ROOT/teardown-wt" id="teardownconform1" diff --git a/tests/fm-secondmate-sync.test.sh b/tests/fm-secondmate-sync.test.sh index 1e5d2290f3..907fec7e18 100755 --- a/tests/fm-secondmate-sync.test.sh +++ b/tests/fm-secondmate-sync.test.sh @@ -624,8 +624,11 @@ case "\$cmd \$sub" in "status --json") printf '{"client":{"version":"0.7.1","protocol":14},"server":{"running":true}}\n' ;; + "session list") + printf '{"sessions":[{"name":"default","running":true,"socket_path":"/tmp/fm-secondmate-sync-herdr.sock"}]}\n' + ;; "pane get") - if [ "\$arg" = "${stale#*:}" ]; then + if [ "\$arg" = "${stale#*:}" ] && [ ! -e "$dir/closed" ]; then printf '{"result":{"pane":{"pane_id":"${stale#*:}"}}}\n' elif [ "\$arg" = "${fresh#*:}" ]; then printf '{"result":{"pane":{"pane_id":"${fresh#*:}"}}}\n' @@ -634,6 +637,9 @@ case "\$cmd \$sub" in exit 0 fi ;; + "pane close") + : > "$dir/closed" + ;; "agent get") if [ "\$arg" = "${stale#*:}" ]; then printf '{"error":{"code":"agent_not_found","message":"gone"}}\n' >&2 diff --git a/tests/fm-test-run.test.sh b/tests/fm-test-run.test.sh index b310015148..691fd2ff8e 100755 --- a/tests/fm-test-run.test.sh +++ b/tests/fm-test-run.test.sh @@ -428,6 +428,21 @@ test_changed_dependency_selection_and_unmapped_failure() { pass "changed selection covers dependents, fails closed for live unmapped source, and accepts retired unconsumed source" } +test_changed_review_evidence_does_not_require_test_mapping() { + local tmp repo listed + tmp=$(mktemp -d "${TMPDIR:-/tmp}/fm-test-run-evidence.XXXXXX") + repo="$tmp/repo" + init_changed_fixture_repo "$repo" + mkdir -p "$repo/.omo/evidence" + printf '# review evidence\n' >"$repo/.omo/evidence/review.md" + + listed=$(cd "$repo" && bin/fm-test-run.sh --list --changed --base HEAD) + [ -z "$listed" ] || fail "review evidence selected unrelated tests: $listed" + + rm -rf "$tmp" + pass "committed review evidence is ignored by changed-test selection" +} + # A direct test reference is per-script evidence. Widening it to the referencing # test's whole family is what turned a one-line change to a shared helper into # every real-Herdr E2E, including scripts with no dependency on it at all. @@ -1744,6 +1759,7 @@ test_task_marker_refuses_the_primary_checkout test_changed_runner_surfaces_select_their_family test_shell_line_ending_policy_selects_runner_contract test_changed_dependency_selection_and_unmapped_failure +test_changed_review_evidence_does_not_require_test_mapping test_changed_bin_reference_selects_per_script_not_per_family test_changed_uses_bounded_automatic_concurrency test_windows_posix_mode_emulation_does_not_fail_parallel_runs