Skip to content

feat: enable advisory Jev review assist and harden runtime reconciliation - #5077

Closed
Ivory2024 wants to merge 20 commits into
kunchenguid:mainfrom
Ivory2024:fm/firstmate-nomistakes-jev-review-assist-20260920
Closed

Ivory2024 wants to merge 20 commits into
kunchenguid:mainfrom
Ivory2024:fm/firstmate-nomistakes-jev-review-assist-20260920

Conversation

@Ivory2024

Copy link
Copy Markdown

Intent

Enable the documented opt-in jev.review_assist advisory pre-brief in this Firstmate repository's .no-mistakes.yaml. The installed no-mistakes is v1.79.0 or newer. The ordinary cold complete review remains authoritative; Jev is advisory only. Deliver this change to the Ivory2024/firstmate fork and never to kunchenguid/firstmate upstream.

What Changed

  • Enabled the documented opt-in jev.review_assist advisory pre-brief in .no-mistakes.yaml, while keeping the ordinary cold-complete review authoritative and documenting its configuration and data boundary.
  • Hardened inactive reconciliation and runtime backends so terminal cleanup requires backend-specific proof of endpoint absence before outcomes are reported or records are removed.
  • Added repository-layout and steering skills, consolidated agent guidance by removing CLAUDE.md, and updated dispatch/quota behavior, CI, documentation, and focused shell tests.

Risk Assessment

🚨 High: The branch does not satisfy the authoritative Jev opt-in criterion, and its new terminal cleanup path can exceed the bounded reconciliation poll and suppress timely delivery diagnostics.

Testing

Ran the targeted repository contract tests, verified no-mistakes v1.79.0, and captured a semantic configuration artifact. No Herdr lab was used because Jev review assist is owned by the external no-mistakes review daemon, not the Herdr runtime surface. No source changes or transient worktree artifacts were created.

  • Live validation: ⚠️ inconclusive - 0 of 3 scenarios driven live against the product
Scenario Result Live Evidence
Operator loads the tracked configuration with no-mistakes v1.79.0+; Jev review assist is enabled and commands.test remains absent. ⏸️ untested no The prior payload recorded only static configuration and version checks, not a live product execution.
Operator runs a review with Jev available; Jev supplies only an advisory pre-brief and the ordinary cold complete review remains authoritative. ⏸️ untested no The external no-mistakes review daemon and permitted TYPESAFE_API_KEY were unavailable for this phase, and invoking review lifecycle commands is prohibited by the test-phase boundary.
Operator runs a review with a missing key or failed Jev call; review falls back to cold review and records the fallback reason. ⏸️ untested no Fallback behavior belongs to the external no-mistakes daemon and requires its review audit logs plus authorized lifecycle execution.
Evidence: Jev config semantic assertion

Source: Jev config semantic assertion

$ no-mistakes --version no-mistakes version v1.79.0 (fc540ac) 2026-09-19T09:34:44Z $ ruby semantic config assertion parsed jev.review_assist=true parsed commands.test=absent

