Repository navigation
Conversation
bin/fm-pr-check.sh prints one risk line after arming the merge poll when TYPESAFE_API_KEY is present. Code sets the level from paths, deletions and size; Jev answers up to three yes/no questions through bin/fm-jev-lib.sh and may only raise it. An unanswered question never yields low.
Owner
Author
|
Superseded by #45, which carries the same work on a fresh branch cut from current main. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
A research report on using Jev (TypeSafe's decision-only model) in the coding workflow proposed ten integrations in one table. The answer on row 4: "impement"
Row 4 as it was shown:
bin/fm-pr-check.sh, the step that already records a pull request when a worker reports it readyStated with the table: Jev only picks from a fixed list; in every row, if Jev is unsure or down, work carries on exactly as today.
What Changed
bin/fm-pr-check.shnow prints one advisoryrisk: low|medium|highorrisk: not ratedline, with reasons, after it has recorded a pull request and armed its merge poll. The line is printed only whenTYPESAFE_API_KEYis available, and a rating failure never changes the registration or its exit status.bin/fm-pr-merge.shre-registers with the new--no-riskflag, so a merge rates nothing.bin/fm-pr-risk-lib.shsets the level in code from the diffbin/fm-review-diff.shreads: migration, login/permissions or payments paths, or 1500+ changed lines are high; a deleted file, 400+ lines or 20+ files are medium. It then asks Jev up to three yes/no questions (behaviour changed with no test, description does not match the change, something hard to undo). A confident yes can only raise the level, and any unanswered question yieldsrisk: not ratedinstead ofrisk: low.bin/fm-jev-lib.shgainsfm_jev_code_allowed, which reads a per-homeconfig/jev-code-projectslist. A project's diff and pull request text go to Jev only when the project is listed; unlisted projects are rated from the path and size facts alone. The branch also addstests/fm-pr-risk.test.sh(wired intobin/fm-test-run.sh), a--no-riskcase intests/fm-pr-merge.test.sh, and docs indocs/configuration.md,AGENTS.mdanddocs/verification/jev-pr-risk.md.Risk Assessment
✅ Low: The rating is advisory, runs only after the registration and
armed:line, is off without the key, sends nothing for projects not listed in config/jev-code-projects, and the round-3--no-riskfix makes the merge-time re-registration skip it entirely; I found no remaining reachable defect.Testing
Drove the real
bin/fm-pr-check.sh34 times against a fixture git project and task worktree in a throwaway home, with real git, the realbin/fm-review-diff.sh, and real curl over HTTP to a local stand-in for the Jev endpoint (gh and glab were shims because no real pull request exists). Covered: key absent (output, stderr and task record identical to the base commit's script), each code fact, each Jev yes raising the level, Jev unsure/erroring/hanging/unreachable givingrisk: not ratedor leaving a fact-set level untouched, the per-project opt-in sending nothing for an unlisted project,--no-riskat merge time, a secondmate home's ready line, and a GitLab merge request. Also rantests/fm-pr-risk.test.sh(all passed) andtests/fm-pr-merge.test.sh(zero failures, including the merge-time no-rating case); these are mocked tests, not live runs. Not exercised live: a realbin/fm-pr-merge.shmerge (no mergeable pull request on a real forge), and the real api.typesafe.ai model, since noTYPESAFE_API_KEYwas provided to this phase. No UI surface, so evidence is a CLI transcript rather than screenshots. Worktree left clean.COMPARE: stdout, stderr and task meta identical to base commit; 0 requests, 0 curl startsrisk: lowprinted after thearmed:line.sqlpath containing a space, both read highuntestedin code, so only two questions are askedrisk: not rated, neverlowunanswered: ... (Jev unsure)Jev error, not rated, remaining questions skippedrisk: not rated, only one call attemptedmismatchis not asked, the other two are, and the line isnot rated# alphacomment, similar names, wildcards, trailing space): nothing is sent and the forge description is not readunanswered: ... (project not listed)--no-risk) with key present and project listed: no risk line, no change read, no forge description read, no Jev callarmed: state/task-a.check.sh, 0 requests, wall 0.9 sbin/fm-pr-merge.shmerge with the key set prints no risk linebash tests/fm-pr-merge.test.sh(ok - fm-pr-merge re-registers without rating the pull request), which runs the real fm-p…armed:done [key=child-pr-task-a]: child task-a PR ready: https://github.com/o/r/pull/8risk: high - database migration; glab called asmr view 5 -R https://gitlab.example.com/grp/sub/proj -F json(glab was a shim)key on curl argv: 0,curl env key vars: 0, stand-in receivedBearer <the key>Evidence: Live CLI transcript of all 34 registrations
Source: Live CLI transcript of all 34 registrations
Evidence: Driver script that produced the transcript
Source: Driver script that produced the transcript
Evidence: Local stand-in for the Jev endpoint
Source: Local stand-in for the Jev endpoint
Evidence: Representative lines printed beside the registration
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 6 issues found → auto-fixed (3) ✅
bin/fm-pr-check.sh:171- Unrequired component with two side effects: therisk=<level>token appended to a secondmate's parent-channel ready line (lines 171, 177-181). The intent asks only for the level "beside the merge request" inbin/fm-pr-check.sh; nothing requires carrying it to the parent. To support it, the rating now runs BEFOREfm_parent_channel_reportand beforearmed:is printed, so (1) the ready line and thearmed:confirmation wait behind up to 60s diff read + 20s forge read + 3x5s Jev calls, and a kill in that window leaves an armed poll with no ready line published, which is not "work carries on exactly as today"; (2)fm_parent_channel_append_oncededupes on the exact line, so a re-registration whose rating differs (first run Jev timed out -> no token, second run ->risk=high; or the PR changed between runs) appends a seconddone [key=child-pr-<id>]line, breaking the once-only invariant tests/fm-teardown.test.sh asserts ("re-registration duplicated the ready line"). Smallest honest remedy: remove the token and callfm_pr_riskafter thearmed:line is printed.tests/fm-teardown.test.sh:2032- Themain-pr-readycase still runs the realfm-pr-check.shwithFM_ROOT_OVERRIDE="$ROOT"and noFM_HOME, soFM_HOMEdefaults to the developer's checkout andfm_jev_key_loadreads$ROOT/.env. The change fixed the three identical sites at 1209/1217/1246 but missed this one. On a checkout that is a live home withTYPESAFE_API_KEYin its gitignored.env, this test reads the fixture diff and makes realcurlcalls to api.typesafe.ai with the real key (no fake curl in that fakebin), adding network dependence and latency. AddFM_HOME="$case_dir/home"as at the other sites.bin/fm-pr-risk-lib.sh:172- Policy decision needed: with this change any home that already holdsTYPESAFE_API_KEY(set for typed dispatch resolution or wake triage) starts sending each registered PR's title, description, file list and the first 30000 bytes of its source diff to api.typesafe.ai, for every project, with no separate switch. The existing callers under the same key send only routing text/status lines (docs: "sends no brief, pane content, or quota"). The intent implies Jev must see the change, but neither it nor repository instructions say that the existing key is consent to upload project source (including private client repos and anything secret-looking in a diff). Confirm that the shared key is the intended opt-in, or name the gate you want.bin/fm-pr-risk-lib.sh:230- Intent conformance check. Criterion: "If Jev is unsure or down: Shows &test: wait on each stall checkpoint's own effect in wake-queue tests #34;risk not rated&test: wait on each stall checkpoint's own effect in wake-queue tests #34;. Never lowers a level." The change printsrisk: not ratedonly when no fact raised the level; otherwise it prints the code-set level with an; unanswered: ...suffix (lines 230-236), e.g.risk: medium - 1 deleted file(s); unanswered: irreversible (Jev error)where an answeredirreversiblecould have made it high. This reads as a reasonable interpretation of "never lowers a level", but it is not the literal criterion, so confirm it. Related: with the key absent no line is printed at all, although the intent says projects without the no-mistakes pipeline "get a risk level too" and the fact-based level needs no model.bin/fm-pr-risk-lib.sh:84-_fm_pr_risk_factsonly recognises headers matching^diff --git a/. Git quotes paths containing non-ASCII bytes, quotes or backslashes by default (diff --git "a/db/migrations/caf\303\251.sql" ...), so such a file is never classified (a migration, auth or deleted file goes unseen), its---/+++headers and hunk lines are counted against the previous file, and a change made only of such files reads asthe change could not be read. The level is silently too low for the mixed case. Accept an optional leading quote in the header/+++patterns (or have the diff read use-c core.quotepath=false).bin/fm-pr-risk-lib.sh:57- Unrequired component: theFM_REVIEW_DIFF_BINenvironment override. No intent requirement needs a configurable diff reader; it exists as a test seam (and for the synthetic live run in docs/verification/jev-pr-risk.md) yet ships as a production knob that lets the environment substitute the executable whose output sets the level. Recommend removing it and having the tests drive the realbin/fm-review-diff.shagainst a fixture worktree, as tests/fm-teardown.test.sh already does.🔧 Fix applied.
1 warning still open:
bin/fm-pr-risk-lib.sh:95- For a path containing a space, git ends the+++ b/<path>line with a tab, and_fm_pr_risk_factskeeps that tab inpath, so every$-anchored pattern misses. Confirmed by running the parser on a realgit diff:my schema.sqlgivesmigration=0, andsrc/my parser.test.jsgivestests=0 code=2. Effect: a root-level.sqlorschema.rb/schema.prismafile with a space in its name is not rated high, and a test file with a space is counted as code, sountestedis asked (or left unanswered) instead of being settled in code. The level is silently too low, and the tab-suffixed path is also what is sent to Jev. Fix: strip the trailing tab when taking the path from the+++line, e.g. addsub(/\t$/, "", path)before the closing-quote strip.🔧 Fix applied.
3 issues (1 warning, 2 infos) still open:
bin/fm-pr-check.sh:184-bin/fm-pr-merge.shcallsfm-pr-check.shon every merge (record_pr_metadata, fm-pr-merge.sh:862, invoked at :1131) while it holds the task's.control-<id>.lockand before the forge merge command. Because the rating now runs unconditionally at the end offm-pr-check.sh, a home withTYPESAFE_API_KEYre-rates the pull request inside every merge: a secondfm-review-diff.shrun (fetches, up to 60 s), and for a listed project a second forge read (20 s) and up to three more Jev calls that re-send the title, description, file list and diff excerpt. The merge is delayed by that much with the control lock held, and arisk:line is printed into the merge output after the merge has already been authorized. The intent places the level at "the step that already records a pull request when a worker reports it ready"; nothing requires a rating at merge time, and the docs' cost statement ("adds at most one change read ... to a registration") does not mention it. Decide whether the merge-time re-registration should skip the rating; the smallest remedy needs a way for the merge caller to say so (or moving thefm_pr_riskcall out of the pathfm-pr-merge.shuses), which is a behaviour choice rather than a mechanical fix.bin/fm-pr-check.sh:184- The opt-in is matched on the last path component of the task'sproject=(${PR_PROJECT##*/}), so two projects in different parent directories that share a directory name are opted in together by one line inconfig/jev-code-projects. This is documented ("the directory name of the task'sproject=metadata") and matches the supervisor's instruction to list project names, so no change is requested; noted only because the consequence of a collision is an unlisted repository's diff being sent.bin/fm-pr-risk-lib.sh:90- A file renamed with no content change has no+++ b/line, so_fm_pr_risk_factskeeps the old path taken from thediff --git a/...header. Moving an unchanged file into e.g.db/migrations/orauth/is classified by where it came from, not where it lands. Rare and fail-soft (the other facts and questions still apply); reading therename toline would close it.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
COMPARE: stdout, stderr and task meta identical to base commit; 0 requests, 0 curl startsrisk: lowprinted after thearmed:line.sqlpath containing a space, both read highuntestedin code, so only two questions are askedrisk: not rated, neverlowunanswered: ... (Jev unsure)Jev error, not rated, remaining questions skippedrisk: not rated, only one call attemptedmismatchis not asked, the other two are, and the line isnot rated# alphacomment, similar names, wildcards, trailing space): nothing is sent and the forge description is not readunanswered: ... (project not listed)--no-risk) with key present and project listed: no risk line, no change read, no forge description read, no Jev callarmed: state/task-a.check.sh, 0 requests, wall 0.9 sbin/fm-pr-merge.shmerge with the key set prints no risk linebash tests/fm-pr-merge.test.sh(ok - fm-pr-merge re-registers without rating the pull request), which runs the real fm-p…armed:done [key=child-pr-task-a]: child task-a PR ready: https://github.com/o/r/pull/8risk: high - database migration; glab called asmr view 5 -R https://gitlab.example.com/grp/sub/proj -F json(glab was a shim)key on curl argv: 0,curl env key vars: 0, stand-in receivedBearer <the key>bash ~/.no-mistakes/evidence/01M46JQAD2T0BB7FNC9WPS14PQ/drive.sh— 34 live runs ofbin/fm-pr-check.sh task-a <url>in an isolated FM_HOME, output in transcript.txtBase comparison:git archive f6a10db binextracted to a temp dir and itsfm-pr-check.shrun on the same keyless registration; stdout, stderr and task meta compared identicalbin/fm-pr-check.sh task-a <url> --no-riskwith key present and project listed: zero requests, zero curl starts, zero forge description readsbash tests/fm-pr-risk.test.sh— all four groups ok,# all fm-pr-risk tests passedbash tests/fm-pr-merge.test.sh— 0not oklines;ok - fm-pr-merge re-registers without rating the pull request(mocked forge, not live)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.