Skip to content

fix: harden endpoint cleanup and enable Jev review assist - #12

Merged
Ivory2024 merged 16 commits into
mainfrom
fm/firstmate-nomistakes-jev-review-assist-20260920
Sep 21, 2026
Merged

Ivory2024 merged 16 commits into
mainfrom
fm/firstmate-nomistakes-jev-review-assist-20260920

Conversation

@Ivory2024

@Ivory2024 Ivory2024 commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Intent

Harden self-hosted Discord relay cleanup and quota dispatch resolution; enable Jev review-assist advisory pre-brief in .no-mistakes.yaml. Deliver to Ivory2024/firstmate fork only.

What Changed

  • Hardened runtime backend cleanup with backend-specific post-close absence checks; ambiguous or failed verification now retains task identity instead of reporting cleanup success.
  • Enabled advisory Jev review-assist pre-briefs in .no-mistakes.yaml and documented their fallback and data boundaries.
  • Updated changed-test selection and backend test fixtures for the cleanup and review-evidence changes.

Risk Assessment

🚨 High: The required Jev configuration is absent, and the Herdr cleanup change introduces a concrete return-contract regression that can fail existing adapter usage; documentation also overstates cleanup ordering.

Testing

Focused dispatch-resolution, backend cleanup, and Herdr-lab guard tests passed. The real resolver safely no-oped without credentials. Real Herdr validation was blocked because herdr is absent from PATH. Jev review validation was not driven because this phase cannot invoke no-mistakes pipeline controls and requires the external Jev daemon credential and audit-log authority. No visual artifact was applicable for these CLI/backend scenarios.

  • Live validation: ⚠️ inconclusive - 0 of 6 scenarios driven live against the product
Scenario Result Live Evidence
Resolve quota dispatch with auth_required AGY evidence; AGY remains eligible but unranked and is not dispatched. ⏸️ untested no The prior payload recorded only non-live focused tests, so it did not establish this result against the live product.
Run backend cleanup through the shared wrapper while preserving legacy adapter compatibility. ⏸️ untested no The prior payload recorded only non-live focused tests, so it did not establish this result against the live product.
Provision and tear down a named non-default Herdr lab and exercise real cleanup. ⏸️ untested no The required herdr executable is absent from PATH. Provide the Herdr CLI, then rerun the guarded fm-lab-* scenario through bin/fm-herdr-lab.sh.
Load tracked configuration with Jev review assist enabled and no commands.test override. ⏸️ untested no This assigned phase cannot invoke no-mistakes pipeline controls. Provide an authorized review phase with no-mistakes v1.79+ and its audit surface.
Run review with Jev available; Jev supplies advisory context while cold review remains authoritative. ⏸️ untested no Requires the external Jev daemon, approved TYPESAFE_API_KEY, and authorized review-step audit logs; those controls are outside this phase.
Run review with missing Jev credentials or a failed Jev call; review falls back and records the reason. ⏸️ untested no Requires invoking the no-mistakes review step and inspecting its audit output, which this assigned phase is prohibited from doing.
Evidence: dispatch resolution tests

Source: dispatch resolution tests

ok - absent key is off: one stderr line, exit 0, no network call
ok - TYPESAFE_API_KEY= in .env activates the tool; environment and config overrides work
ok - clear: one rule Choice request, key on the fd header only, spendPriority argmax over every candidate
ok - rules snapshots and shell quoting preserve the profile protocol
ok - auth_required AGY evidence stays unknown while its external cause remains visible
ok - auth_required object errors preserve the eligible unranked outcome
ok - auth_required AGY remains unranked before rule-floor selection
ok - auth_required AGY rule floors block cross-provider ranking
ok - no-rule fallback, Agy, Gemini, and documented configurations resolve
ok - ambiguous: confidence below the fixed floor hands the decision back
ok - escalate: a rule declared approval: captain never yields a profile
ok - rule floor: known shortfall falls through while unavailable evidence escalates
ok - declared provider and profile floor evidence are applied in code
ok - nonnumeric spendPriority evidence is never ranked
ok - partial and missing quota evidence remain eligible but unranked
ok - provider-wide and exact quota rows combine into one limiting candidate
ok - default: no rule matched resolves among the default profiles
ok - tie: equal spendPriority never breaks by array order
ok - no rankable candidate: the tool escalates instead of guessing
ok - quota evidence comes from one quota-axi --json read, and its failure is an error outcome
ok - API, transport, and response failures are error outcomes with exit 0
ok - configuration errors exit 2 before any network call
# all fm-dispatch-resolve tests passed
Evidence: backend cleanup tests