$ no-mistakes --version
no-mistakes version v1.79.0 (fc540ac) 2026-09-19T09:34:44Z
$ ruby semantic config assertion
parsed jev.review_assist=true
parsed commands.test=absent
- Outcome: ⚠️ 2 warnings across 2 runs (10m49s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

⚠️ **Rebase** - 1 warning

Confirm these commits belong in this PR before approving, or manually separate the intended work onto origin/main before gating.

🔧 No changes applied.
1 warning still open:

Confirm these commits belong in this PR before approving, or manually separate the intended work onto origin/main before gating.

no changes applied: bundled local-default commits require manual separation or explicit approval; the rebase conflict resolver cannot safely select commits to discard.

⚠️ **Review** - 2 issues (1 error, 1 warning)
  • 🚨 docs/configuration.md:234 - The authoritative intent requires: “Enable the documented opt-in jev.review_assist advisory pre-brief in this Firstmate repository's .no-mistakes.yaml.” The changed documentation instead states at docs/configuration.md:234 that the option is global-only and cannot be set from tracked .no-mistakes.yaml, and the final .no-mistakes.yaml contains no jev block. The required repository-level opt-in is therefore absent; confirm whether to revise the intent to the operator-local configuration or provide a supported repository configuration path.
  • ⚠️ bin/fm-inactive-reconcile.sh:413 - The new cleanup call at bin/fm-inactive-reconcile.sh:413 runs from ledger_pass at :471 before scan establishes its deadline at :658, so each poll can perform backend cleanup outside the reconciliation budget. Herdr lock acquisition alone can wait up to 5 seconds per child at bin/backends/herdr.sh:3349-3367; multiple terminal children can exceed the outer timeout at bin/fm-inactive-reconcile.sh:701-708, which treats timeout as success and may terminate while a child meta lock is held before an actionable notice is emitted. Bound or budget this cleanup path. The same invariant is contradicted by the file-only claims at bin/fm-inactive-reconcile.sh:18-19 and :454-456 and docs/secondmate-parent-channel.md:35.

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

  • ⚠️ bin/fm-inactive-reconcile.sh:413 - The new cleanup call at bin/fm-inactive-reconcile.sh:413 runs from ledger_pass at :471 before scan establishes its deadline at :658, so each poll can perform backend cleanup outside the reconciliation budget. Herdr lock acquisition alone can wait up to 5 seconds per child at bin/backends/herdr.sh:3349-3367; multiple terminal children can exceed the outer timeout at bin/fm-inactive-reconcile.sh:701-708, which treats timeout as success and may terminate while a child meta lock is held before an actionable notice is emitted. Bound or budget this cleanup path. The same invariant is contradicted by the file-only claims at bin/fm-inactive-reconcile.sh:18-19 and :454-456 and docs/secondmate-parent-channel.md:35.
  • 🚨 bin/fm-inactive-reconcile.sh:420 - The bounded cleanup fix still performs an unbounded failure-notice publication after the timeout: report_child_ledger_locked calls notice_parent_report_failed at bin/fm-inactive-reconcile.sh:420, which reaches publish_actionable/fm_wake_append and can wait indefinitely for the wake-queue lock. The caller still holds the child meta lock from bin/fm-inactive-reconcile.sh:469 until :481; if the outer scan watchdog kills this path during that wait, the lock and pending outcome can be stranded. The same invariant remains in the inactive path at bin/fm-inactive-reconcile.sh:579-587, including the main-home queue_notice_once branch at :582-583. Bound or defer failure publication so the budgeted scan always releases its meta lock before any unbounded wake-queue operation.
⚠️ **Test** - 2 warnings
  • ⚠️ live validation verdict: inconclusive (0 of 2 scenarios were driven live against the product); untested: Run a review turn with Jev review assist enabled; Jev provides only an advisory pre-brief while the ordinary cold complete review remains authoritative., Run review with a missing Jev key or failed Jev call; review falls back to cold review and records the fallback reason.
  • Live validation: ⚠️ inconclusive - 0 of 2 scenarios driven live against the product
Scenario Result Live Evidence
Run a review turn with Jev review assist enabled; Jev provides only an advisory pre-brief while the ordinary cold complete review remains authoritative. ⏸️ untested no The assigned phase forbids invoking the no-mistakes review gate, and the daemon review/audit surface is outside this worktree. Provide the outer executor's no-mistakes v1.79.0+ review run with permitt…
Run review with a missing Jev key or failed Jev call; review falls back to cold review and records the fallback reason. ⏸️ untested no Fallback behavior belongs to the external no-mistakes daemon and cannot be exercised here without the authorized review-step runner and audit logs. Provide those through the outer executor.
  • no-mistakes --version
  • ruby -e 'require "yaml"; c=YAML.load_file(".no-mistakes.yaml"); abort unless c.dig("jev", "review_assist") == true; puts "config semantic check: jev.review_assist=true"'
  • git status --short --branch

🔧 No changes applied.
2 warnings still open:

  • ⚠️ docs/configuration.md:233 - Live Jev review behavior was not demonstrable in this test phase. It requires the external no-mistakes review daemon, permitted TYPESAFE_API_KEY, and review audit logs; lifecycle execution is outside this assigned phase.
  • ⚠️ live validation verdict: inconclusive (0 of 3 scenarios were driven live against the product); untested: Operator loads the tracked configuration with no-mistakes v1.79.0+; Jev review assist is enabled and commands.test remains absent., Operator runs a review with Jev available; Jev supplies only an advisory pre-brief and the ordinary cold complete review remains authoritative., Operator runs a review with a missing key or failed Jev call; review falls back to cold review and records the fallback reason.
  • Live validation: ⚠️ inconclusive - 0 of 3 scenarios driven live against the product
Scenario Result Live Evidence
Operator loads the tracked configuration with no-mistakes v1.79.0+; Jev review assist is enabled and commands.test remains absent. ⏸️ untested no The prior payload recorded only static configuration and version checks, not a live product execution.
Operator runs a review with Jev available; Jev supplies only an advisory pre-brief and the ordinary cold complete review remains authoritative. ⏸️ untested no The external no-mistakes review daemon and permitted TYPESAFE_API_KEY were unavailable for this phase, and invoking review lifecycle commands is prohibited by the test-phase boundary.
Operator runs a review with a missing key or failed Jev call; review falls back to cold review and records the fallback reason. ⏸️ untested no Fallback behavior belongs to the external no-mistakes daemon and requires its review audit logs plus authorized lifecycle execution.
  • no-mistakes --version
  • ruby -ryaml semantic assertion for .no-mistakes.yaml
  • bash tests/fm-no-mistakes-required.test.sh
  • bash tests/fm-review-diff.test.sh
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

irene and others added 20 commits September 19, 2026 08:04
…ate-20260919

feat: gate fresh CLAUDE.md pointer creation on Claude Code version >= 2.1.277
…tage 2) (#2)

* feat(bin): complete stage 2 CLAUDE.md pointer removal

* no-mistakes(review): Restore column-0 heredoc regression fixture with generic content

* no-mistakes(review): Remove stale CLAUDE.md pointer claim from updatefirstmate skill

---------

Co-authored-by: irene <irene@ireneui-MacBookPro.local>
Prior commits on this branch regressed past stage2, restoring the
unconditional CLAUDE.md pointer-write logic stage2 removed. Reset to
fork/main (stage2's merged head) and redo stage3 correctly: delete the
now-dead fm_version_at_least/claude_supports_native_agents_md functions
and their header-comment reference, and drop the now-vestigial
with_mock_claude/with_no_claude test helpers (the script no longer
reads claude --version at all).

Co-authored-by: irene <irene@ireneui-MacBookPro.local>
…eering skills (#4)

* feat: add lazy specialist tool routing

Expose ECC, paperthin, and ultrawork as captain-approved specialist paths while keeping Firstmate intake and lifecycle authority. Load only the selected skill or mode and keep ECC hooks, MCP, and legacy sync opt-in.

* docs(agents): recover firstmate-layout and task-steering skills

These two skills existed only on an orphaned local branch, never pushed.
firstmate-layout is re-extracted from AGENTS.md section 2's current
(much larger) layout tree rather than reusing the stale 2026-09-14
snapshot. task-steering's underlying AGENTS.md paragraph was byte-identical
to the 2026-09-14 extraction, so it is reused as-is. Both get a one-line
trigger in section 13 and a documentation-audiences.json entry, matching
how specialist-tools (recovered earlier on this branch) is registered.

---------

Co-authored-by: irene <irene@ireneui-MacBookPro.local>
Co-authored-by: irene <irene@ireneui-MacBookPro.local>
.treehouse/ holds only runtime pool bookkeeping (treehouse-state.json,
treehouse-state.lock), never captain work, but its absence from
.gitignore makes it show up as an untracked dirty-tree blocker for
bin/fm-update.sh's self-update fast-forward check.

Co-authored-by: irene <irene@ireneui-MacBookPro.local>
Verified against no-mistakes v1.79.0's own e2e tests and upstream PR kunchenguid#1120:
jev.review_assist is global-only and has no effect when set in a repo's
tracked .no-mistakes.yaml. Revert that no-op change and document the real
activation path, data sent, and data-boundary guidance instead.
@Ivory2024

Copy link
Copy Markdown
Author

Opened by mistake against upstream; closing. Work continues against the maintainer's own fork only.

@Ivory2024 Ivory2024 closed this Sep 20, 2026
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