Repository navigation
Conversation
…v, opt-in With TYPESAFE_API_KEY present, bin/fm-spawn.sh points each ship and scout worker's git at a per-task hooks directory through one exported setting, so nothing is written into the project. The commit-msg hook stops an added line matching a fixed credential pattern, warns about a filler subject from a word list, and - only for a project listed in config/jev-code-projects - asks Jev in one request whether the message is filler, contradicts the change, the change carries debug leftovers, or a staged file is unaccounted for. Every Jev outcome lets the commit through; the project's own hooks keep running. Adds fm_jev_code_allowed to bin/fm-jev-lib.sh as the shared per-project gate for sending code.
Owner
Author
|
Superseded by #50, which carries this change's final content together with the other three Jev items on one branch. |
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 5: "implement". Later, on merging: "merge all jev items".
Row 5 as it was shown:
bin/fm-spawn.sh)Stated 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.
Rulings given the same evening on earlier rows, which apply to every Jev item: nothing a worker typed in its shell is ever sent to TypeSafe; no project's source code or diff is sent unless that project is explicitly named for it; Jev advises and never becomes the only thing that blocks, merges, approves or discards anything; a fix or feature adds nothing beyond what the row describes.
What Changed
bin/fm-commit-check.sh, wired intobin/fm-spawn.shfor ship and scout launches. WhenTYPESAFE_API_KEYis available and the project is listed in the newconfig/jev-code-projects(checked byfm_jev_code_allowedinbin/fm-jev-lib.sh), spawn writes a hooks directory under the task's temp root and exportsGIT_CONFIG_PARAMETERS(core.hooksPath) into that worker's pane only. Nothing is written into the project or its git config, and the repository's own hooks are forwarded to. Without the key or the listing, the launch is unchanged.commit-msghook acts only on commits to the task's own worktree. By pattern matching alone, it stops a commit whose added lines match a recognised credential format (private key block, AWS, GitHub, Slack, Google,sk-keys), naming file, line and kind but never the value; a quoted password/secret/token literal only prints an advisory. It then sends Jev one request holding the commit message and staged file names only (no diff or file content) with up to three yes/no questions: filler, contradicts the file names, unmentioned file. A confidentyesprints one advisory line; every Jev outcome, including timeout or error, lets the commit through. Debug leftovers are not judged, because no content is sent. Merges, rebases, cherry-picks, reverts andfixup!/squash!/amend!commits are skipped.tests/fm-commit-check.test.sh(registered inbin/fm-test-run.sh), a "Commit check" section indocs/configuration.md, theconfig/jev-code-projectsentry inAGENTS.md, anddocs/verification/commit-check.md, which records that the final message-and-file-names request has not yet been run against the live endpoint.Risk Assessment
✅ Low: The last fix round is a one-line diff-filter change plus one behavioural test and a doc sentence, traced correct for type changes, copies, renames, deletions and quoted names; the only side effect is a stricter stop on moved files, which the user's instruction asked for.
Testing
Ran the change's own suite
tests/fm-commit-check.test.sh(all 17 cases pass), then drove 22 realgit commitruns through the hooks--installwrites in a throwaway home and repository, with a localcurlstand-in recording every request. All driven scenarios passed: gating, one names-only request per commit, advisory lines, commit goes through when Jev is unsure or down, credential stop including four adversarial placements, scope limited to the task's worktree, and the project's own hooks still deciding. No UI surface, so the evidence is a CLI transcript. The live typesafe.ai endpoint and a real-backend launch through fm-spawn.sh were not exercised live.Evidence: Transcript of real git commits through the installed commit check hooks
Source: Transcript of real git commits through the installed commit check hooks
Evidence: Driver script that produced the transcript
Source: Driver script that produced the transcript
Evidence: Request sent for a two-file 'wip' commit, and the credential stop
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-spawn.sh:4687- The hooks are attached through an exportedGIT_CONFIG_PARAMETERS, so they fire for everygit commitin the worker's process tree, in any repository, while<home>and<project>are baked in at install (bin/fm-commit-check.sh:125). Nothing infm_commit_checkchecks that the repository being committed to is the task's worktree. Concrete sequence: a worker runs the project's test suite, whose fixturesgit inita temp repo and commit.tests/lib.shdoes not clear the variable (only tests/fm-test-fixtures.test.sh:60 and the new test do), so this applies to firstmate's own suite. Each fixture commit then (a) is refused if a fixture line matches a credential pattern, (b) getscommit-check: advisory...on stderr for subjects likewip, which breaks tests asserting on commit stderr, and (c) for a listed project makes one Jev request per fixture commit, sending that other repository's message, file names and diff under the listed project's name. Point (c) conflicts with the ruling "no project's source code or diff is sent unless that project is explicitly named for it", because the name gate is applied to the spawn-time project and not to the repo actually committed. Smallest remedy: pass the task worktree to--installand have--hookonly forward to the repo's own hook when the current repository is not that worktree. Marked ask-user because it touches a stated ruling; the fix itself is a correction, not an extension.bin/fm-commit-check.sh:83- Thepassword or secret literalpattern refuses commits for ordinary non-secret code. Probed against the shipped pattern, each of these added lines matches and would stop the commit:token_type = "access_token","tokenizer": "bert-base-uncased",TOKEN_URL = "https://example.com/oauth",secret_name: "my-app-db-secret". This is the only hard stop, and it applies to every ship and scout worker in every project as soon as a key exists (no project opt-in). The printed way past is--no-verify, which also skips the project's own pre-commit and commit-msg hooks, so an autonomous worker that hits a false positive will bypass the project's hooks. The row says the check "blocks only credentials"; whether to narrow this pattern (for example require the name to end at the keyword, or exclude URL and identifier-shaped values) or make it warn-only is a product decision.bin/fm-commit-check.sh:194- Simplification: the code-side filler detector (FM_CC_FILLERat line 73, lines 187-194, and thefiller_knowngating at line 210) is not required by row 5, which describes filler as one of the questions in the single Jev request; the ruling says a feature "adds nothing beyond what the row describes". It also runs for projects that are not listed. It carries a defect: the subject is reduced withtr -cd 'a-z0-9 ', so a subject with no ASCII letters or digits (probed:Добавить счётчикgives an empty word) hits line 194, is told "the message does not say what changed or why", and the Jev filler question is then suppressed. Removing the word list and always asking Jevfilleris the smallest honest remedy and removes the defect with it.bin/fm-commit-check.sh:125- Intent conformance. The row says "Where it plugs in: A git pre-commit hook set on each worker's isolated copy at launch" and "Build size: Small (adopt the existing tool)" (jev-commit). The change instead installs acommit-msghook reached through an exportedcore.hooksPathplus 17 forwarder hooks, and is a 253-line reimplementation that does not adopt jev-commit. Both deviations look deliberate and defensible: a pre-commit hook cannot see the message, a hook written into a worktree would land in the shared git directory, and jev-commit has no per-project gate for sending diffs. They still differ from the row as written, so the author should confirm them.bin/fm-commit-check.sh:166- Ongit commit --amend,git diff --cachedis the delta against HEAD, not the amended commit's whole change. An amend that only rewords is skipped as "nothing staged", and an amend that adds a small follow-up is judged against a message describing the whole commit, which can produce a spuriousunmentionedorcontradictsadvisory. Advisory only, so the commit still goes through.🔧 Fix applied.
4 issues (2 warnings, 2 infos) still open:
bin/fm-commit-check.sh:204- Introduced by the fix round that made the generic password/secret literal advisory. For a project listed inconfig/jev-code-projects, an added line such asdb_password = "hunter2-hunter2"insrc/db.pynow prints the advisory at line 204 and then falls through to line 213, where the full staged diff of that file, including the literal the code has just flagged as a possible credential, is sent to TypeSafe. Before the fix round this line stopped the commit, so it was never sent.house_rules_path_skippedonly filters by path (.env,*secret*, ...), not by line, and the only test for this case (tests/fm-commit-check.test.sh:178) uses an unlisted project, so the send is not exercised. The intent says "Passwords and keys are caught by plain pattern matching, not by Jev" and allows a named project's diff to be sent, but does not say whether a line the pattern matcher flagged may still be sent. Policy decision needed: either send as now, or leave flagged lines (or the whole request) out when the literal pattern matched.bin/fm-spawn.sh:4689- With a key present, every ship and scout worker in every project (no project opt-in) runs withcore.hooksPathforced throughGIT_CONFIG_PARAMETERS. The forwarders keep existing hooks running, but tools that install hooks read the effectivecore.hooksPath:pre-commit installrefuses outright when it is set ("Cowardly refusing to install hooks withcore.hooksPathset"), andgit lfs install/lefthook installtarget the task's temp hooks directory, where a forwarder of the same name already exists, instead of the repository. A worker that runs a project's setup step (make setup, apreparescript) calling one of these gets a failure or a hook written into/tmp/fm-<id>/git-hooksthat replaces the forwarder. This is a consequence of the confirmed core.hooksPath design, not a coding slip, so the author should decide whether it is acceptable or needs documenting in docs/configuration.md "Commit check".bin/fm-spawn.sh:4715-GIT_CONFIG_PARAMETERSwas added to the names the opt-inenv -ilaunch filter always retains, for every kind including second mates and whether or not a key exists. A pane whose shell already carries an ambientGIT_CONFIG_PARAMETERSused to have it filtered out and now passes it to the agent, which is a small departure from "with the key absent a worker is launched exactly as it does without the feature". Only reachable when the operator's pane environment already has that variable set; retaining it only when the install succeeded would close it.bin/fm-commit-check.sh:160- For a staged path containing a space, git ends the+++ b/<path>header with a tab, sofilekeeps a trailing tab andcut -f1at lines 190 and 203 drops the:<line>part. The credential stop and the literal advisory still fire and name the file, but without the line number the header promises. Stripping a trailing tab fromfilefixes it.🔧 Fix applied.
2 issues (1 warning, 1 info) still open:
bin/fm-commit-check.sh:221- The recognised-format credential patterns (FM_CC_CREDENTIALS) are applied only to added lines (line 196-211, from the-U0added-line list), while the diff sent to Jev at line 221 is-U3and so includes removed and context lines, filtered only byFM_CC_SECRET_LITERAL. For a listed project, a commit that removes a hard-coded credential that is not a quoted password/secret/token/api-key assignment - e.g.-aws_key: AKIAABCDEFGHIJKLMNOPin a YAML or shell file, or an embedded-----BEGIN RSA PRIVATE KEY-----block and its body in a.py/.yamlfixture - or that merely edits a line within three lines of one, is not stopped (nothing matching was added) and sends those lines verbatim to TypeSafe.house_rules_path_skippedonly filters by path. The intent says "Passwords and keys are caught by plain pattern matching, not by Jev" and the earlier ruling was that a line the matcher flagged is never sent, but neither says whether a recognised-format credential on a removed or context line may be sent. Policy decision needed: send as now, or also replace diff lines matchingFM_CC_CREDENTIALSwith[line withheld](private-key body lines would additionally need the whole file left out). No test covers a removed or context credential line.bin/fm-commit-check.sh:222- The withholdingsed -E "s#^(.).*(...).*#...#"runs in the ambient locale. In a UTF-8 locale GNU sed's.does not match an invalid byte, so a diff line that has a non-UTF-8 byte before the literal (a Latin-1 comment or string in a legacy-encoded source file) is not rewritten and is sent in full. Reproduced with GNU sed 4.9 underLC_ALL=C.UTF-8:+# caf\xe9 db_password = "hunter2-hunter2"passes through unchanged, while the same line is withheld once the invalid byte is absent. This breaks the documented "A diff line that matches the password-or-secret-literal pattern is never sent". Running that sed (and, for consistency, the greps at lines 199 and 212) underLC_ALL=Cmakes matching byte-wise and closes it.🔧 Fix applied.
2 issues (1 warning, 1 info) still open:
bin/fm-commit-check.sh:230- The text sent to Jev is built by a secondgit diff --cachedlimited to the kept file names as pathspecs, and that pathspec changes what git reports. Two cases reproduced in a throwaway repository with the realfm_commit_checkand a stubbedfm_jev_choices: (1) Rename:git mv a.py b.pyplus one appended line. With onlyb.pyas pathspec git cannot pair the rename and reports the file as new, so all 30 unchanged lines of the file were sent as+lines alongside the one added line. This contradicts the round-3 ruling and the docs ("Lines the change removes and lines it leaves unchanged are never sent") and makesleftoversjudge old code as newly added. (2) A file replaced by a directory of the same name:git rm config, then addconfig/.env. The deletedconfigis kept (not path-skipped) and its pathspec prefix-matchesconfig/.env, so+DB_URL=postgres://u:pw@h/dbwas sent under=== config/.env ===even thoughhouse_rules_path_skippedexcludes that path. Names with glob characters are a third route of the same kind. Line-level withholding and the private-key file exclusion still apply, so recognised credential formats stay out, but unflagged content of a secret-shaped path does not. The round-3 tests miss this because they never stage a rename or a file-to-directory replacement. Fix: drop the second diff and buildsentfrom the already computed$added(rename-aware, exact file names), keeping only lines whose file is inkept. That is also less code.docs/verification/commit-check.md:9- The verification note says honestly that every live row was produced by the earlier unified-diff request, and that the added-lines-only request with its reworded instructions andcontradictscriterion "has not been run live". So how the model answers the request the final code actually sends, especiallycontradictswhen the message describes a removal that added lines cannot show, is unverified. The impact is bounded: every Jev outcome is advisory and exits 0. A live re-run of the five rows before relying on the advisories would close this.🔧 Fix applied.
5 issues (1 warning, 4 infos) still open:
bin/fm-commit-check.sh:162-_fm_cc_added_linesblanksfilewhenever git C-quotes the+++header ($0 ~ /^\+\+\+ "/), and hunks are only read whenfile != "". Git still quotes a name holding a double quote, backslash, tab or newline even withcore.quotePath=false, so every added line of such a file is dropped from$added. Reproduced in a throwaway repository: withwe"ird.pyandtab<TAB>x.pyeach holding a GitHub-token-shaped line plusok.py, the function printed onlyok.py:1, so neither the credential stop nor the secret-literal advisory can fire for those files and the commit goes through. Rare names, but this is the one check that is supposed to stop a commit. Fix: keep the quoted header text as the file label (e.g.substr($0, 5)) instead of blanking it; the quoted form escapes tabs, socut -f1still works. No test stages such a name.bin/fm-commit-check.sh:183- The message is always passed throughgit stripspace --strip-comments, butgit commit -muses cleanup modewhitespace, which keeps lines starting with#. Reproduced: a-mmessage#123 fix parser+ blank +bodyis committed with that subject, while the check sendsbodyas the whole message (a one-line#123 fix parseris sent as an empty message). Jev then judges a message that is not the one committed, and thefixup!/squash!/amend!test reads the wrong line. The effect is limited to a wrong or missing advisory line; the commit still goes through. The hook cannot see the cleanup mode, so there is no clean mechanical remedy; the existingponytail:comment covers only the comment character, not this case.bin/fm-commit-check.sh:186- The file list isgit diff --cached --name-only, which compares the index with HEAD. Forgit commit --amendthat is only what was staged since the commit being amended, not the files the amended commit will contain. Example: a worker amends a five-file commit after staging one more file; the message describes the five-file change,commit.fileslists one name, and thecontradictscriterion ("says a named file was changed and that file is not listed") can answer yes. An amend that only rewords is skipped as "nothing staged". Advisory only, and a commit-msg hook has no reliable way to detect an amend, so this is a limitation to document rather than a fix.tests/fm-commit-check.test.sh:197- Line 184 says the credential fixtures are "Built from pieces so this file never holds a line the check itself would stop", but line 197 holds a literal-----BEGIN RSA PRIVATE KEY-----, which matches theprivate keypattern. If this repository is ever listed inconfig/jev-code-projects, a worker commit that adds or moves that line is stopped by the check. Build the header from pieces likeghtokenandakia(e.g.pk="-----BEGIN RSA PRIVATE"; stage src/key.pem "$pk KEY-----\\n").bin/fm-commit-check.sh:57- Intent row 5 lists "are there debug leftovers (prints, commented-out blocks, skipped tests)" among the questions asked, and the final code asks onlyfiller,contradictsandunmentionedand sends no change content. This is not re-raised as a contradiction because the round-4 instruction explicitly ordered it: "stop sending any change content at all ... drop the leftovers question, since it cannot be judged without content, and say so in the header and docs". The header (lines 55-58), docs/configuration.md and the verification note all say so. Recorded only so the deviation from the row text is visible at merge.🔧 Fix applied.
2 issues (1 warning, 1 info) still open:
bin/fm-commit-check.sh:159-_fm_cc_added_linesrunsgit diff --cached ... --diff-filter=AMR, so a staged file whose status isT(type change) orC(copy) contributes no added lines, and neither the credential stop nor the secret-literal advisory can fire for it. Both reproduced in a throwaway repository with the real function: (1) a tracked symlinklinkreplaced by a regular file holding a GitHub-token-shaped line is reported asT linkand the function printed nothing; (2) withdiff.renames=copiesin the committer's git config,copy.py(a copy of a modifiedbase.pyplus an appended token line) is reported asC085 base.py copy.pyand the function printed onlybase.py:41. In both cases the commit goes through with the credential. This is the same class as the quoted-name gap fixed in the last round: rare inputs, but in the one check that is meant to stop a commit. Fix: replace--diff-filter=AMRwith--diff-filter=d(everything except deletions; a deleted file's+++ /dev/nullis already handled by the awk), with one test staging a symlink-to-file replacement.docs/verification/commit-check.md:33- The "Portable coverage" paragraph lists what the suite asserts and ends with "a stop for a path holding a space names its line"; the quoted-name stop added in the last round is not listed. The round-5 instruction said "Nothing else changes", so this is recorded only; the code and test are correct (traced:+++ "b/we\"ird.py"yields the label"b/we\"ird.py":1, and the new test fails without the fix because the check previously returned 0).🔧 Fix applied.
1 info still open:
bin/fm-commit-check.sh:159- With--no-renames, a renamed or copied file is diffed as a whole new file, so every line of it counts as added. A puregit mvof an already-tracked file that holds a credential-shaped line (for example a fixture key file moved to another directory, with no content change) is now stopped, and a moved file with password-style literals lists each such line in the advisory. Before this round only the lines actually added to a renamed file were scanned. The header and docs/configuration.md still say "an added line". This is the direct result of the round-6 instruction ("with --no-renames or equivalent so copies and renames contribute their added lines"), and it errs on the stopping side, so it is recorded only; the narrower form would be-M -Ckept on with--diff-filter=d, which scans only the lines a rename or copy adds.✅ **Test** - passed
✅ No issues found.
bash tests/fm-commit-check.test.sh- 17 cases, all passbin/fm-commit-check.sh --installagainst a home with no key, an unlisted project, a non-git directory, and a listed project with a key22 realgit commitruns under the exportedGIT_CONFIG_PARAMETERSwith a recordingcurlstand-in (driver saved as commit-check-drive.sh)Stand-in endpoint modes: yes, no, confidence 0.55, HTTP 500, malformed body, curl exit 7Credential placements: GitHub token, AWS key in a file whose name git quotes, private-key header in a symlink replaced by a regular file, token on a line with an invalid UTF-8 byte underLC_ALL=C.UTF-8, a message telling the checker to ignore the credential, removal of a tracked credentialSearched every recorded request body for staged-content markers (new code,config/.envbody, lockfile body, line appended to a renamed file, password literal)Commits under the same setting in an unrelated repository and in a second worktree of the same repositoryCommit with the project's ownpre-commitand a refusingcommit-msghook presentCommit after the project was removed fromconfig/jev-code-projects✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.