Repository navigation
feat(hooks): judge session changes with jev-lint model rules at Stop - #150
furedea-agent[bot] wants to merge 9 commits into
Conversation
Model rules judge the commits made since the session started. Registration therefore keeps the starting commit in baseline.json. A record without one, or an empty repository, has none. Refs #74 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Run jev-lint review under op after the verification passes, for repositories of trusted owners. Findings on inputs not judged before block Stop once; the same inputs only notify. Anything that prevents a run is a one-line message and never blocks. Add Python and TypeScript rule files for M-05, M-21 and M-60, and a guard on the verdict cache. Refs #74 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…he setup Replace never-block with one block per set of inputs, as the verification receipts do. Record that jev-lint review needs two reviews. Record that the Environment ID and the token come from the Keychain. Document the one-time setup of a new machine in DESIGN.md and DESIGN_ja.md. Refs #74 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@codex review |
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| _save(record, state, {"input": inputs, "outcome": "clean", "summary": message}) | ||
| return Advice(message) | ||
| message = model_rules_report.describe(outcome) | ||
| _save(record, state, {"input": inputs, "outcome": "findings", "summary": message}) |
There was a problem hiding this comment.
P1 — If the first run happens while stop_hook_active is set, the agent never sees the findings.
The "findings" receipt is saved even when stop_hook_active stops the block, so the findings only go out in a systemMessage, which only the user sees. On the next Stop with the same inputs, _reused matches the receipt and returns notify-only.
This is a common path:
- Stop 1: verification blocks, so model rules don't run.
- The agent fixes the problem.
- Stop 2 (
stop_hook_active=true): verification passes, model rules run, and they find something. - The findings appear only in
systemMessage, the receipt is saved, and the agent stops. - Every later Stop on these inputs is "unchanged; not rerun".
So the main promise ("findings on unjudged inputs block once so the agent sees them") fails right after any verification block.
The receipt already stops the same inputs from blocking twice, so suppressing the block under stop_hook_active isn't needed to prevent a loop. Either block anyway, or record that the findings were not yet shown (e.g. "outcome": "findings-unshown") so the next Stop without stop_hook_active blocks once. The test test_the_stop_hook_blocks_through_the_provider_entry_point only covers the case where the receipt came from a non-active Stop.
There was a problem hiding this comment.
P1 は妥当でした．912f298 で修正しました．stop_hook_active により block できなかった指摘は，レシートに「findings-unshown」として記録し，同じ入力の次の（active でない）Stop で 1 回だけ block します．その後は shown に変わり，通知のみです．active のままの Stop では何度でも通知のみで，ループしません．テスト test_findings_that_could_not_block_because_the_stop_hook_was_active_block_once_at_the_next_stop と test_unseen_findings_still_do_not_block_while_the_stop_hook_stays_active を追加しました．
| if result.get("decision") == "block": | ||
| return result | ||
| with record.lock(unbounded=True): | ||
| advice = model_rules_gate.advise(record, stop_hook_active=stop_hook_active) |
There was a problem hiding this comment.
P2 — The model-rules budget does not account for time verification already used.
advise() is called with the default BUDGET_SECONDS = 90, measured from when advise starts. Verification can use up to GATE_BUDGET_SECONDS = 540, and the Stop hook timeout in agents/hooks.json is 600s. In the worst case that is 540 + 90 = 630s, plus up to 2×10s of Keychain lookups that are not bound by the deadline. That can exceed the hook timeout.
When the provider kills the hook:
- No receipt is saved.
- The
op/jev-lintchildren survive, because they were started withstart_new_session=Trueand are not in the hook's process group.
The PR description says the budget is limited by the remaining Stop time, but nothing passes that value in. Suggest passing budget_seconds=min(BUDGET_SECONDS, HOOK_TIMEOUT - elapsed - margin) from the time the Stop began, and adding a test for it.
There was a problem hiding this comment.
P2 は妥当でした（PR 本文に書いた残り時間の上限が未実装でした）．912f298 で修正しました．session_gate.stop が Stop 開始からの経過を測り，モデルルールの予算を min(90, 600 - 20 - 経過) にして渡します．残りが無ければ実行しません．Keychain 検索の timeout も 10 秒から 5 秒にし，超過がマージン 20 秒に収まるようにしました．テスト test_the_model_rules_get_only_the_time_the_stop_hook_has_left と test_no_model_rules_run_when_the_stop_hook_has_no_time_left を追加しました．なお，プロバイダが hook を強制終了した場合に op と jev-lint の process group が残る点は，予算内で終わる前提のためこの PR では扱いません（jev-lint 自体は API 呼び出しの上限で終わります）．
Review summaryBlocking (P0/P1): 1
Everything else checked out:
|
|
自動セキュリティレビューの指摘（モデルルールのレシートは
このため，今回コードは変更しない． |
The audit job reported DEP001 for the sibling modules the Stop hook imports. Refs #74 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…e Stop budget Findings kept from blocking by stop_hook_active are recorded as not yet shown. They block once at the next Stop on the same inputs, so the agent still sees them. The model rules get only the time left after verification, within the Stop timeout. Refs #74 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
|
||
| def _advise(record: SessionRecord, stop_hook_active: bool, deadline: model_rules_run.Deadline) -> Advice: | ||
| inputs = _inputs(record) | ||
| receipt = record.results().get(RECEIPT_KEY) |
There was a problem hiding this comment.
P2 — The kill switch is ignored when a receipt can be reused. _advise checks the saved receipt before _judge → _preconditions looks at AGENT_MODEL_RULES. If a findings-unshown receipt exists (it was saved while stop_hook_active was set) and the user then sets AGENT_MODEL_RULES=off, the next Stop on the same inputs still returns {"decision":"block"}. Other receipts keep showing the "findings unchanged" message. The PR's behavior table says the kill switch gives only a one-line systemMessage and never blocks. Suggested fix: check the kill switch, and probably the trusted-repo check too, before reading the receipt. Also add a test for kill switch plus an existing findings-unshown receipt.
There was a problem hiding this comment.
妥当でした．7b910e5 で修正しました．AGENT_MODEL_RULES=off を，保存済みレシートの再利用より前（advise の最初）で判定します．テスト test_the_kill_switch_wins_over_a_stored_receipt_of_unseen_findings を追加しました．
| cache = model_rules_run.cache_path(state_directory()) | ||
| start = record.head | ||
| committed = start is not None and head_commit(record.root) != start | ||
| bases = (start, None) if committed else (None,) |
There was a problem hiding this comment.
P2 — The uncommitted review (--base omitted) does not judge only this session's changes. It judges the whole working tree against HEAD, plus every untracked file that git does not ignore. The only gate is that baseline.changed_paths is non-empty, and the plan never narrows the paths to those session changes. So if the session starts in a dirty tree (user edits already in progress, or untracked scratch files), one small agent edit sends all of those files to the model. Findings in them then block the agent as if it wrote that code. Suggested fix: when calling jev-lint, pass only the paths that record.baseline.changed_paths(current) returns, intersected with the plan. Also add a test where the session starts in a dirty tree.
There was a problem hiding this comment.
妥当でした（ADR は過剰包含を許容していましたが，狭められるため直しました）．7b910e5 で，未コミットのレビューを baseline と内容が異なるファイル（このセッションで変えた，または作ったファイル）に限定し，そのパスだけを jev-lint に渡します．最初から dirty で触っていないファイルは送りません．触ったファイルは HEAD との差分全体で採点するため，そのファイルの以前の未コミット部分は含まれます（baseline がファイル単位の署名であるため．ADR に明記しました）．テスト test_a_file_the_session_never_touched_is_not_sent_even_when_it_is_untracked と test_a_new_file_created_by_the_session_is_sent を追加し，argv の完全一致テストも更新しました．
Review summaryNo blocking findings (no P0 or P1). Two P2s are posted inline:
What I checked and found sound: the token appears only in the 🤖 Generated with Claude Code |
…hanged Check AGENT_MODEL_RULES=off before a stored receipt can make the rules speak. Limit the review of uncommitted changes to the files that differ from the baseline. Files that were already dirty and that the session never touched are not sent. Refs #74 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| record, stop_hook_active=stop_hook_active, budget_seconds=min(model_rules_gate.BUDGET_SECONDS, left) | ||
| ) | ||
| if advice.blocking: | ||
| return {"decision": "block", "reason": advice.message} |
There was a problem hiding this comment.
P2 — When the model rules block, this throws away the verification's non-blocking systemMessage.
Under the "same inputs never block again" rule, a check that keeps failing on unchanged inputs (or an invalid test configuration that has already been reported) shows up only as result["systemMessage"]. If the model rules then find something to block on, this Stop returns only {"decision": "block", "reason": advice.message}. So:
- the user never sees that verification is still failing or unresolved at this Stop, and
- the agent's block reason mentions only the advisory findings, not the failing checks.
Example: verification fails → it blocks once → the agent declines the fix and stops again on the same inputs → verification now only notifies → the model rules have new findings and block. The failing-verification notice disappears from that Stop.
Fix: keep the verification message, for example by adding it to the reason ("\n".join(filter(None, (result.get("systemMessage"), advice.message)))) or by returning it as systemMessage next to the block. No test covers what happens to a verification systemMessage when the model rules block; please add one.
There was a problem hiding this comment.
妥当でした．2315672 で修正しました．モデルルールが block するとき，block の reason を「検証の systemMessage，続けてモデルルールの指摘」にし，検証の報告を捨てません．テスト test_a_model_rules_block_keeps_the_verification_report を追加しました．
|
Review summary: 0 blocking (P0/P1), 1 P2.
Checked and found no problems:
🤖 Generated with Claude Code |
A check that keeps failing on unchanged inputs is reported only through systemMessage. The block reason now starts with that message instead of dropping it. Refs #74 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Review summary (read-only)No blocking findings, and nothing else I'd flag. What I checked:
Known gap, already stated in the PR: the real |
macOS has no parent-death signal, so a hook killed by Claude Code left op and jev-lint running. Run each child under timeout --signal=TERM --kill-after=5s <budget>s. The in-hook process-group kill stays as a backstop. A missing or unusable timeout falls back to it silently. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| start = record.head | ||
| reviews = [] | ||
| if start is not None and head_commit(record.root) != start: | ||
| reviews.append(model_rules_run.Review(record.root, start, RULES_DIRECTORY, cache)) |
There was a problem hiding this comment.
P3: The committed review covers all of start...HEAD, with no limit to the files this session touched. The uncommitted review is limited by _changed_files, so the two behave differently. If the session runs git pull, a merge, or a rebase onto an updated main, other people's upstream commits fall inside start...HEAD. Those commits get sent to the model and count against the shared 20-subject cap. _capped_plan picks files greedily in batch order, so upstream files can push out the session's own files, and the receipt then stops those files from ever being judged on the same inputs. One option is to pass _changed_files(record) as paths here too, or the intersection with git diff --name-only start...HEAD.
There was a problem hiding this comment.
現象は正しく，既知の限界として PR 本文に追記しました．ただし提案された修正では解消しません．_changed_files は「作業ツリーの内容が baseline と異なるファイル」を返すため，git pull や rebase で取り込まれた上流の変更ファイルも baseline と異なり，そのまま含まれます．start...HEAD との共通部分を取っても同じです．baseline は内容のスナップショットで，どのコミットがセッション由来かは持っていないため，この方式では区別できません．区別するにはセッション中の自分のコミットを記録する仕組み（コミット hook など）が別途必要で，この PR の範囲外です．影響は「上流のファイルが共有上限 20 を押し出しうる」ことに限られ，推測ではなく上限の既存の仕様（ファイル単位の貪欲採用，採れなかった分は報告）の範囲内です．未コミットのレビューは，取り込み後の作業ツリーが HEAD と同一なら差分が空なので影響を受けません．
Review summaryNo P0/P1 findings. Checked:
Finding:
|
PATH cannot be relied on to hold GNU coreutils, and a PATH lookup can be hijacked. macOS ships a BSD timeout and Linux a GNU one at /usr/bin/timeout. Use only options both take: -s TERM -k 5 and a plain number of seconds. Drop the --version probe. Fall back to the in-hook kill when the file is absent. Read exit status 124, 137 and -9 (BSD timeout dies of the KILL it sends) as a timeout. Split the model rules tests into delivery and process-boundary files. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Review summary (read-only): no P0–P3 findings. What I checked:
Non-blocking note: the PR description says 🤖 Generated with Claude Code |
Stop 時に，セッションの変更を
jev-lint reviewのモデルルール（M-05，M-21，M-60）で採点する．既存の検証 Stop フローに統合し，指摘は「未採点の入力に対して 1 回だけ block，同じ入力では通知のみ」とする．ADR-0021 をこれに合わせて改訂した．判断してほしい点：下記「ADR の想定と違った点」（
jev-lint reviewは 1 回で「開始 HEAD からの作業ツリー差分」を採点できない）．挙動
{"decision":"block","reason":...}を 1 回（エージェントに見える）systemMessageのみ（findings unchanged; not rerun），再 block しないstop_hook_activeが立っているsystemMessageのみ．見せられなかった指摘は「未提示」として記録し，同じ入力の次の Stop で 1 回 blockAGENT_MODEL_RULES=offop・jev-lintなし / timeout / 非 0 終了 / 読めない JSONsystemMessageの 1 行のみ，block しない，レシートを残さず次の Stop で再試行統合の理由（別 hook にしなかった）
session_gate.stop()が検証の後にmodel_rules_gate.advise()を呼び，results.jsonのレシート（model_rules）を同じ流儀で扱う．verifyの明示コマンドは従来どおり検証のみ．ADR の想定と違った点（実機で確認）
ADR-0021 は「開始 HEAD に対する作業ツリーの差分を 1 回の
jev-lint review --baseで」と書いていたが，jev-lint 0.7.0 のdist/diff.jsを読み，実機（キー不要の--dry-run）で確かめたところ次のとおりだった．--base <commit>は<commit>...HEAD（コミット済みのみ）．ステージ済み・未コミット・未追跡は対象外．--baseなしは「作業ツリー vs HEAD」＋未追跡ファイル（未コミットのみ）．そこで 2 回のレビューに分けた（コミット済み：
--base <開始 HEAD>，未コミット：--baseなし）．上限 20 は 2 つで共有し，同じ rule・file・line の指摘は 1 件にまとめる．開始 HEAD はbaseline.jsonのheadに記録する（古い記録や空 repo では未コミットのみ）．ADR にこの理由を書いた．テストtest_model_rule_files.pyが両モードの対象を固定している．process 境界
jev-lint review --json --no-config --fail-on error -R <rules> -c <cache> [--base <commit>]をop run --environment <id> --の下で実行．件数の事前計測は--dry-run（キー不要，opなし）．--no-config（repo の.jev-lint.yamlが送信先--base-urlを変えるのを防ぐ），--fail-on error（警告の指摘で非 0 終了にならないため）．opの子にだけ渡す．stderr は捨てる．token と key は読まず・出さず・ログに残さない（Credentials.__repr__も隠す）．min(90, 600 - 20 - 検証が使った時間)で，残りが無ければ実行しない（Keychain 検索は 5 秒）．opとjev-lintが残りうる．そこで子プロセスを/usr/bin/timeout -s TERM -k 5 <予算>の下で起動し，hook が死んでも自分の期限で終わるようにした（--dry-runを含む全ての子）．hook 内の process group kill は，その 1 秒後の保険として残す．timeoutの解決：/usr/bin/timeoutを絶対パスで呼ぶ（PATHに左右されず，PATH上の偽物に差し替えられない）．macOS は BSD 版，Linux は GNU 版が同じ場所にある．両方が受け付けるものだけを使う（短いオプション-s TERM -k 5，期限は小数の秒．BSD 版の man page で確認）．--versionの判定（probe）はやめた．/usr/bin/timeoutが無ければ，通知なしで hook 内の kill だけに戻る（その場合，hook が殺されると子は自分で終わるまで残る）．timeoutは自分が属する process group に KILL を送るため，自身がシグナルで死に，Python では負の値で見える）をすべて「timed out」として通知する．timeout，キー不要）： hook を SIGKILL したあと，TERMを受け付ける子は 2.00 秒後（予算 2 秒），TERMを無視する子は 2.96 秒後（予算 2 秒＋-k 1）に終了した．_Child.run経由では，TERMを無視する子が予算 1 秒＋-k 5の 6.01 秒で終わり，「timed out」と判定された．--base <開始 HEAD>はstart...HEAD全体を採点する．セッション中にgit pullや rebase で取り込んだ上流のコミットも対象になり，共有上限 20 を使いうる．baseline は内容のスナップショットで，どのコミットがセッション由来かを区別できないため，この PR では解消しない（claude-reviewの P3，返信済み）．--dry-runのsubjectsとcachedで，キャッシュ済みを除いて 20 以下ならそのまま．超える場合はファイル単位で貪欲に採り，残りを報告．ルール（Python / TypeScript）
agents/hooks/lint/model-rules/<language>/<id>/rule.yml（lint/rulesの ast-grep 静的ルールとは別ディレクトリ）．しきい値 0.5 は未調整の仮値で，助言用．Go と Rust は後続．command-query-naming（M-05）function_definitionfunction_declaration，method_definitionerror-message-names-operation（M-21）function_definitionかつraise_statementを含むthrow_statementを含むknown-defect-marker-names-removal（M-60）decorated_definitionで decorator がxfailcall_expressionでtest.fails/it.failsstate: located（ファイル全体を送る）．他はlocal．キャッシュ書き込みの guard とその限界
secret_path_policy.jsonに**/agent-harness/model-rules/**を追加（テストあり）．この方針は access 種別を区別せず，パスを名指しする呼び出しを拒否する．限界：リテラルのパスしか見ない．実行時にパスを組み立てるコマンドや，書き込むプログラムは対象外で，sandbox は state ディレクトリ配下への書き込みを許している．偶発的・安易な編集への対策であり，意図的な書き換えは防げない（ADR と DESIGN に明記）．新しい lint 規則との関係 / ignore の削除
なし．
検証
uv run --frozen pytest：4642 passed，1 skipped（最終コミット 76bdf08 で実行）timeoutまわりは Red 先行：fake を絶対パスに置き，argv（-s TERM -k 5 <秒>）と終了状態（124，137，-9）のテストを先に変えて，期待どおりの理由で失敗するのを確認してから実装した．file-length（500 行）に当たったため，テストを「判断」と「プロセス境界」の 2 ファイルに分け，共通の準備をmodel_rules_support.pyに出した（テスト数は同じ）．codex-review以外すべて通過（claude-reviewは最新コミットで P0/P1 なし）．codex-reviewは Codex の利用上限（"You have reached your Codex usage limits"）で失敗しており，共通ルールどおり許容（ready, codex skipped: usage limit）．all-greenはその集約として赤．claude-reviewの指摘はすべて対応済み（P1:stop_hook_activeで見せられなかった指摘，P2: Stop 予算，キルスイッチの順序，未触のファイルの送信，検証報告の欠落）．セキュリティレビューの「レシート偽造」は，検証の既存レシートと同じ信頼モデルのため範囲外とし，コメントで回答（別 Issue 候補）．auditの deptry はknown_first_partyに 5 モジュールを追加して解消．ruff check，ruff format --check，ty check：すべて通過security/op/jev-lintを PATH に置く．Keychain・1Password・モデル・ネットワーク不使用，キー不使用）：stop_hook_active，clean，対象 0 件opの env だけ，許可リスト外の変数なし，token が出力に出ないjev-lint review --dry-runで rule の読み込みと subject の選択（Python 4/1/1，TypeScript 3/1/1），コミット済み・未コミット・未追跡の扱い--jsonの findings の形はdist/types.d.tsに合わせた fake で確認しており，実際の出力は新しいマシンの準備（docs/DESIGN.md）の後に一度確認が必要．TSDD に関する正直な注記
実装を先に書き，既存 3602 件が通ることを確認してからテストを書いた（Red を先に取っていない）．その代わり，fake で境界の契約（argv・env・秘密の非露出）を固定し，実
jev-lintで matcher を確認した．ミューテーションテストは未実施．ADR の改訂
「指摘は block しない」を「未採点の入力に 1 回だけ block，同じ入力では二度と block しない」に置き換えた．理由：Stop の非 block 出力（
systemMessage）はユーザーにしか届かず，エージェントが指摘を見られない．毎回 block するとエージェントが指摘を断った場合に同じ入力で止められ続ける．Environment ID と token はともに Keychain から読む（ブリーフに従い，ADR のAGENT_MODEL_RULES_OP_ENVIRONMENTを置き換え）．新しいマシンの準備
docs/DESIGN.mdとdocs/DESIGN_ja.mdの「Stop 時のモデルルール」に記載．スコープ・出典・前提
jev-lintを PATH に入れる．🤖 Generated with Claude Code