Skip to content

Skills installer: do not delete content the pinned CLI did not write (F3), and align the status check - #1035

Merged
seathatflowsinourveins merged 8 commits into
mainfrom
claude/cc-native-practice-f3-foreign-content-20261012
Oct 12, 2026
Merged

Skills installer: do not delete content the pinned CLI did not write (F3), and align the status check#1035
seathatflowsinourveins merged 8 commits into
mainfrom
claude/cc-native-practice-f3-foreign-content-20261012

Conversation

@seathatflowsinourveins

@seathatflowsinourveins seathatflowsinourveins commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Scope

  • What this PR changes, in one or two sentences: F3 of Require the Claude Code link in the installer's ok state for a global skill #1024 (research-queue row 42): the skills installer no longer lets the pinned CLI's add delete content it did not write without --force. Wherever an add is about to run over an existing folder (an unlocked canonical folder; a folder locked at the pin whose Claude link, or in a project whose Claude target, is missing or whose SKILL.md was edited; a folder whose lock does not name another revision of the manifest's source in the form the CLI writes one (a global skillFolderHash that is a git tree id, a project ref that is a full commit id): every other form the CLI writes or leaves, such as its own digest of the folder, a URL source, a branch or a tag as ref, an optional field missing, a lock file it reads as empty, is read as naming nothing; a Claude-only copy; a project) the folder must be the pinned tree: an extra file, an edited or missing file, a regular file or link that merely bears the name of a runtime directory (.venv, node_modules, __pycache__), and an entry a git tree cannot hold (an empty directory, a FIFO, a socket, a device) are refused as local-modified. The status check agrees with the installer on the layouts they disagreed on (a link at the canonical skill path for an entry installed for Claude Code, a link on a parent folder, a file or link named like a runtime directory), and three text items from the reads of Require the Claude Code link in the installer's ok state for a global skill #1024 (P3-2, P3-3, P3-5) are fixed. The narrower canonical-link rule that Require the Claude Code link in the installer's ok state for a global skill #1024's ruling left to this change is not taken (decision record).
  • Base commit: 2f59d7448c7e4a51cf592f78818838353ba2dca9 (main after Reconcile the MCP templates with the live registrations: code graph key, promptfoo and a drift report #1034, the target of the second rebase of 2026-10-12 below; the first rebase of the day targeted 66584736c076873ca7e5898e3bd8111cca1a1447, main after Validate dated grant counts and the newest advisory delta #1030). The branch was cut at 83cec12eec323cf3fc3d62177e38fe4163dbd3bb (main after Add fenced Claude usage sampling and Grafana panels #954), which stays the historical base of the first reads.
  • Lane: lane:foundation
  • Owned paths touched: tools/adoption/install_skills.py, scripts/skills_status.py, adoption/skills/lifecycle.md, docs/decisions/2026-10-12-skills-installer-foreign-content.md, docs/decisions/2026-10-10-daily-skill-invoke-rate.md (two links to moved lines), tests/test_install_skills.py, tests/test_skills_status.py, manifests/evidence.json.
  • Commits, in order: (1) the installer rule with its tests, the --force help text and the lifecycle paragraph; (2) the status check's two layouts with their tests; (3) the evidence registry; (4) the fix wave for the first reads of 295ada5e; (5) the fix wave for the second reads of 9a77a96e; (6) the fix wave for the third reads of 5ace90c1; (7) the fix wave for the fourth reads of 453ac683, by the command center's adjudication; (8) the fix wave for the fifth reads of 23e2dcbe, released by the command center (all below). Each fix wave is one commit that re-registers its own files.
  • Rebased on 2026-10-12 onto main 66584736c (the PR was DIRTY: a conflict in manifests/evidence.json only). It is a pure rebase: each commit's diff without the registry has the same patch-id, so has the whole branch diff, and the registry rows of the 7 files the branch registers or changes are identical to those of the old head except the row of docs/decisions/2026-10-10-daily-skill-invoke-rate.md, which main changed in Validate dated grant counts and the newest advisory delta #1030 and which now hashes main's version with this branch's two link edits; the registry conflicts were resolved by writing each commit's rows again with scripts/host_receipts.py (register_file) at the rebased bytes. The commit names in this description are the heads the reads were made on: 7749e5181 is now 4f7c57c1f, 2f9bdde88 is 1b2331db8, 295ada5e8 is dc01ad015, 9a77a96e8 is 2c289e529 and the fix wave of commit 5 is 5ace90c12.
  • Rebased a second time on 2026-10-12 onto main 2f59d7448 (Reconcile the MCP templates with the live registrations: code graph key, promptfoo and a drift report #1034 landed; the PR was DIRTY again, in manifests/evidence.json only), at the command center's request, as a pure rebase with the same method: each commit's diff without the registry has the same patch-id (7 of 7), so has the whole branch diff, and the registry rows of the 7 files the branch registers or changes are identical to those of the old head (7 of 7); the registry conflict in the commit that registered the F3 files was resolved by writing each commit's rows again with register_file at the rebased bytes. The heads of the reads are now: 4f7c57c1f is be47e5c19, 1b2331db8 is bbfa8ac96, dc01ad015 is c6565ef95, 2c289e529 is cabd3c89a, 5ace90c12 is 091848186, 453ac6834 is 59f476caa and 23e2dcbe9 is 5f23fc358. Astra's delta read of 23e2dcbe9 was made before this rebase; the command center's tests-only check carries through it.

Fix wave 5 (commit 8), for the fifth reads at 23e2dcbe

Released by the command center for ONE commit on 5f23fc358 (its tests-only Claude check passed at 23e2dcbe9 and carries through the pure rebase: pr1035-claude-verify-23e2dcbe.md). Input: Astra's delta read of 23e2dcbe9 (CHANGES_REQUESTED: three P1, one P2 carried under the scope lock, seven P3; GPT-VERDICT-23e2dcbe9.md, 30,112 bytes, sha256 2ec782f8124d7c931a45609228508cf0bd2c98aa80d45ffb5d9686eb925fd958). Fail-first: the final tests.test_install_skills and tests.test_skills_status (212 tests) give FAILED (failures=76, errors=1) on 5f23fc358 (fail-before-5f23fc358.txt) and pass here (136 + 76).

Finding Fix Regression test
P1 1 (Astra blocker 1): a lock that holds NaN, Infinity or -Infinity still lets the add run over local work: json.loads accepts the tokens, JSON.parse throws (src/skill-lock.ts L85, src/local-lock.ts L80) and both readers return an empty lock (L99-L101, L97-L98). Reproduced on the complete installer path and with the real pinned CLI load_lock passes parse_constant=reject_constant, CPython's hook for invalid JSON numbers (Doc/library/json.rst L324-L329, Lib/json/decoder.py L46-L49 at python/[email protected]): a lock with a constant anywhere is unreadable and the folder is compared like an unlocked one. 1e999 is a valid number, parsed as Infinity by both, and stays readable. Measured: the pinned reader bodies executed on Node 24.21.0 (reader-measure-20261012.txt) LOCK_FILE_TABLE GL11-GL16 and PL10-PL15 (the version, another entry, the entry itself, a field outside skills) through the classifier and the whole installer path (refused, no add, folder untouched; the pristine and absent-folder controls install, but for the six forms of NO_CLEAN_INSTALL_CONTROL, whose reasons are in the test); the unit test of load_lock on text; the real-CLI control for the global scope and the three tokens
P1 2 (Astra blocker 2): an invalid local-source sibling does not invalidate a project lock, so an unrelated entry excuses the deletion lock_skills reads a project lock as empty for a null entry and for a local entry whose source is missing or is not text, in the managed entry or in any other (src/local-lock.ts L90-L98: isAbsolute(entry.source) throws). The description is corrected: the reader does not throw on an entry that is a string, a number, true or a list (measured), and this installer still reads such a lock as empty, which is stricter and now documented as such PL16-PL22 and LOCK_FILE_STRICTER_TABLE PS1-PS4 through the classifier and the installer path; the cases of lock_skills (text, empty text, Local in capitals, a non-local entry with a null source, the global reader); the real-CLI control for the project scope (null, number and list sources)
P1 3 (Astra blocker 3, the brief's privacy rule): two email literals in the new Git fixture The fixture's author and committer address is no-address, which is not an email (git takes it); no email-shaped literal is left in the added lines the real-CLI generic-Git test still passes
The fake CLI read a lock more leniently than the pinned readers, so a damaged lock survived its own add and the clean-install controls could not run The fake's load_lock mirrors the readers (a constant, a version that is not a number or is below the first, a falsy skills, and in a project a null entry or a local entry whose source is not text) 4 mutants of the fake's reader, all killed

Mutation check of the change: 10 mutants of the new reader rules (the hook missing, rejecting NaN only or all but -Infinity; the local-source rule missing, for null only, for every entry or for a missing source accepted, its test inverted; entries that are not objects kept; the per-entry rule applied to the global reader) and 4 of the fake's reader: all killed (survivors: none).

Fix wave 4 (commit 7), for the fourth reads at 453ac683

By the command center's adjudication (adjudication-pr1035-453ac683-20261012.md: the lock-shape class recurred, so the fix starts from the upstream definition, enumerates every lock-entry form the pinned CLI writes or leaves, builds the test table from it and fails closed for anything outside it). Inputs: Astra's delta read (CHANGES_REQUESTED: 1 P1, 1 P2, 7 P3) and the command center's Claude delta (CHANGES_REQUESTED: B1 at P1, one P2, five P3). Fail-first: the final tests.test_install_skills and tests.test_skills_status (208 tests) give FAILED (failures=41, errors=85) in 5 tests on 453ac683 (fail-before-453ac6834-final.txt) and pass here (132 + 76).

Finding Fix Regression test
P1 (Astra blocker 1; B1 of the command center): a global entry whose skillFolderHash is any non-empty text that is not the pin's tree was read as an older pin and sent to install before the content check: Astra's not-a-tree, 39 hex digits and one space; the CLI's own sha256 digest of the folder, which src/add.ts L2108-L2111 writes where the GitHub tree lookup returns nothing and for every gitlab, git and local source; an entry of another source or type (src/add.ts L2114-L2122 records both for every global install). The add recreated the folder (src/installer.ts L195, called at L389) and deleted local work without --force One reading, lock_reading, built from the table below: an entry names another revision only through a field the CLI writes as one, in that form (a global skillFolderHash that is a git tree id, 40 hex digits matched case-insensitively as src/update.ts L638 does; a project ref that is a full commit id) and only for the manifest's GitHub source. A tree id equal to the pin's is a content address and reads "same" whatever source wrote it. Everything else is unidentified and the folder is compared like an unlocked one. ok needs the whole entry the table tests (below), and with the real pinned CLI for a generic git source (it writes the digest, a URL and no ref; the native add deletes the extra file), global and project
The enumeration (adjudication ruling 2, wave rule 4): every lock-entry form the pinned CLI writes or leaves, project and global, cited, with a test table built from it The "Lock entries" section of the decision record: the lock file around an entry (3 rows), the global entries (8 rows) and the project entries (4 rows), each with its src/add.ts, src/skill-lock.ts, src/local-lock.ts, src/update.ts, src/blob.ts or src/sync.ts lines at the pin. LOCK_ENTRY_TABLE (53 forms: 26 global, 27 project) and LOCK_FILE_TABLE (20 forms) of tests/test_install_skills.py are built from it. Anything outside the table is unidentified and compared (fail closed) test_the_lock_entry_table_gives_the_classifier_its_reading_for_every_form (every form x global, project/universal, project/claude-code x pristine, local file, folder gone), test_a_lock_file_the_pinned_cli_reads_as_empty_has_no_entries, test_the_installer_path_follows_the_table_in_the_state_that_destroys (the full installer path with the fake CLI, --check-only and an ordinary run, --force for the representative forms), test_lock_skills_holds_only_what_the_pinned_cli_reads, test_the_source_tree_oracle_decides_a_project_entry_that_says_the_pin
P3-2 (command center): a project ref that is a branch or a tag counted as another revision (src/local-lock.ts L19-L20: what the user typed) Only a full commit id other than the pin's is another revision; a branch, a tag, an abbreviated id, an empty or a malformed ref is compared rows P3a to P3h of the table
P3-3 (command center): the guard read entries from a lock file that the CLI reads as empty (src/skill-lock.ts L89-L101, src/local-lock.ts L83-L98) lock_skills: no numeric version, a version below 3 (global) or 1 (project), no skills table, a project lock with an entry that is not an object, a boolean version: no entry LOCK_FILE_TABLE, test_lock_skills_holds_only_what_the_pinned_cli_reads
P3-4, P3-5 (command center): the fixtures wrote no numeric version and no source of the manifest; the quotation of src/local-lock.ts belongs to skillPath only; the pinned CLI writes no global lock for a local folder The fixtures write what the CLI writes (global_entry, the fake CLI's lock); the quotation and the local-folder test are corrected none (fixtures and text)
P2-1, P3-1 (command center): the body listed F3 as open and counted four real-CLI tests Fixed in this body (F3 is fixed since commit 5; the class has 8 tests) none

Mutation check of the change: 28 of the lock reading, 11 retained of the installer and 13 of the status check: all killed (survivors: none); nine earlier installer mutants named branches of classify_skill that this commit rewrites, and the 28 replace them.

Fix wave 3 (commit 6), for the third reads at 5ace90c1

Inputs: Astra's delta read (CHANGES_REQUESTED: 1 P1, 1 P2, 4 P3) and the command center's Claude full read (PASS_WITH_P3: no P1, one P2 and five P3). Fail-first: the final tests.test_install_skills and tests.test_skills_status (202 tests) give FAILED (failures=34) in 5 tests on 5ace90c1 (fail-before-5ace90c12-final.txt) and pass here (126 + 76).

Finding Fix Regression test
P1 (Astra blocker 1): a project lock written before the optional skillPath existed (matching source, type and ref), or with a computedHash that is not 64 hex digits, returned install before the content check; the add recreated the folder (src/installer.ts L195, called at L389) and deleted local work without --force (real pinned CLI, the exact pinned find-skills source). The one-step-wider check, as the command center asked ("every lock shape the pinned CLI supports"), found the same early return for a project entry without ref, one that names another source, the entry the CLI itself writes for a local folder (sourceType: local, no ref, no skillPath), and a global entry whose skillFolderHash is missing, empty or not text Only an entry that positively names another revision of the manifest's source (a project ref that is non-empty text and is not the pin's; a global skillFolderHash that is non-empty text and is not the pin's tree) is an upgrade over an older pin and runs as before. Every other entry is read like no lock: the folder must be the pinned tree, a folder that is gone is a clean repair, and ok needs the whole entry (source, type, ref, skillPath, a 64-digit computedHash; the global tree) with the pinned SKILL.md and the link. Cited: src/local-lock.ts L16-L37 (ref? and skillPath? "optional for backward compatibility with lock files written before this field existed", "omitted for non-repo sources"; computedHash is the CLI's own folder hash) and L83-L98, src/skill-lock.ts L89-L101 (an unreadable or too old lock reads as empty) test_a_global_lock_that_names_no_tree_is_not_an_older_pin (5 shapes), test_a_project_lock_that_identifies_no_older_pin_is_compared_like_an_unlocked_folder (13 shapes x 2 agents, each with the extra file refused, the pinned folder alone repaired, a folder that is gone repaired), the two upgrade controls (another ref, another global tree), the whole-entry ok control, and the real pinned CLI: a lock without skillPath, the lock the CLI writes for a local folder, a global lock without a tree (the native add deletes the extra file in each)
P2 (Astra blocker 2): the body declared the pre-rebase base and repeated the previous review's premise that the two live locators were two lines short The base field names 66584736c076873ca7e5898e3bd8111cca1a1447. Dated correction of the previous review (2026-10-12): its finding was a false positive. Lines are counted by LF (editors, grep -n, GitHub); str.splitlines() also breaks at U+2028 and U+2029, one of each in skills_status.py, which shifted its count by two (252 and 560 against the LF 250 and 558). The links to claude_skills_dir and budget_report were correct, and the edits of 5ace90c1 (now 260 and 571) are updates for this change's own line movement none (the body)
F3 (command center): the module docstring, which --help prints, still gave the round-1 scope It gives the scope as it is: the edited SKILL.md, the lock shapes, the entries a git tree cannot hold none (text; the lifecycle paragraph and the decision record, decision 5, say the same)
F4, F5 (command center): the base field; a test description that overstated the test Fixed in the body (the fix of this table's row above; the test is described as what it asserts) none

Mutation check of the change: 15 mutants of the new lock logic (another revision not recognised, an empty ref or an empty global hash counted as another revision, another revision of any source counted, a lock that names no revision sent to install again, an incomplete lock ok, a malformed computedHash or a missing skillPath whole, the guard skipped for an incomplete lock, a global entry without a tree another tree or ok, the global guard skipped, a folder that is gone judged, a lock from another source type or name taken for the manifest's), with 11 retained of the installer and 13 of the status check: all killed (survivors: none). Nine earlier installer mutants named branches of classify_skill that this commit rewrites; the 15 replace them.

Fix wave 2 (commit 5), for the second reads at 9a77a96e

Astra's second read (CHANGES_REQUESTED: 1 P1, 1 P2, 3 P3) and the open review thread of the Codex reviewer (P2). Each is fail-first: the final tests.test_install_skills and tests.test_skills_status (194 tests) fail on 9a77a96e (FAILED (failures=23, errors=6), 9 of the tests, fail-before-9a77a96e8.txt) and pass here.

Finding Fix Regression test
P1 (Astra blocker 1): a lock at the requested pin plus an existing canonical folder is not an upgrade, yet an edited SKILL.md (md_matches false) returned install before the content check, so the add recreated the folder from nothing (src/installer.ts L195, called at L389) and deleted the edit and every extra file without --force; a project with a matching source, ref, path and pinned tree and a missing Claude target took the same early return When the lock names the pin (a project: source, ref, path and the pinned tree; a global entry: skillFolderHash), the folder is compared whatever made the add necessary (a missing target, a missing link, an edited or missing SKILL.md); a folder that is gone is a clean repair; a lock that names another tree stays an upgrade over an older pin (the documented boundary) global and project, through the full installer path: an edited SKILL.md with a local file, with and without the link; a folder that is gone is repaired without --force (both scopes); the older-pin upgrade stays out of scope (control); PinnedCliForeignContentTests with the real pinned CLI in both scopes (the native add deleted both the edit and the file; 2 of 2 fail on 9a77a96e)
P2 (Astra blocker 2): check_folder_tree skipped runtime-artifact names by name at every level, so a regular .venv, scripts/.venv or a .venv link was runtime_artifacts for the status check and local-modified for the installer; the new body claim ("skips only real directories") was false at that head check_folder_tree passes skip_dirs_only=True, as the installer does status integration tests: root and nested regular files and symlinks named like runtime directories give drift, real directories keep runtime_artifacts; test_the_status_check_judges_content_as_the_installer_does (the status check's state for each entry the installer refuses; the installer's side is the ForeignContentTests cases of the same entries)
P2 (Codex thread, git_tree_sha): an entry a git tree cannot hold (an empty directory, a FIFO, a socket, a device) is left out of the hash, so a folder holding one equals the pinned tree while the add deletes it git_tree_sha(strict=True) raises UnrepresentedEntry; the installer asks for it, refuses with a fixed text that names the folder and prints no tree for it; the status check stays a git-tree comparison and its docstring says so an empty directory (at the top, below references, or holding only a regenerated directory) and a named pipe give local-modified with no add, while an empty regenerated .venv stays a clean install; strict mode of git_tree_sha on both kinds; the real pinned CLI deletes an empty directory and a named pipe (the extra-entry test, 2 subtests that fail on 9a77a96e)
P3 (Astra follow-up 3): the two live line locators in the daily-record decision were two lines short they name budget_report (571) and claude_skills_dir (260) at this head none (links)

Mutation check: 20 mutants of the installer change (the check disabled, each branch of classify_skill not asking or asking in the wrong place, a gone folder judged, the older-pin upgrade judged, runtime directories not skipped or skipped by name, a read error or an unrepresented entry counted as a pass, strict mode not asked for, the comparison inverted, the project scope not passed, an ok install judged, the refusal text without the folder or the trees) and 13 of the status change (the canonical-link and parent-alias rules, the copy check, skip_dirs_only and strict not passed down, an empty directory or a special file accepted in strict mode): all killed (survivors: none).

SOTA sources

  • vercel-labs/skills@7407f3893ad4dceab546ac002c3ef806e4000c73, src/installer.ts (the pinned CLI, read from the local mirror): L193-L200 cleanAndCreateDirectory removes the directory recursively and L389-L390 writes the canonical folder anew, so everything the CLI did not write is lost with the old folder; L240-L250 createSymlink returns without touching anything when the link path and the canonical folder are one real path (the parent-link layout); L255-L263 replaces whatever sits at the link path; L458-L466 lists what a copy leaves out (metadata.json, .git, __pycache__, __pypackages__) and makes the directory exclusions depend on the entry being a directory. The real pinned CLI reproduces the loss (below).
  • This repository at the base: scripts/skills_status.py git_tree_sha and RUNTIME_ARTIFACT_DIRS (the folder_tree state): the installer imports this existing predicate instead of a second one. Git's object format for trees and blobs is what git_tree_sha implements; a git tree holds files, links and non-empty directories, which is why strict refuses the rest.
  • Research row 42 (research-queue.tsv, trigger #1024 LANDED, fired 06:55:24Z) and the command center's rulings on Require the Claude Code link in the installer's ok state for a global skill #1024 (04:06Z, 04:41Z).

Evidence-class table

Claim Evidence class Command / receipt
With the real pinned CLI (skills 1.7.0) and manifest row gh-fix-ci, a file added to an unlocked canonical folder is deleted by the installer at main 83cec12ee and survives this change native_proven scratch-home runs of both installers, 2026-10-12: before installed, exit 0, the folder back to its 6 pinned files; after local-modified, exit 1, 7 files; receipts retained locally (real-cli-before.jsonl, real-cli-after.jsonl)
The states of the reads (a regular file or link .venv, a locked project with its Claude link missing, a locked install, global or project, with an edited SKILL.md, an empty directory) are deleted by the native add at the earlier heads and refused here native_proven repro-f3-pushed.jsonl and repro-f3.jsonl (the first read's states at both heads), and tests/test_install_skills.py PinnedCliForeignContentTests (4 tests; run where the pinned CLI is installed, skipped elsewhere; two fail on 295ada5e, three on 9a77a96e)
All 28 managed folders on the host equal the manifest's tree_sha (25 exactly, 3 with __pycache__ left out), so the comparison needs no network and no per-file list native_proven python3 scripts/skills_status.py --json, folder_trees; and a file-by-file blob SHA-1 comparison against the pinned vendor listing, 28 of 28
The installer protects every state in which an add runs over an existing folder (unlocked, locked at the pin with the link missing or SKILL.md edited, a lock that does not name another revision of the manifest's source in the form the CLI writes one, in either scope, Claude-only copy, project, a name that only looks like a runtime directory, an entry a git tree cannot hold), and leaves an upgrade that a lock names as a git tree id or a commit id of the manifest's GitHub source, a missing folder and an ok install alone local_integration + native_proven (the real pinned CLI deletes each extra entry in the controls) tests/test_install_skills.py (136 tests), ForeignContentTests (35) and PinnedCliForeignContentTests (10); the table of 53 lock-entry forms and 39 lock-file forms (and 4 the installer reads more strictly than the CLI); 28 of the lock reading, 11 retained of the installer, 10 of the fifth reads and 4 of the fake CLI's reader, and 13 of the status check
The status check accepts the two parent-link layouts, rejects a link at the canonical skill path for an entry installed for Claude Code (including the reverse alias), skips only real directories of a runtime name, and leaves the Claude-only copy check unchanged local_integration tests/test_skills_status.py (76 tests); 13 mutants, all killed
The change adds no read site to the push gate's monitoring baseline local_integration tests.test_runtime_worker_openhands_push_gate: 127 of 127, run in the module batch (the checker of #1030 reads the committed tree); no dated delta entry is needed
On the host with this change: install_skills.py --check-only reports 28 ok; skills_status.py reports ok native_proven both commands run 2026-10-12 against the real home with the code of the fix wave, exit 0 (host-check-only.json, host-skills-status.json)
A lock with NaN, Infinity or -Infinity, or in a project a null entry or a local entry whose source is not text, is read as empty by the pinned CLI, and the add recreates the folder; the installer now reads such a lock as empty and refuses the add over local work native_proven the pinned reader bodies executed on Node 24.21.0 (reader-measure-20261012.txt: 24 forms, empty or kept as the table says); the real pinned CLI (PinnedCliForeignContentTests, the two new controls); Astra's reproductions in GPT-VERDICT-23e2dcbe9.md
The refusal holds on the complete installer path for every lock-file form with the entry at another revision: refused with no add and the folder untouched where local work sits, installed where nothing is at risk local_integration tests.test_install_skills.ForeignContentTests (the fake CLI, --check-only and an ordinary run, 39 forms + 4, both agents in a project)

Local commands run

(this head, the fix wave of commit 8, on main 2f59d7448; `git merge-tree` clean:)
$ python3 -m unittest tests.test_install_skills tests.test_skills_status
  the final tests on 5f23fc358: Ran 212 tests, FAILED (failures=76, errors=1); at this head: Ran 212 tests, OK (136 + 76)
$ python3 -E -s -B <round-5 mutation scripts> -> 10 of the new reader rules and 4 of the fake CLI's reader: all killed (survivors: none)
$ python3 -m unittest <the 64 modules that name a changed file or that #1030 changed, one process each, three at a time, temporary directory on the repository's filesystem>
  64 modules, 4484 tests, OK (skipped=19)
$ python3 scripts/validate.py  ->  exit 0 (11,801 files); scripts/evidence_manifest.py --check  ->  exit 0; scripts/final_stack.py --check  ->  exit 0
(commit 7, on main d8f7e2051:)
$ python3 -m unittest tests.test_install_skills tests.test_skills_status
  the final tests on 453ac683: Ran 208 tests, FAILED (failures=41, errors=85); at this head: Ran 208 tests, OK (132 + 76)
$ python3 -E -s -B <lock-reading mutation script> -> 28 of the lock reading, 11 retained of the installer and 13 of the status check: all killed (survivors: none); nine earlier installer mutants named branches of `classify_skill` that this commit rewrites, and the 28 replace them.
$ python3 -m unittest <the 64 modules that name a changed file, or that #1030 changed, one process each, three at a time, temporary directory on the repository's filesystem>
  64 modules, 4472 tests, OK (skipped=19); among them tests.test_runtime_worker_openhands_push_gate: Ran 127 tests, OK
$ python3 scripts/validate.py  ->  exit 0 (11,774 files); scripts/evidence_manifest.py --check  ->  exit 0
(commit 6:)
$ python3 -m unittest tests.test_install_skills tests.test_skills_status
  the final tests on 5ace90c1: Ran 202 tests, FAILED (failures=34); at this head: Ran 202 tests, OK (126 + 76)
$ python3 -E -s -B <installer mutation script> -> 11 killed (9 retired: their anchors were rewritten);  <lock mutation script> -> 15 of 15 killed;  <status mutation script> -> 13 of 13 killed
$ python3 -m unittest <the 64 modules that name a changed file or that #1030 changed, one process each, three at a time, temporary directory on the repository's filesystem>
  64 modules, 4466 tests, OK (skipped=19); among them tests.test_runtime_worker_openhands_push_gate: Ran 127 tests, OK, tests.test_skill_correction_claims (5) and tests.test_memory_landscape_capture_doc (16)
$ python3 scripts/validate.py  ->  exit 0 (11,774 files); scripts/evidence_manifest.py --check  ->  exit 0; scripts/final_stack.py --check  ->  exit 0
(commit 5, rebased onto main 66584736c:)
$ python3 -m unittest tests.test_install_skills tests.test_skills_status
  the final tests on 9a77a96e: Ran 194 tests, FAILED (failures=23, errors=6); at this head: Ran 194 tests, OK (118 + 76)
$ python3 -E -s -B <installer mutation script>  ->  20 of 20 mutants killed;  python3 -E -s -B <status mutation script>  ->  13 of 13 mutants killed
$ python3 -m unittest <the 64 modules that name a changed file or that #1030 changed, one process each, three at a time, temporary directory on the repository's filesystem>
  64 modules, 4458 tests, OK (skipped=19); among them tests.test_runtime_worker_openhands_push_gate: Ran 127 tests, OK (a genuine worktree at the committed head), tests.test_skill_correction_claims (5) and tests.test_memory_landscape_capture_doc (16)
$ python3 scripts/validate.py
exit 0 ({"components": 70, "hashed_files": 11774, "profiles": 4, "receipts": 239, "status": "passed"})
$ python3 scripts/evidence_manifest.py --check
exit 0 ({"files": 11774, "status": "passed"})
$ python3 scripts/final_stack.py --check
exit 0
$ python3 tools/adoption/install_skills.py --manifest adoption/skills/manifest.json --skills-bin <skills 1.7.0> --check-only --json
exit 0 ({"ok": true, 28 x "ok"})
$ python3 scripts/skills_status.py --json
exit 0 (result "ok")
(earlier heads:)
$ python3 -m unittest tests.test_install_skills tests.test_skills_status tests.test_skills_manifest tests.test_bootstrap_full_profile tests.test_runtime_worker_openhands tests.test_skill_usage tests.test_runtime_worker_skills tests.test_render_config tests.test_skill_routing tests.test_runtime_worker_openhands_resolver tests.test_runtime_worker_openhands_push_gate tests.test_adoption_docs_consistency
  Ran 955 tests in 206.1s, OK (skipped=9) (commits 1 to 3); commit 4: the 16 modules that read install_skills, skills_status, the lifecycle or the two records, in two batches: Ran 795 tests, OK (skipped=2); Ran 504 tests, OK (skipped=8); tests.test_runtime_worker_openhands_push_gate: Ran 125 tests, OK

Decision record

docs/decisions/2026-10-12-skills-installer-foreign-content.md carries the measurement (the first and the second, from the reads), the decision, the alternatives (a GitHub listing at check time, a per-file list in the manifest, a marker file, refusing every unlocked folder, the narrower canonical-link rule), what is not covered and the conditions that overturn it.

Host evidence

Only relevant when this PR adds or changes files under evidence/hosts/. This PR changes none.

Checklist

  • New/changed GitHub Actions are pinned to a full commit SHA with a version comment (no floating tags). None changed.
  • New/changed workflows declare top-level permissions: {} and grant each job only what it needs. None changed.
  • No secrets are printed, logged or committed; no new required secret was added without a documented owner. No network and no credential is involved; the real-CLI tests run offline in a scratch home with telemetry off.
  • No new paid hosting, subscription or billing surface was introduced.
  • Peer-owned untracked files and worktrees were preserved (not deleted, moved or overwritten).

Follow-ups

  • Not covered, by design: an upgrade over a lock that names another revision of the manifest's GitHub source as a git tree id (global) or a commit id (project) (the older version's files cannot be named offline; the lock proves the CLI wrote some earlier version there) and an ok install (no add runs). A pinned vendor folder that itself contains a file the CLI leaves out of a copy would count as local content; no managed row has one, and the status check reports the same folder as drift.
  • The command center's ruling of 2026-10-12 left the narrower canonical-link rule (refuse only a link that leads to the Claude entry) to this change. It is not taken: a link the installer did not make is content it did not write, the CLI writes a real folder there, and none of the 32 entries of the host's canonical skills folder is a link. The status check enforces the same rule for entries installed for Claude Code.
  • P3 of Astra's third and fourth reads: .claude/skills/claude-native-practice/reference/slots.json L987 names adoption/skills/lifecycle.md:167 for /reload-plugins; the text is at L212 of this head (it moves with the paragraph above it) (the locator was stale before this PR; bind the historical observation to a revision or update it with the slot's provenance label).
  • P3s of the reads, for a later change: (a) post-add verification does not attest installed support files (install_skills.py checks SKILL.md, the recorded tree and the link after an add; a simulated incomplete add that omits support files returns installed while check_folder_tree reports drift): a separate post-add content verification with the same predicate and a recovery test; (b) the machine name in manifests/evidence.json (21 occurrences on 19 lines in files this change touches, none added here) and in files of Require the Claude Code link in the installer's ok state for a global skill #1024 is the separate hygiene item of those reads; (c) the original native receipts are retained locally and named above (real-cli-before.jsonl, real-cli-after.jsonl, repro-f3-pushed.jsonl, repro-f3.jsonl, the fail-before and mutation outputs) for the next packet; the all-28 host comparison is skills_status.py --json and the file-by-file comparison named above.
  • Findings of the command center's Claude full read at the rebased head (pr1035-claude-read-5ace90c1.md, PASS_WITH_P3; no P1), for a later change unless the command center rules otherwise: (F1, P2) a project-scope repair is refused, fail-closed and with nothing deleted, for a folder the pinned CLI itself wrote when the pinned tree holds a symlink or a metadata.json (the copy dereferences symlinks, src/installer.ts L525-L534, and leaves metadata.json out, L458-L466, so the folder never hashes to the pinned tree_sha; the runtime-worker manifest has 62 such rows with symlinks and 3 with metadata.json): compare a project row with the tree the CLI writes, or record a per-row expected tree, with a regression whose pinned tree holds a symlink; the 28 host rows are not affected (the decision's measurement is scoped to them); (F2, P3) a project canonical leaf that is a link is replaced by the add (claude_link_problem is global-only): apply the canonical-link rule in project scope or record the boundary; (F6) an emptied canonical folder is refused with "git tree None" in the text (cosmetic).
  • Findings of the fourth reads that stay follow-ups (P3; none reproduced a deletion): F1 and Astra's P2 (a project repair is refused for a folder whose pinned tree holds a symlink or metadata.json: compare against the tree the CLI writes, or record a per-row expected tree); F2 and Astra's P3 3 (a project canonical leaf that is a link is replaced); F6 and Astra's P3 5 (an emptied folder is refused with git tree None in the text); post-add verification of support files (Astra's P3 4); the hostname hygiene (21 occurrences on 19 lines, none in the patch or the body); the historical captures (Astra's P3 8); Astra's P3 9 (on the merged tree with the live base, convert.py calls scripts/host_name_scan.py, which needs the host identity: 441 landscape subtests and 51 saturation-ledger failures in her restricted runner, none from this PR's paths; the complete merged suite belongs in the intended host environment).
  • Findings of the fifth reads (Astra at 23e2dcbe9) that stay follow-ups: the P2 carried under the scope lock (pristine vendor-copy transformations are still over-refused: compare with the vendor's installed representation or record a per-row expected tree, the F1 item above); P3 5 (a project canonical leaf link is replaced), 6 (post-add verification does not attest support files), 7 (an emptied folder is refused with git tree None in the text), 8 (the locator in slots.json L987), 9 (the host name in unchanged content, 21 occurrences on 19 lines), 10 (the cited captures are not attached; the packet's hash manifest for them is a packaging item); and 11, new here: the native Git-source test and cli() build a child environment without TMPDIR, so a runner whose /tmp is read-only fails two subtests before the assertion (pass TMPDIR through; not a production defect).
  • Three pinned sources are not in the local mirrors (cloudflare/security-audit-skill, grafana/skills, this repository's own claude-native-practice at c59655c7): the no-excluded-members claim is checked for the other 25 rows (exact pinned trees) and by the host comparison for all 28.

🤖 Generated with Claude Code

Follow-ups added at landing (command center, 2026-10-12)

  • Codex P2 (thread on 8ffcf9a, install_skills.py:375): an exact copy of a pinned skill that tracks a real runtime-named directory (.venv, node_modules, pycache and the rest) is classified local-modified, so it needs --force. That is an over-refusal: nothing is deleted. The follow-up accepts a strict full-tree match first, then retries with the regenerable directories excluded (as check_folder_tree() does), with a fail-before test. It is the same class as the carried P2 above.
  • Codex P2 (thread on 5f23fc3, NaN lock versions): fixed in 8ffcf9a (regressions GL11 and GL12); thread answered and resolved.

@seathatflowsinourveins seathatflowsinourveins added the lane:foundation Foundation lane: Claude/Codex setup, hosts, memory, RAG, research, workers label Oct 12, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T18:38:54.839975Z 8ffcf9a Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 12, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 295ada5e8b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/adoption/install_skills.py Resolved
Comment thread scripts/skills_status.py Resolved
Comment thread tools/adoption/install_skills.py Resolved
Comment thread tools/adoption/install_skills.py Resolved
seathatflowsinourveins added a commit that referenced this pull request Oct 12, 2026
@seathatflowsinourveins @claude
Refuse a regular file named like a runtime directory, and a repairing…
9a77a96 
… add over local work in a locked project; keep the push gate's read inventory unchanged

The first reads of #1035 (Astra at 295ada5 and the four open Codex review threads) found two ways the guard still lost local work and one regression. One commit:

- A regular file or a link named `.venv`, `node_modules`, `__pycache__` and the like was taken for a regenerable directory (the name was excluded before the entry's type was looked at),
  and the add deleted it. git_tree_sha now skips such a name only for a real directory and does not follow a link. The SKILL.md-alone shortcut, which had the same flaw and also followed a
  SKILL.md link, is removed: a lone SKILL.md is hashed like any other folder.
- A project locked at the pin whose Claude target is missing returned `install` before the content check, so the repairing add deleted local files. The check now runs before that repair in
  a project as it does in the global scope.
- The shortcut's os.listdir was a sixth unclassified read site of the push gate's monitoring baseline (342 -> 343; tests.test_runtime_worker_openhands_push_gate failed 1 of 125 and
  repeated). With the shortcut gone the installer has its five reads again: 125 of 125 pass at this head.
- The status check rejected a canonical-leaf link for a Codex-only entry that the installer accepts; both now apply the rule to entries installed for Claude Code only. The moved link to
  budget_report in the 2026-10-10 record points at its current line.

tests/test_install_skills.py::PinnedCliForeignContentTests repeats the two measurements offline with the pinned skills 1.7.0 CLI where it is installed (a local source folder, a scratch
home and project, telemetry off): the classifier must say local-modified before any add, and the same add on a copy must delete the entry (the control). Where the CLI is not installed it
is skipped; the fake-CLI tests cover the same states. The record and the lifecycle are updated; the evidence rows are re-registered at their final bytes.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins

seathatflowsinourveins commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 12, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a77a96e80

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/adoption/install_skills.py Resolved
seathatflowsinourveins added a commit that referenced this pull request Oct 12, 2026
@seathatflowsinourveins @claude
Refuse a regular file named like a runtime directory, and a repairing…
2c289e5 
… add over local work in a locked project; keep the push gate's read inventory unchanged

The first reads of #1035 (Astra at 295ada5 and the four open Codex review threads) found two ways the guard still lost local work and one regression. One commit:

- A regular file or a link named `.venv`, `node_modules`, `__pycache__` and the like was taken for a regenerable directory (the name was excluded before the entry's type was looked at),
  and the add deleted it. git_tree_sha now skips such a name only for a real directory and does not follow a link. The SKILL.md-alone shortcut, which had the same flaw and also followed a
  SKILL.md link, is removed: a lone SKILL.md is hashed like any other folder.
- A project locked at the pin whose Claude target is missing returned `install` before the content check, so the repairing add deleted local files. The check now runs before that repair in
  a project as it does in the global scope.
- The shortcut's os.listdir was a sixth unclassified read site of the push gate's monitoring baseline (342 -> 343; tests.test_runtime_worker_openhands_push_gate failed 1 of 125 and
  repeated). With the shortcut gone the installer has its five reads again: 125 of 125 pass at this head.
- The status check rejected a canonical-leaf link for a Codex-only entry that the installer accepts; both now apply the rule to entries installed for Claude Code only. The moved link to
  budget_report in the 2026-10-10 record points at its current line.

tests/test_install_skills.py::PinnedCliForeignContentTests repeats the two measurements offline with the pinned skills 1.7.0 CLI where it is installed (a local source folder, a scratch
home and project, telemetry off): the classifier must say local-modified before any add, and the same add on a copy must delete the entry (the control). Where the CLI is not installed it
is skipped; the fake-CLI tests cover the same states. The record and the lifecycle are updated; the evidence rows are re-registered at their final bytes.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
seathatflowsinourveins added a commit that referenced this pull request Oct 12, 2026
@seathatflowsinourveins @claude
Compare the whole folder before any add over a locked skill, fail clo…
5ace90c 
…sed on entries a git tree cannot hold, and judge runtime-named files the same way in the status check

Fix wave 2 of #1035, on 9a77a96: the second GPT read (blockers 1 and 2, follow-up 3) and the open Codex thread (P2, "Fail closed on entries omitted from the tree hash").

- A lock at the requested pin plus an existing canonical folder is not an upgrade, and the add recreates the folder from nothing (vercel-labs/skills@7407f389 src/installer.ts L195, called at L389). classify_skill
  returned "install" when SKILL.md was not the pinned one, before the content check ran, so an edited SKILL.md plus a local file were deleted without --force. Now, when the lock names the pin (a project:
  source, ref, path and the pinned tree; a global entry: skillFolderHash), the folder is compared whatever made the add necessary (a missing Claude target, a missing link, an edited or missing SKILL.md); a
  folder that is gone is a clean repair; a lock that names another tree stays an upgrade over an older pin (the documented boundary).
- git_tree_sha(strict=True) raises UnrepresentedEntry for an entry a git tree cannot hold (an empty directory, a FIFO, a socket, a device), which the add deletes all the same; the installer refuses it with a
  fixed text and never prints a tree for it. The status check stays a git-tree comparison.
- check_folder_tree skipped runtime-artifact names by name at every level; it now skips only real directories (skip_dirs_only), as the installer does, so a regular file or a link named .venv is drift.
- The two live line locators in the daily-record decision now point at budget_report (571) and claude_skills_dir (260).

Fail-first: the final tests of tests.test_install_skills and tests.test_skills_status (194) give 23 failures and 6 errors against the implementation of 9a77a96 (9 tests: global and project edited-SKILL.md
through the full installer path, the real pinned CLI in both, unrepresented entries, strict mode, the status check against the installer) and pass here; 20 of 20 installer mutants and 13 of 13 status mutants killed.
The evidence rows of the changed files are updated.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins
seathatflowsinourveins force-pushed the claude/cc-native-practice-f3-foreign-content-20261012 branch from 9a77a96 to 5ace90c Compare October 12, 2026 11:15
@seathatflowsinourveins

seathatflowsinourveins commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

seathatflowsinourveins added a commit that referenced this pull request Oct 12, 2026
@seathatflowsinourveins @claude
Read a lock as naming an older pin only when it names another revisio…
453ac68 
…n, and compare the folder in every other lock shape

Fix wave 3 of #1035, on 5ace90c: Astra's delta read (~/.cache/ns2604-gpt-reads/fix2-pr1035-5ace90c12/VERDICT.md: one P1, one P2) and the command center's Claude full read (pr1035-claude-read-5ace90c1.md: PASS_WITH_P3, no P1; F3 here, F4 and F5 are claims of the PR body).

- P1 (Astra): classify_skill returned "install" before the content check when a project lock entry lacked the optional `skillPath` (a lock written before the field existed), or its `computedHash` was not
  64 hex digits, so the add recreated the folder from nothing (vercel-labs/skills@7407f389 src/installer.ts L195, called at L389) and deleted local work without --force. The pinned CLI makes these
  fields optional or does not need them whole: src/local-lock.ts L16-L37 (`ref?` and `skillPath?` are "optional for backward compatibility with lock files written before this field existed" and
  "omitted for non-repo sources"; `computedHash` is the CLI's own hash of the folder, not a revision), and readLocalLock and readSkillLock return an empty lock for a file that is too old or does not
  parse (src/local-lock.ts L83-L98; src/skill-lock.ts L89-L101). An entry that lacks them says nothing about which revision wrote the folder. The same early return took a project entry without `ref`, one that names
  another source and the entry the CLI writes for a local folder (`sourceType: local`, no `ref`, no `skillPath`; measured with the real CLI), and a global entry whose `skillFolderHash` is missing, empty
  or not text.
  The rule: only an entry that positively names another revision of the manifest's source (a project `ref` that is non-empty text and is not the pin's; a global `skillFolderHash` that is non-empty text
  and is not the pin's tree) is an upgrade over an older pin and runs as before. Every other entry is read like no lock: the folder must be the pinned tree, a folder that is gone is a clean repair, and
  `ok` needs the whole entry (source, type, `ref`, `skillPath`, a 64-digit `computedHash`; the global tree) with the pinned SKILL.md and the link.
- F3 (command center): the module docstring (printed by --help) now gives the scope of the rule as it is: the edited SKILL.md, the lock shapes, the entries a git tree cannot hold.
- P2 (Astra), the earlier review's locator premise: the previous read found the two live line locators of the daily-record decision "two lines short" (250 and 558 against 252 and 560). That was a false
  positive: lines are counted by LF (editors, grep -n, GitHub), and str.splitlines() also breaks at U+2028 and U+2029, one of each in skills_status.py, which shifted its count by two. The links of
  5ace90c (260 and 571) are updates for this change's own line movement and stay.

Fail-first: the tests of tests.test_install_skills and tests.test_skills_status (202) give FAILED (failures=34) in 5 tests on 5ace90c (the global lock shapes, the project lock shapes for both agents,
and the real pinned CLI for a lock without skillPath, for the lock the CLI writes for a local folder and for a global lock without a tree) and pass here. Mutants: 15 of the new lock logic, 11 retained of the
installer and 13 of the status check, all killed; nine earlier mutants of branches that this commit rewrites are replaced by the 15. Not taken, listed in the PR body: F1 (a project repair is refused,
fail-closed, for a folder whose pinned tree holds a symlink or metadata.json, which the CLI's copy dereferences or leaves out), F2, F6.
The evidence rows of the changed files are updated.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
seathatflowsinourveins added a commit that referenced this pull request Oct 12, 2026
@seathatflowsinourveins @claude
Read a lock from the upstream definition of what the pinned CLI write…
23e2dcb 
…s: an entry names an older pin only as a git tree id or a commit id of the manifest's source

Fix wave 4 of #1035, on 453ac68, by the command center's adjudication (adjudication-pr1035-453ac683-20261012.md: the lock-shape class recurred, so the fix starts from the upstream definition, enumerates every
lock-entry form the pinned CLI writes or leaves, project and global, builds the test table from it and fails closed for anything outside it). Inputs: Astra's delta read (~/.cache/ns2604-gpt-reads/fix3-pr1035-453ac6834/VERDICT.md:
one P1, one P2, seven P3) and the command center's Claude delta (pr1035-claude-micro-453ac683.md: B1 at P1, one P2, five P3).

- P1, both files: classify_skill returned "install" before the content check for a global entry whose `skillFolderHash` is any non-empty text that is not the pin's tree (Astra: "not-a-tree", 39 hex digits, one
  space), for the CLI's own sha256 digest of the folder (the CC: what src/add.ts L2108-L2111 writes where the GitHub tree lookup returns nothing, and for every gitlab, git and local source), and for an entry
  of another source or type (src/add.ts L2114-L2122 records `source` and `sourceType` for every global install); the add then recreated the folder (src/installer.ts L195, called at L389) and deleted local
  work without --force. Reproduced with the real pinned CLI for a generic git source, global and project (the entry it writes holds the digest, a URL and no `ref`).
- The reading is now one function, lock_reading, built from the table: an entry names another revision only through a field the CLI writes as one, in that form (a global `skillFolderHash` that is a git tree id,
  40 hex digits matched case-insensitively as src/update.ts L638 does; a project `ref` that is a full commit id), and only for the manifest's GitHub source; a tree id equal to the pin's is a content address and
  reads "same" whatever source wrote it. Everything else is "unidentified" and the folder is compared like an unlocked one: a digest, a URL source, a branch, a tag or an abbreviated id as `ref` (src/local-lock.ts
  L19-L20: what the user typed), an empty, missing or malformed field. A lock file the CLI reads as empty (no numeric `version`, a version below 3 global or 1 project, no `skills` table; for a project any
  entry that is not an object: src/skill-lock.ts L89-L101, src/local-lock.ts L83-L98) holds no entry (lock_skills). `ok` needs the whole entry and the pinned SKILL.md and link.
- The table: 53 entry forms (26 global, 27 project) and 20 lock-file forms, each with its upstream citation, in the "Lock entries" section of the decision record and in LOCK_ENTRY_TABLE and LOCK_FILE_TABLE of
  tests.test_install_skills, run for the global scope and for a project under both agents in three folder states (pristine, a local file beside the pinned ones, folder gone), through the classifier and through the
  full installer path with the fake CLI, plus the real pinned CLI for the generic git source.
- The command center's P2 (body line 107, F3 listed as open), P3-1 (the test count), P3-2 (a ref that is a branch or a tag is compared: it can name the pin's own commit), P3-3 (the lock version rule),
  P3-4 (the fixtures write the numeric `version` and the source the CLI writes), P3-5 (the quotation belongs to `skillPath`; the pinned CLI writes no global lock for a local folder): fixed here or in the body.
  F1 (a project repair is refused for a folder whose pinned tree holds a symlink or metadata.json), F2, F6 and the P3s of both reads stay in the PR body's Follow-ups.

Fail-first: tests.test_install_skills and tests.test_skills_status (208 tests) give FAILED (failures=41, errors=85) in 5 tests on 453ac68 (the lock-entry table, the lock-file table, lock_skills, the installer
path and the real-CLI generic git source) and pass here. Mutants: 28 of the lock reading (the version rule and its minimums, the boolean and text versions, the null-entry rule, what counts as an id, its case and length, the pin comparison, the source and its type, the project and global field, every branch that sends an entry that identifies nothing to an install or to ok, the whole-entry test, the tree oracle, a gone folder), 11 retained of the installer and 13 of the status check: all killed (survivors: none); nine earlier installer mutants named branches of classify_skill that this commit rewrites, and the 28 replace them.
The evidence rows of the changed files are updated.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins

seathatflowsinourveins commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

seathatflowsinourveins and others added 7 commits October 12, 2026 13:45
@seathatflowsinourveins @claude
Do not let the skills add delete content the pinned CLI did not write…
be47e5c 
… (F3 of #1024)

Where an add is about to run over an existing folder whose SKILL.md is the pinned one (an unlocked folder, or one locked at the pin whose Claude link is missing), the folder must hold SKILL.md alone or be the pinned tree: the git tree SHA that scripts/skills_status.py already computes, with the regenerable runtime-artifact directories left out, equals the manifest's tree_sha. Anything else is local-modified, refused unless --force, for the global canonical folder, a Claude-Code-only copy and a project folder. Measured with the real pinned CLI: a file added to an unlocked gh-fix-ci folder was deleted at main 83cec12 and survives now. The canonical-link rule stays literal; an upgrade over an older lock and an ok install are not covered. Also the --force help text (P3-2), the Codex-only scope of the link rule in the lifecycle record (P3-3) and a missing space in a test (P3-5).

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins @claude
Make the status check agree with the installer on canonical links and…
bbfa8ac 
… parent aliases

install_skills.py refuses a link at the canonical skill path and accepts a link on a parent folder (createSymlink L240-250); scripts/skills_status.py followed the first and failed the second. It now reports canonical_link for a link at the skill path (the reverse alias of #1024 included) and accepts a real folder that a parent link makes the canonical folder itself (kind parent_link), for a global entry only: the Claude-Code-only copy check is unchanged.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins @claude
Register the F3 files in the evidence manifest
c6565ef 
Last commit of the change: the rows of every file it adds or changes, at their final bytes.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins @claude
Refuse a regular file named like a runtime directory, and a repairing…
cabd3c8 
… add over local work in a locked project; keep the push gate's read inventory unchanged

The first reads of #1035 (Astra at 295ada5 and the four open Codex review threads) found two ways the guard still lost local work and one regression. One commit:

- A regular file or a link named `.venv`, `node_modules`, `__pycache__` and the like was taken for a regenerable directory (the name was excluded before the entry's type was looked at),
  and the add deleted it. git_tree_sha now skips such a name only for a real directory and does not follow a link. The SKILL.md-alone shortcut, which had the same flaw and also followed a
  SKILL.md link, is removed: a lone SKILL.md is hashed like any other folder.
- A project locked at the pin whose Claude target is missing returned `install` before the content check, so the repairing add deleted local files. The check now runs before that repair in
  a project as it does in the global scope.
- The shortcut's os.listdir was a sixth unclassified read site of the push gate's monitoring baseline (342 -> 343; tests.test_runtime_worker_openhands_push_gate failed 1 of 125 and
  repeated). With the shortcut gone the installer has its five reads again: 125 of 125 pass at this head.
- The status check rejected a canonical-leaf link for a Codex-only entry that the installer accepts; both now apply the rule to entries installed for Claude Code only. The moved link to
  budget_report in the 2026-10-10 record points at its current line.

tests/test_install_skills.py::PinnedCliForeignContentTests repeats the two measurements offline with the pinned skills 1.7.0 CLI where it is installed (a local source folder, a scratch
home and project, telemetry off): the classifier must say local-modified before any add, and the same add on a copy must delete the entry (the control). Where the CLI is not installed it
is skipped; the fake-CLI tests cover the same states. The record and the lifecycle are updated; the evidence rows are re-registered at their final bytes.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins @claude
Compare the whole folder before any add over a locked skill, fail clo…
0918481 
…sed on entries a git tree cannot hold, and judge runtime-named files the same way in the status check

Fix wave 2 of #1035, on 9a77a96: the second GPT read (blockers 1 and 2, follow-up 3) and the open Codex thread (P2, "Fail closed on entries omitted from the tree hash").

- A lock at the requested pin plus an existing canonical folder is not an upgrade, and the add recreates the folder from nothing (vercel-labs/skills@7407f389 src/installer.ts L195, called at L389). classify_skill
  returned "install" when SKILL.md was not the pinned one, before the content check ran, so an edited SKILL.md plus a local file were deleted without --force. Now, when the lock names the pin (a project:
  source, ref, path and the pinned tree; a global entry: skillFolderHash), the folder is compared whatever made the add necessary (a missing Claude target, a missing link, an edited or missing SKILL.md); a
  folder that is gone is a clean repair; a lock that names another tree stays an upgrade over an older pin (the documented boundary).
- git_tree_sha(strict=True) raises UnrepresentedEntry for an entry a git tree cannot hold (an empty directory, a FIFO, a socket, a device), which the add deletes all the same; the installer refuses it with a
  fixed text and never prints a tree for it. The status check stays a git-tree comparison.
- check_folder_tree skipped runtime-artifact names by name at every level; it now skips only real directories (skip_dirs_only), as the installer does, so a regular file or a link named .venv is drift.
- The two live line locators in the daily-record decision now point at budget_report (571) and claude_skills_dir (260).

Fail-first: the final tests of tests.test_install_skills and tests.test_skills_status (194) give 23 failures and 6 errors against the implementation of 9a77a96 (9 tests: global and project edited-SKILL.md
through the full installer path, the real pinned CLI in both, unrepresented entries, strict mode, the status check against the installer) and pass here; 20 of 20 installer mutants and 13 of 13 status mutants killed.
The evidence rows of the changed files are updated.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins @claude
Read a lock as naming an older pin only when it names another revisio…
59f476c 
…n, and compare the folder in every other lock shape

Fix wave 3 of #1035, on 5ace90c: Astra's delta read (~/.cache/ns2604-gpt-reads/fix2-pr1035-5ace90c12/VERDICT.md: one P1, one P2) and the command center's Claude full read (pr1035-claude-read-5ace90c1.md: PASS_WITH_P3, no P1; F3 here, F4 and F5 are claims of the PR body).

- P1 (Astra): classify_skill returned "install" before the content check when a project lock entry lacked the optional `skillPath` (a lock written before the field existed), or its `computedHash` was not
  64 hex digits, so the add recreated the folder from nothing (vercel-labs/skills@7407f389 src/installer.ts L195, called at L389) and deleted local work without --force. The pinned CLI makes these
  fields optional or does not need them whole: src/local-lock.ts L16-L37 (`ref?` and `skillPath?` are "optional for backward compatibility with lock files written before this field existed" and
  "omitted for non-repo sources"; `computedHash` is the CLI's own hash of the folder, not a revision), and readLocalLock and readSkillLock return an empty lock for a file that is too old or does not
  parse (src/local-lock.ts L83-L98; src/skill-lock.ts L89-L101). An entry that lacks them says nothing about which revision wrote the folder. The same early return took a project entry without `ref`, one that names
  another source and the entry the CLI writes for a local folder (`sourceType: local`, no `ref`, no `skillPath`; measured with the real CLI), and a global entry whose `skillFolderHash` is missing, empty
  or not text.
  The rule: only an entry that positively names another revision of the manifest's source (a project `ref` that is non-empty text and is not the pin's; a global `skillFolderHash` that is non-empty text
  and is not the pin's tree) is an upgrade over an older pin and runs as before. Every other entry is read like no lock: the folder must be the pinned tree, a folder that is gone is a clean repair, and
  `ok` needs the whole entry (source, type, `ref`, `skillPath`, a 64-digit `computedHash`; the global tree) with the pinned SKILL.md and the link.
- F3 (command center): the module docstring (printed by --help) now gives the scope of the rule as it is: the edited SKILL.md, the lock shapes, the entries a git tree cannot hold.
- P2 (Astra), the earlier review's locator premise: the previous read found the two live line locators of the daily-record decision "two lines short" (250 and 558 against 252 and 560). That was a false
  positive: lines are counted by LF (editors, grep -n, GitHub), and str.splitlines() also breaks at U+2028 and U+2029, one of each in skills_status.py, which shifted its count by two. The links of
  5ace90c (260 and 571) are updates for this change's own line movement and stay.

Fail-first: the tests of tests.test_install_skills and tests.test_skills_status (202) give FAILED (failures=34) in 5 tests on 5ace90c (the global lock shapes, the project lock shapes for both agents,
and the real pinned CLI for a lock without skillPath, for the lock the CLI writes for a local folder and for a global lock without a tree) and pass here. Mutants: 15 of the new lock logic, 11 retained of the
installer and 13 of the status check, all killed; nine earlier mutants of branches that this commit rewrites are replaced by the 15. Not taken, listed in the PR body: F1 (a project repair is refused,
fail-closed, for a folder whose pinned tree holds a symlink or metadata.json, which the CLI's copy dereferences or leaves out), F2, F6.
The evidence rows of the changed files are updated.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins @claude
Read a lock from the upstream definition of what the pinned CLI write…
5f23fc3 
…s: an entry names an older pin only as a git tree id or a commit id of the manifest's source

Fix wave 4 of #1035, on 453ac68, by the command center's adjudication (adjudication-pr1035-453ac683-20261012.md: the lock-shape class recurred, so the fix starts from the upstream definition, enumerates every
lock-entry form the pinned CLI writes or leaves, project and global, builds the test table from it and fails closed for anything outside it). Inputs: Astra's delta read (~/.cache/ns2604-gpt-reads/fix3-pr1035-453ac6834/VERDICT.md:
one P1, one P2, seven P3) and the command center's Claude delta (pr1035-claude-micro-453ac683.md: B1 at P1, one P2, five P3).

- P1, both files: classify_skill returned "install" before the content check for a global entry whose `skillFolderHash` is any non-empty text that is not the pin's tree (Astra: "not-a-tree", 39 hex digits, one
  space), for the CLI's own sha256 digest of the folder (the CC: what src/add.ts L2108-L2111 writes where the GitHub tree lookup returns nothing, and for every gitlab, git and local source), and for an entry
  of another source or type (src/add.ts L2114-L2122 records `source` and `sourceType` for every global install); the add then recreated the folder (src/installer.ts L195, called at L389) and deleted local
  work without --force. Reproduced with the real pinned CLI for a generic git source, global and project (the entry it writes holds the digest, a URL and no `ref`).
- The reading is now one function, lock_reading, built from the table: an entry names another revision only through a field the CLI writes as one, in that form (a global `skillFolderHash` that is a git tree id,
  40 hex digits matched case-insensitively as src/update.ts L638 does; a project `ref` that is a full commit id), and only for the manifest's GitHub source; a tree id equal to the pin's is a content address and
  reads "same" whatever source wrote it. Everything else is "unidentified" and the folder is compared like an unlocked one: a digest, a URL source, a branch, a tag or an abbreviated id as `ref` (src/local-lock.ts
  L19-L20: what the user typed), an empty, missing or malformed field. A lock file the CLI reads as empty (no numeric `version`, a version below 3 global or 1 project, no `skills` table; for a project any
  entry that is not an object: src/skill-lock.ts L89-L101, src/local-lock.ts L83-L98) holds no entry (lock_skills). `ok` needs the whole entry and the pinned SKILL.md and link.
- The table: 53 entry forms (26 global, 27 project) and 20 lock-file forms, each with its upstream citation, in the "Lock entries" section of the decision record and in LOCK_ENTRY_TABLE and LOCK_FILE_TABLE of
  tests.test_install_skills, run for the global scope and for a project under both agents in three folder states (pristine, a local file beside the pinned ones, folder gone), through the classifier and through the
  full installer path with the fake CLI, plus the real pinned CLI for the generic git source.
- The command center's P2 (body line 107, F3 listed as open), P3-1 (the test count), P3-2 (a ref that is a branch or a tag is compared: it can name the pin's own commit), P3-3 (the lock version rule),
  P3-4 (the fixtures write the numeric `version` and the source the CLI writes), P3-5 (the quotation belongs to `skillPath`; the pinned CLI writes no global lock for a local folder): fixed here or in the body.
  F1 (a project repair is refused for a folder whose pinned tree holds a symlink or metadata.json), F2, F6 and the P3s of both reads stay in the PR body's Follow-ups.

Fail-first: tests.test_install_skills and tests.test_skills_status (208 tests) give FAILED (failures=41, errors=85) in 5 tests on 453ac68 (the lock-entry table, the lock-file table, lock_skills, the installer
path and the real-CLI generic git source) and pass here. Mutants: 28 of the lock reading (the version rule and its minimums, the boolean and text versions, the null-entry rule, what counts as an id, its case and length, the pin comparison, the source and its type, the project and global field, every branch that sends an entry that identifies nothing to an install or to ok, the whole-entry test, the tree oracle, a gone folder), 11 retained of the installer and 13 of the status check: all killed (survivors: none); nine earlier installer mutants named branches of classify_skill that this commit rewrites, and the 28 replace them.
The evidence rows of the changed files are updated.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins
seathatflowsinourveins force-pushed the claude/cc-native-practice-f3-foreign-content-20261012 branch from 23e2dcb to 5f23fc3 Compare October 12, 2026 17:57
@seathatflowsinourveins

seathatflowsinourveins commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 12, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f23fc358a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/adoption/install_skills.py Resolved
@seathatflowsinourveins @claude
Read a lock as the pinned reader reads it in every rejection it has: …
8ffcf9a 
…JSON constants and a local source that is not text

Fix wave 5 of #1035 on 5f23fc3 (23e2dcb rebased onto main 2f59d74), released by the command center. Inputs: Astra's delta read of 23e2dcb (GPT-VERDICT-23e2dcbe9.md, 30,112 bytes, sha256 2ec782f8: three P1,
one P2 carried under the scope lock, seven P3) and the command center's tests-only Claude check, which passed.

- P1, global and project: load_lock took the tokens NaN, Infinity and -Infinity, which json.loads accepts and JSON.parse rejects (skills@7407f389 src/skill-lock.ts L85, src/local-lock.ts L80); both pinned readers
  return an empty lock when it throws (L99-L101, L97-L98), so the entry in such a lock excused nothing, but the installer read it as an older pin and let the add recreate the folder over local work. load_lock now
  passes parse_constant=reject_constant (CPython's hook for invalid JSON numbers, Doc/library/json.rst L324-L329 at python/[email protected]): a lock with a constant anywhere is unreadable and the folder is
  compared like an unlocked one. 1e999 is a valid number (both parse it as Infinity) and stays readable.
- P1, project: lock_skills read every entry that is an object, but the pinned reader's loop throws, and its catch returns an empty lock, for a null entry and for a local entry whose source is missing or is not
  text (src/local-lock.ts L90-L98: isAbsolute(entry.source)); another entry of that kind excused the deletion for the managed entry at another commit. lock_skills now reads such a project lock as empty, for the
  managed entry or any other. The source description is corrected: the reader does not throw on an entry that is a string, a number, true or a list (measured by executing the pinned reader bodies on Node
  24.21.0, reader-measure-20261012.txt); this installer still reads such a lock as empty, which is stricter than the reader and is now documented as such.
- P1, privacy: the Git fixture of the native generic-source test gave author and committer an email-shaped literal; both are now the placeholder no-address, which is not an email (git takes it).
- The fake CLI of the tests read a lock more leniently than the pinned readers, so a damaged lock survived its own add; its load_lock now mirrors them (constants, version, falsy skills, the project null and
  local-source rules), which lets the clean-install controls run on the complete installer path.

Tests: LOCK_FILE_TABLE gained 19 forms (GL11-GL16, PL10-PL22; 39 in all) and LOCK_FILE_STRICTER_TABLE holds the four forms the reader keeps and this installer does not; every form runs through the classifier and
through the whole installer path (refused with no add and the folder untouched where local work sits; installed where nothing is at risk), plus a unit test of load_lock on text, cases of lock_skills, and two
controls with the real pinned CLI (the native add deletes the file the classifier protects).

Fail-first: tests.test_install_skills and tests.test_skills_status (212 tests) give FAILED (failures=76, errors=1) in 6 tests on 5f23fc3 and pass here (136 + 76). 10 mutants of the new reader rules and 4 of the
fake CLI's reader killed. Astra's P2 and the P3s go to the PR body's Follow-ups. The evidence rows of the changed files are updated.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@seathatflowsinourveins

seathatflowsinourveins commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 12, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8ffcf9af52

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/adoption/install_skills.py Resolved
@seathatflowsinourveins
seathatflowsinourveins merged commit 0333734 into main Oct 12, 2026
37 checks passed
@seathatflowsinourveins
seathatflowsinourveins deleted the claude/cc-native-practice-f3-foreign-content-20261012 branch October 12, 2026 19:25
@seathatflowsinourveins seathatflowsinourveins mentioned this pull request Oct 17, 2026
Merged
5 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lane:foundation Foundation lane: Claude/Codex setup, hosts, memory, RAG, research, workers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@seathatflowsinourveins