Source: backend cleanup tests

ok - fm_backend_name: FM_BACKEND env > config/backend > default tmux
ok - fm_backend_detect: no markers -> undetected, HERDR_ENV=1 -> herdr, $TMUX -> tmux, CMUX_WORKSPACE_ID -> cmux, nested combinations resolve innermost-first
ok - fm_backend_detect: falls back to __CFBundleIdentifier=com.cmuxterm.app when CMUX_WORKSPACE_ID is absent (signal bundle-id; foreign bundle ids rejected)
ok - fm_backend_detect: the cmux fallback signals are macOS-only (inert on a non-Darwin uname)
ok - fm_backend_detect: an inherited cmux bundle id never outranks $TMUX or HERDR_ENV (tmux/herdr-inside-cmux false positive absorbed)
ok - fm_backend_detect: ancestry fallback matches the lsappinfo-resolved (bundle-id) cmux app pid in the parent chain
ok - fm_backend_detect: ancestry fallback matches a bundle-shaped cmux comm path at any install location when lsappinfo cannot resolve a pid
ok - fm_backend_detect: ancestry fallback stops undetected at launchd (a reparented tmux server never reaches cmux)
ok - fm_backend_name: a fallback-detected cmux prints a NOTICE naming the fallback signal; the primary-marker notice is unchanged
ok - fm_backend_name: verified Herdr and tmux stay silent while experimental cmux remains loud
ok - fm_backend_name: an explicit FM_BACKEND or config/backend setting always wins over runtime auto-detection, including an ambient cmux marker
ok - fm_backend_validate: implemented adapters accepted, unknown and blocked codex-app backends refused loudly
ok - zsh: fm_backend_source recognizes known backends and rejects unknown ones
ok - bash: fm_backend_source recognizes known backends and rejects unknown ones
ok - fm_backend_validate_spawn: all implemented lifecycle backends are spawn-supported
ok - fm_meta_get / fm_backend_of_meta: read last key=value and default backend to tmux
ok - fm_backend_resolve_selector: session:window literal, exact task id first, legacy fm-<id> label fallback, ad hoc bare name via tmux list-windows
ok - fm_backend_of_selector: exact task ids, legacy fm-<id> labels, and matching explicit targets inherit metadata backend
ok - fm-send.sh: explicit tmux targets are verified; text types once and submits with Enter
ok - fm-peek.sh: capture-pane invocation and output are byte-identical old vs new
ok - fm-spawn.sh: a project reached through a symlinked prefix (e.g. macOS /tmp -> /private/tmp) does not trip the isolation guard's false refusal
ok - fm-teardown.sh: treehouse return remains compatible while tmux cleanup uses exact selectors
ok - fm-spawn.sh --backend bogus is refused loudly
ok - fm-spawn.sh --backend codex-app is refused
ok - fm-spawn.sh honors FM_BACKEND and refuses an unimplemented value loudly
ok - fm-spawn.sh: an explicit --backend tmux resolves silently and writes no backend= (missing means tmux)
ok - fm-spawn.sh: explicit --backend tmux wins over an ambient HERDR_ENV=1 auto-detect marker
ok - fm-spawn.sh: auto-detect resolves nested tmux-in-herdr to tmux and stays silent end to end
Evidence: Herdr lab guard tests

Source: Herdr lab guard tests

ok - fm-herdr-lab: names fail closed and require the lab prefix
ok - fm-herdr-lab: provisioning, scoped calls, guarded teardown, and fleet tripwire are deterministic
ok - fm-herdr-lab: missing tripwire refuses teardown before any Herdr call
ok - fm-herdr-lab: changed default fleet state is a hard failure
ok - fm-herdr-lab: an owned stopped lab can re-provision safely
ok - fm-herdr-lab: failed deletion retains ownership until absence is confirmed
ok - fm-herdr-lab: timed-out provisioning cancels the launch before teardown
ok - fm-herdr-lab: the viewer attaches only to a session this lab owns
ok - fm-herdr-lab: timed-out viewer startup cancels its exact launcher
ok - fm-herdr-lab: startup timeout allows launcher child escalation
ok - fm-herdr-lab: viewer start requires an identity-matched owned process
ok - fm-herdr-lab: viewer stop signals only recorded processes and confirms the detach
ok - fm-herdr-lab: viewer ownership requires the recorded parent
ok - fm-herdr-lab: interrupted viewer start cancels its launcher
ok - fm-herdr-lab: teardown refuses to destroy a session an attached viewer still holds
ok - fm-herdr-lab: unreadable detach results fail closed on running sessions
ok - fm-herdr-lab: the viewer launcher refuses unsafe sessions and pidfiles
Evidence: live tool attempts

Source: live tool attempts

== tool availability ==
no-mistakes version v1.79.0 (fc540ac) 2026-09-19T09:34:44Z
herdr=
== guarded Herdr live attempt ==
fm-herdr-lab: herdr is required
prepare_exit=1
== real dispatch resolver without credentials ==
dispatch-resolve: off (TYPESAFE_API_KEY absent from the environment and /tmp/fm-dispatch-home.zP1fKR/.env)
dispatch_exit=0
== worktree status ==
- Outcome: ⚠️ 1 warning across 2 runs (28m34s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 3 issues (2 errors, 1 warning)
  • 🚨 docs/configuration.md:245 - The authoritative intent requires “enable Jev review-assist advisory pre-brief in .no-mistakes.yaml,” but the target explicitly documents that jev.review_assist is global-only at docs/configuration.md:245 and the final .no-mistakes.yaml has no jev block. The required tracked opt-in is therefore absent; decide whether the intent or the verified configuration boundary should govern.
  • 🚨 bin/backends/herdr.sh:3398 - The new return propagation breaks the existing best-effort contract of direct Herdr adapter calls: close failure now returns nonzero at bin/backends/herdr.sh:3398, and lock/target refusal also returns nonzero at :3377 and :3401. Existing callers/tests rely on this adapter remaining best-effort (tests/fm-backend-herdr.test.sh:2804,2836,2869,3667), while bin/fm-backend.sh:813 already performs the independent endpoint proof. Preserve the adapter's 0 return and let the shared wrapper carry the cleanup refusal.
  • ⚠️ docs/secondmate-parent-channel.md:31 - The changed documentation claims silent-child outcomes are published after endpoint cleanup is confirmed, but the actual path reports first at bin/fm-inactive-reconcile.sh:556-557, then calls reap_terminal_child_locked at :562; that helper swallows fm_backend_kill failure at :510. A failed close can therefore leave the endpoint live while the parent has already received the outcome. Narrow the documentation to the actual ordering unless the source is intentionally changed to enforce cleanup before publication.

🔧 Fix applied.
3 issues (2 errors, 1 warning) still open:

  • ⚠️ docs/secondmate-parent-channel.md:31 - The changed documentation claims silent-child outcomes are published after endpoint cleanup is confirmed, but the actual path reports first at bin/fm-inactive-reconcile.sh:556-557, then calls reap_terminal_child_locked at :562; that helper swallows fm_backend_kill failure at :510. A failed close can therefore leave the endpoint live while the parent has already received the outcome. Narrow the documentation to the actual ordering unless the source is intentionally changed to enforce cleanup before publication.
  • 🚨 .no-mistakes.yaml:12 - The final restore commit reintroduces the unsupported tracked opt-in at .no-mistakes.yaml:12-13. History (5d4de6c) verified that no-mistakes v1.79 treats jev.review_assist as global-only, and this range adds no daemon support; docs/configuration.md:246 therefore claims behavior that is unreachable. This contradicts the required criterion “enable Jev review-assist advisory pre-brief in .no-mistakes.yaml.” Choose a supported operator-local configuration or authorize a daemon/configuration change.
  • 🚨 bin/fm-bootstrap.sh:831 - The 6366fec cleanup fix added a Zellij-specific recovery path that passes fm-$id but runs under the primary FM_HOME at bin/fm-bootstrap.sh:831. A dead secondmate's tab is titled with the child home's scoped tag; bin/backends/zellij.sh:604-613 therefore refuses to close it, and the new proof at :624-638 also computes the wrong scoped title and sees the pane still present. Startup logs “endpoint cleanup could not be confirmed” and never respawns the dead secondmate. Run this cleanup under the recorded child home/root context, as the existing child teardown path does at bin/fm-teardown.sh:3078.
⚠️ **Test** - 1 warning
  • ⚠️ tests/fm-backend.test.sh:971 - tests/fm-backend.test.sh snapshots git archive HEAD, so it still exercised the pre-fix wrapper in this uncommitted worktree. Re-run after the source fix is committed.
  • ⚠️ live validation verdict: inconclusive (0 of 7 scenarios were driven live against the product); untested: Operator runs the self-hosted Discord poll/reply path; tokenless mode no-ops, ingestion emits x-inbox/x-mention, and replies route through the self-hosted adapter., Operator resolves quota dispatch; known evidence ranks candidates while auth_required AGY remains eligible but unranked and is not dispatched., Operator loads tracked configuration; Jev review_assist is true and commands.test remains absent., Backend cleanup handles a legacy adapter without the new endpoint-proof helper while preserving adapter refusal status., Operator provisions a named non-default Herdr lab and exercises real cleanup lifecycle., Operator runs review with Jev available; Jev adds only an advisory pre-brief while ordinary cold review remains authoritative., Operator runs review with missing Jev credentials or a failed Jev call; review falls back to cold review and records the reason.
  • Live validation: ⚠️ inconclusive - 0 of 7 scenarios driven live against the product
Scenario Result Live Evidence
Operator runs the self-hosted Discord poll/reply path; tokenless mode no-ops, ingestion emits x-inbox/x-mention, and replies route through the self-hosted adapter. ⏸️ untested no The prior payload explicitly recorded live=false and only established results from controlled fixtures, not the live product.
Operator resolves quota dispatch; known evidence ranks candidates while auth_required AGY remains eligible but unranked and is not dispatched. ⏸️ untested no The prior payload explicitly recorded live=false and only established results from deterministic quota fixtures, not live provider execution.
Operator loads tracked configuration; Jev review_assist is true and commands.test remains absent. ⏸️ untested no The prior payload explicitly recorded live=false and only established a semantic configuration check; it did not drive the live pipeline.
Backend cleanup handles a legacy adapter without the new endpoint-proof helper while preserving adapter refusal status. ⏸️ untested no The prior payload explicitly recorded live=false and only established an executable adapter simulation, not a real multiplexer session.
Operator provisions a named non-default Herdr lab and exercises real cleanup lifecycle. ⏸️ untested no Untested: herdr is absent from PATH and no repository-local binary is supplied. Provide the Herdr CLI, then rerun the guarded prepare/provision/run/teardown flow.
Operator runs review with Jev available; Jev adds only an advisory pre-brief while ordinary cold review remains authoritative. ⏸️ untested no Untested: this phase cannot invoke no-mistakes pipeline controls and requires the external v1.79+ daemon, approved TYPESAFE_API_KEY, and review audit logs. Provide that authority and rerun the review…
Operator runs review with missing Jev credentials or a failed Jev call; review falls back to cold review and records the reason. ⏸️ untested no Untested: this phase cannot invoke no-mistakes pipeline controls or inspect its review audit logs. Provide the authorized review-step runner and audit-log access.
  • bash tests/fm-discord-selfhosted.test.sh
  • bash tests/fm-quota-choose.test.sh
  • bash tests/fm-dispatch-resolve.test.sh
  • bash tests/fm-backend-herdr.test.sh
  • bash tests/fm-test-run.test.sh
  • bash tests/fm-herdr-lab.test.sh
  • bash tests/fm-backend.test.sh (historical HEAD fixture still observed the pre-fix wrapper)
  • Direct backend compatibility driver
  • Semantic YAML configuration check
  • bin/fm-herdr-lab.sh prepare fm-lab-70582-test

🔧 Fix applied.
1 warning still open:

  • ⚠️ live validation verdict: inconclusive (0 of 6 scenarios were driven live against the product); untested: Resolve quota dispatch with auth_required AGY evidence; AGY remains eligible but unranked and is not dispatched., Run backend cleanup through the shared wrapper while preserving legacy adapter compatibility., Provision and tear down a named non-default Herdr lab and exercise real cleanup., Load tracked configuration with Jev review assist enabled and no commands.test override., Run review with Jev available; Jev supplies advisory context while cold review remains authoritative., Run review with missing Jev credentials or a failed Jev call; review falls back and records the reason.
  • Live validation: ⚠️ inconclusive - 0 of 6 scenarios driven live against the product
Scenario Result Live Evidence
Resolve quota dispatch with auth_required AGY evidence; AGY remains eligible but unranked and is not dispatched. ⏸️ untested no The prior payload recorded only non-live focused tests, so it did not establish this result against the live product.
Run backend cleanup through the shared wrapper while preserving legacy adapter compatibility. ⏸️ untested no The prior payload recorded only non-live focused tests, so it did not establish this result against the live product.
Provision and tear down a named non-default Herdr lab and exercise real cleanup. ⏸️ untested no The required herdr executable is absent from PATH. Provide the Herdr CLI, then rerun the guarded fm-lab-* scenario through bin/fm-herdr-lab.sh.
Load tracked configuration with Jev review assist enabled and no commands.test override. ⏸️ untested no This assigned phase cannot invoke no-mistakes pipeline controls. Provide an authorized review phase with no-mistakes v1.79+ and its audit surface.
Run review with Jev available; Jev supplies advisory context while cold review remains authoritative. ⏸️ untested no Requires the external Jev daemon, approved TYPESAFE_API_KEY, and authorized review-step audit logs; those controls are outside this phase.
Run review with missing Jev credentials or a failed Jev call; review falls back and records the reason. ⏸️ untested no Requires invoking the no-mistakes review step and inspecting its audit output, which this assigned phase is prohibited from doing.
  • bash tests/fm-dispatch-resolve.test.sh
  • bash tests/fm-backend.test.sh
  • bash tests/fm-herdr-lab.test.sh
  • FM_HERDR_LAB_STATE_DIR=/tmp/... bash bin/fm-herdr-lab.sh prepare fm-lab-jev-review-test
  • env -u TYPESAFE_API_KEY -u TYPESAFE_API_PRIVATE FM_HOME=/tmp/... bin/fm-dispatch-resolve.sh <brief>
  • no-mistakes --version
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@Ivory2024
Ivory2024 force-pushed the fm/firstmate-nomistakes-jev-review-assist-20260920 branch from 5af8f41 to e9e9ec8 Compare September 20, 2026 22:12
@Ivory2024 Ivory2024 changed the title fix: harden cleanup and quota dispatch resolution fix: harden endpoint cleanup and enable Jev review assist Sep 21, 2026
…t.sh by making the Herdr fixture provide session-lock metadata and model pane-close disappearance, allowing respawn metadata to rotate. Verified with the targeted sync test, fm-backend-herdr.test.sh, bash -n, and git diff --check
@Ivory2024
Ivory2024 merged commit d3cbd59 into main Sep 21, 2026
19 checks passed
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.

1 participant