fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely - #1321

Merge Risk: Low · up to e2591

With many malformed skills, the diagnostics warning can crowd out the skills list in settings. Bound the list before merging. Two smaller persistence-related follow-ups remain: a missing test for a failed permission read, and cross-process sidecar cleanup that needs a maintainer decision.

Failed checks: 1 error · 2 warnings

Check name	Status	Explanation	Resolution
Out of Scope Changes check		The change set includes work that #859 does not require. In SkillsManager.ts, the following are not needed for clear YAML errors or YAML-safe serialization: best-effort stale-sidecar sweeps and retr…	Move these into separate changes: stale-sidecar sweeps and retries, initialization caching, cross-instance directory locks, disposal and queued-work rejection, permission-bit preservation, symlink and realpath validation, and the `outside_s…
Regression Evidence		The changed behavior is the publish-time fail-closed check in writeSkillFileAtomically (src/services/skills/SkillsManager.ts, lines 805-816). It calls fs.stat(filePath) on the target SKILL.md.…	Add unit tests in src/services/skills/__tests__/SkillsManager.spec.ts for the publish-time target stat: 1. Run discoverSkills() so the skill directory and SKILL.md discovery succeed. 2. Make mockStat reject for the target `SKILL.m…
Lifecycle Resource Cleanup		The PR adds an atomic SKILL.md writer and a startup sweep for its leftover sidecars. The sweep does not cover every sidecar that the writer can create. This is a changed lifecycle path that can leak a…	Make the restart sweep cover every sidecar the atomic writer can create: 1. Match sidecars by the target basename, not only SKILL.md. For example, match ^\\.(.+)\\.\\d+\\.[a-z0-9]+\\.tmp$ and confirm the captured name is the basename of a fi…

Passed checks: 5 passed

Check name	Status	Explanation
Title check		The title "fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely" matches the main changes in the changeset. The changes add frontmatter parse diagnostics with line and colum…
Description check		The description follows the template structure. It has a linked issue (Fixes #859), a Description section, a Test Procedure described through the Verification section, a Pre-Submission Checklist, and …
Linked Issues check		[#859] The only active directly linked issue is #859. Its coding requirement is that malformed SKILL.md frontmatter (unescaped double quotes in description) must not fail silently or show the mislea…
Security Boundaries		The check fails only for a changed path that (a) leaks secrets or PII, (b) trusts or executes unvalidated input, or (c) bypasses approval or allowlist controls. I found no changed path that meets any …
Persistence Integrity		The changed persistence paths in src/services/skills/SkillsManager.ts at the reviewed head (refs/coderabbit/pre-merge-check/head) do not show a concrete loss or corruption path under the Persisten…

Full details: Title check

Explanation

The title "fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely" matches the main changes in the changeset. The changes add frontmatter parse diagnostics with line and column locations, a targeted unescaped-quote hint, and lineWidth-safe serialization in createSkill and updateSkillModes. The title omits the larger atomic-write and sidecar-sweep work, which the title does not need to cover. The title is 79 characters, which is within a reasonable length for a commit subject.

Full details: Description check

Explanation

The description follows the template structure. It has a linked issue (Fixes #859), a Description section, a Test Procedure described through the Verification section, a Pre-Submission Checklist, and a Merge order. It is largely complete. Two gaps remain. The Description section describes the changes, but the Test Procedure lacks manual steps for reviewers. The description also contains a 'Review ask' section about code-owner approvals and branch protection rules. That section does not fit the template and does not describe the code change. The description also says it is stacked on #934, which is a dependency that reviewers must track. The description states that the first-listed change set is stacked on #934, and the stated scope does not match the broader sidecar and atomic-write work named in the objectives.

Resolution

Consider these updates before merge: (1) Remove or move the 'Review ask' section, because it covers branch protection and ownership rules and not the code change. (2) Add manual test steps in the Test Procedure section. (3) Update the Description section to cover the atomic-write, sidecar-sweep, and directory-lock changes shown in the raw summary, because the current description does not mention them. (4) Check the Pre-Submission Checklist boxes that apply, and state whether documentation updates are needed.

Full details: Linked Issues check

Explanation

[#859] The only active directly linked issue is #859. Its coding requirement is that malformed SKILL.md frontmatter (unescaped double quotes in description) must not fail silently or show the misleading "missing required 'name' field" message. The expected outcome is a clear YAML parse error that identifies the problem. The change set covers this. loadSkillMetadata() parses with explicit options (matter(fileContent, {})), which bypasses gray-matter's content cache, so repeat scans report the same parse error. A parse failure is recorded as a SkillDiagnostic with line and column data. The "unescaped double quotes" hint is logged only when the parser error mark is on the raw description: line. The settings UI lists the diagnostics, and the webview state carries them. Tests cover the exact #859 content, the no-false-hint case, and rescan behavior. The reviewed summary does not show a full test run, so these test claims rest on the summary and not on a verified run. The linked-issue requirement is met by the diagnostic path. Non-coding items in the PR description (review ownership and approval state) are outside this assessment.

Full details: Security Boundaries

Explanation

The check fails only for a changed path that (a) leaks secrets or PII, (b) trusts or executes unvalidated input, or (c) bypasses approval or allowlist controls. I found no changed path that meets any of these. Prior failure (permission window in writeSkillFileAtomically). I confirmed the mechanism in the head revision (src/services/skills/SkillsManager.ts, lines 780–820). The sidecar is created with fs.writeFile(..., { flag: "wx" }) and no mode. The payload is written. The target mode is then read with fs.stat, and fs.chmod runs before fs.rename. In the base, updateSkillModes wrote through fs.writeFile(skill.path, ...), which kept the existing inode and its mode. So the code is as the prior run described. The claim is still not a failure under this check. It describes a transient permission window on a SKILL.md file. Nothing in the evidence shows that the file holds secrets or PII. The exposure needs a local reader on a shared host that polls the skills directory during a short window. That is a permissions-hygiene defect for the author to fix (create the sidecar with the target mode or 0o600 through fs.open(..., "wx", mode)). It is not a leak of secrets or PII under the stated criteria. Other changed paths checked: - Skill names pass validateSkillNameShared before creation (createSkill, around line 851 of the head file). - updateSkillModes resolves skill.path with fs.realpath. It then enforces assertTargetWithinSkillsRoots and re-checks the resolved path inside the directory queue. This is a containment check, not a bypass. - handleUpdateSkillModes (src/core/webview/skillsMessageHandler.ts) only validates required fields. It forwards mode slugs into YAML through matter.stringify. No code executes that input. - The new getSkillsState() API and diagnostics fields expose only skill metadata and parse-error locations. They add no secret or PII data. - The stale-sidecar sweep matches a filename pattern under known skills roots only. It does not touch approval or allowlist logic. - No approval, allowlist, or auto-approval control is changed or bypassed in the changed paths. Gaps. I did not run the tests. The containment race noted by the prior run needs write access to the skills root. I treated that as supporting context only. Prior failure refuted: The mechanism is accurate: writeSkillFileAtomically (lines 791-820) creates the sidecar with default mode, writes the payload, then chmods to the target mode. The prior failure's condition is a transient permission window on SKILL.md. It is not a leak of secrets or PII, which this check requires. No secret or PII in SKILL.md is shown. The exposure needs a local reader racing a short window. The...

Full details: Persistence Integrity

Explanation

The changed persistence paths in src/services/skills/SkillsManager.ts at the reviewed head (refs/coderabbit/pre-merge-check/head) do not show a concrete loss or corruption path under the Persistence integrity criterion. What I checked: - Atomic writer (writeSkillFileAtomically, lines 769-830): Every SKILL.md publish stages a same-directory temp file with exclusive wx creation, awaits the full write, then awaits fs.rename over the target. The catch block removes the temp file only when this call created it, and rethrows the original error. Failures therefore surface to the caller. - Directory serialization (withDirectoryLock, lines 737-759): Writes and the sidecar sweep share a per-directory queue. A disposed manager rejects queued work before it runs. - Create rollback (createSkill, lines 884-925): The write is awaited inside try/catch. On failure, the code removes only the empty skill directory it created (fs.rmdir), so it does not delete sibling skills. Errors propagate. - Mode update (updateSkillModes, lines 1241-1315): The read-modify-write runs as one queued unit. Serialization uses the same atomic writer. It fails closed on a retarget and on stat errors other than ENOENT/uncoded errors, and the caller sees the error. - Stale-sidecar sweep (lines 165-200): The sweep deletes only temp files matching the writer's generated name. It skips files younger than ten minutes and awaits its operations. Residual concerns, none of which meet the criterion: - The sweep does not coordinate with writers in other extension-host processes. A sidecar from another process that stays unrenamed for more than ten minutes could be deleted. The affected rename then fails with an explicit error. Nothing is silently lost or partially published, and the window is far longer than a normal write. - The atomic writer does not fsync before rename. A power loss could leave an empty file. This is a durability limit, not a missing rollback or unawaited operation, and the criterion does not name it. - updateSkillModes is a read-modify-write. An external editor that saves between the queued read and the rename can lose that edit. This pattern predates the PR, and the PR does not make it worse. The PR description's review-ownership section concerns approvals, not persistence. I did not use it for this check.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Codecov Report

❌ Patch coverage is 97.71429% with 4 lines in your changes missing coverage. Please review.

📢 Thoughts on this report? Let us know!

CodeRabbit review findings — all addressed in 5153db6

• e2e fixture: now a double-quoted description with unescaped inner quotes (the exact [Bug] SKILL.md YAML parsing fails silently when description contains unescaped double quotes #859 failure mode), so it reproduces the unescaped-quote parse failure instead of failing earlier on the : sequence.
• e2e teardown: only removes the skill directories this suite created; pre-existing fixtures under .roo/skills are left intact.
• api-get-skills-state.spec: the partial doubles need a documented double-assertion; added a beforeEach comment explaining why it is a last resort.
• serialize overlapping discovery runs (major): discoverSkills() is now a non-async serializer chaining each run onto a discoveryChain promise; a delayed older scan can no longer append a stale diagnostic after a newer scan observed the repaired file (regression test added).
• quote hint scoped to the failing line: the "unescaped double quotes" hint is only emitted when the parser error mark is on the description line itself (regression test with a valid quoted description plus an error on another line).
• state transition that clears diagnostics: the skills message test now asserts stored skills/diagnostics are cleared, including a message that omits skills entirely.
• nitpick (typed message fixture / non-empty diagnostics assertion): the omitted-newSkillModeSlugs test declares a WebviewMessage instead of an as cast, and asserts a concrete non-empty diagnostic list is forwarded.

Additional root-cause fix found while making the e2e deterministic: gray-matter keeps a global content-keyed cache that it populates before parsing, so a frontmatter that throws on first parse was cached with an empty data object and every later parse of the same content silently returned that object instead of re-throwing (resurfacing the misleading "missing required name field" symptom). loadSkillMetadata() now parses with explicit empty options to bypass the cache, with a unit-level regression test that a re-scan of unchanged malformed content keeps reporting the parse failure.

src subset: 101/101 | webview: 45/45 | e2e (skills-diagnostics): 1 passing
check-types 11/11 clean | eslint clean, suppression counts unchanged

What 1321 is actually waiting on

Open human verdict: review 5580793451, from edelauna, state CHANGES_REQUESTED, submitted 2026-10-20T04:34:10Z, body 0 bytes. It is still CHANGES_REQUESTED at head 9b6dc1a. My own review read at 2026-11-09T23:07:04Z (GET /pulls/1321/reviews?per_page=100, page 1 then page 2: 86 rows then 0, so the read is complete) splits those 86 rows as COMMENTED 57, CHANGES_REQUESTED 25, DISMISSED 4. Of the 25 change requests, 24 are coderabbitai[bot] and one is edelauna's - one human verdict, not twenty-five.

Bot rows are answered or fixed. My two thread replies, both verified by GET /pulls/comments/ at 23:07:33Z (status 200): 4518837603, 3,223 bytes, in reply to 4516247317 on src/services/skills/SkillsManager.ts:152; 4518838207, 3,330 bytes, in reply to 4516247309 on SkillsManager.ts:150. My thread census of 22:41:30Z stood at 37 threads, 35 resolved, 2 unresolved, 21 outdated.

CI at 22:59:43Z, filter=all: 35 legs, total_count 35 (pagination complete), 34 completed/success, 1 completed/skipped (CodeRabbit, "Review skipped: excluded by label configuration"), 0 in progress. mutation-diff completed/success with 8 annotations; codecov/patch success at 97.71% of diff hit and webview-patch success at 100.00%. All six contexts the main ruleset requires are completed/success at this head; that list is from my own read of ruleset 15494382 "Protect Main" at 23:07:04Z with strict_required_status_checks_policy false: check-translations, platform-unit-test (ubuntu-latest), platform-unit-test (windows-latest), compile, knip, e2e-mock. REST says mergeable true and mergeable_state blocked.

Labels as of 22:59:43Z read ["awaiting-author"] (earlier in this round the same endpoint returned awaiting-coderabbit plus coderabbit-review-active). I issued no label mutation, no review mutation, no resolve and no review re-request in this unit. The label claims the author owes something; what is actually open is a human change request that this token cannot dismiss, cannot approve and cannot re-request, and a push would not clear it - this unit pushed nothing, and I am not treating dismiss_stale_reviews_on_push as a lever against another person's verdict.

What the next mutation report on this PR will clear and what it will not

The push landed as 6b59564, parent 9b6dc1a. Two files changed, 57 insertions and 2 deletions, and both of them are specs: webview-ui/src/components/settings/tests/SkillsSettings.spec.tsx and webview-ui/src/context/tests/ExtensionStateContext.spec.tsx. No source file changed and no Stryker directive was added.

Priced before writing, with the gate's own selectFromGit at 23:22:30Z against base e487cd7 (merge base 5319fe9): extension 282 changed executable lines, webview 37, identical to the head I pushed from. Feeding the two spec paths through buildManifest on their own returns 0 package entries, because packageForPath returns nothing for a path carrying /tests/ - gate line 85 is that exclusion list and both files are in it. So this push charges no lines: the deciding figure on this PR is still the extension package at 282 of 500 with 218 of headroom, and webview's 37 is not what prices it.

What the specs now do, with the command that shows the suites still pass. Interpreter C:/work/Zoo-Code-fork/.tmp-node22/node_modules/node/bin/node.exe v22.23.4, cwd webview-ui, entry webview-ui/node_modules/vitest/vitest.mjs. Before the edits the shipped suites printed Tests 22 passed (22) for SkillsSettings.spec.tsx and Tests 42 passed (42) for ExtensionStateContext.spec.tsx. After them the same two commands print Tests 22 passed (22) and Tests 44 passed (44), both exit 0. In the same directory tsc --noEmit exits 0 with no output, and eslint on the two spec files exits 0 with no output, both with and without --prune-suppressions. SkillsSettings.spec.tsx now asserts the exact text of each diagnostics row's location span rather than a substring match, and the case set gained a third row carrying both a line and a column so the inner column template is pinned too. ExtensionStateContext.spec.tsx now seeds skills and skillDiagnostics through initialState, and separately asserts both come back as empty arrays when initialState omits them.

Each of the ten reported mutants re-run against the suites as they now stand, one at a time, source restored after every run:

• SkillsSettings.tsx:40, ArrayDeclaration to []: the mutated file is byte-identical to the original, sha256_16 4154162d9c72d047 on both sides, and the suite prints Tests 22 passed (22) at exit 0.
• SkillsSettings.tsx:249, StringLiteral replacing the outer location template: Tests 2 failed | 20 passed (22) at exit 1. The same replacement applied to the inner column template on that line: Tests 2 failed | 20 passed (22) at exit 1.
• SkillsSettings.tsx:250, StringLiteral replacing the empty fallback: Tests 1 failed | 21 passed (22) at exit 1.
• ExtensionStateContext.tsx:391 and :393, ArrowFunction to () => undefined: each prints Tests 2 failed | 42 passed (44) at exit 1.
• ExtensionStateContext.tsx:391 and :393, LogicalOperator to the form the report names, "initialState?.skills && []" and its twin: each prints Tests 2 failed | 42 passed (44) at exit 1.
• ExtensionStateContext.tsx:391 and :393, ArrayDeclaration to the report's "["Stryker was here"]": each prints Tests 1 failed | 43 passed (44) at exit 1, failing the empty-initialState case, which is the case that fallback changes.

Two of the ten will still read Survived on the next report, and that is expected rather than a failed fix. SkillsSettings.tsx:40 is equivalent by construction: ArrayDeclaration replaces an empty array literal with an empty array literal, so the mutated file is the same bytes as the original and no test can tell them apart. SkillsSettings.tsx:252 is React's key for a diagnostics row; the reported replacement is an empty template literal, the rendered text and the element structure do not change, jsdom exposes no key, and that mutant survives the shipped suite and the strengthened exact-text suite alike. Under the gate's own guidance the route for that one is a narrow, reasoned exclusion with maintainer approval, which is a maintainer call and not mine to make.

CI at the new head, one read with the filter named: GET /repos/Zoo-Code-Org/Zoo-Code/commits/6b5956473168e9459b737724ce838f28f2d91a93/check-runs?per_page=100&filter=all at 23:32:15Z returned 17 legs with total_count 17 on a single page - 1 completed/success, 1 completed/skipped, 5 in_progress (mutation-diff 129515329886, knip, e2e-mock, platform-unit-test windows-latest and the CodeQL analyze leg) and 10 queued, so at that moment no required context had finished on this head and I am not claiming green. The extension advisory - 282 lines and a run that exceeded 12 minutes - will keep printing on this PR whatever the specs do, because that package has never been measured.

New gate report for head 6b5956473168 read once, and what moved since 9b6dc1a639db

Upstream main read in this same call: 332d30fbb9d8852a61b4512a5ce550e10cf84f24 (2026-11-10T00:23:48Z). Widths below are quoted against that tip.

The read. One read, filter GET /repos/Zoo-Code-Org/Zoo-Code/issues/1321/comments?per_page=100 (issue-comment listing, picking bodies that carry the zoo-code-mutation-report marker), moment 2026-11-10T00:23:26Z→00:23:27Z, page 1 returned 70 comments and exactly one mutation-report body, comment 6663025908, which the gate updates in place. It now carries head:6b5956473168e9459b737724ce838f28f2d91a93 blocking:2 advisories:1, body 3,635 B (sha256-16 996127787e132ce5). The copy I held for 9b6dc1a639db from that same comment id was 5,052 B (2197a9bdb3487a9f) with blocking:10 advisories:1.

Both package rows, printed the way the report prints them.

Survivor tables as printed — one file now, webview-ui/src/components/settings/SkillsSettings.tsx:

webview-ui/src/context/ExtensionStateContext.tsx no longer appears: its six rows (391 and 393, ArrowFunction / LogicalOperator / ArrayDeclaration) are gone from the report, which prints only Survived and NoCoverage mutants, so their absence means killed. The advisory block is unchanged, word for word: “extension mutation run exceeded 12 minutes. Split the PR or obtain a maintainer-reviewed narrow exclusion.”

The delta, with the deciding package named on every figure — every one of them is the webview package. Surviving/uncovered went 10 → 2 (webview). Killed went 21 → 29 (webview), so eight of the ten named rows died. Valid stayed 31 and changed executable lines stayed 37 (webview) — the push added no new code line for the gate to charge, it only added specs. Timeout stayed 0 (webview). No coverage stayed 0 (webview). The blocking count went 10 → 2 and advisories stayed 1, and the report body shrank 5,052 B → 3,635 B because two per-file tables are gone. Nothing in these figures comes from the extension package: its row prints 0 valid, 0 killed, 0 survived because its run never completed, so extension mutation behaviour is unmeasured, not clean.

Forecast check. Comment 6754372427 forecast eight of the ten dying and two surviving, naming SkillsSettings.tsx:40 and SkillsSettings.tsx:252. The new report prints exactly those two rows and nothing else, so the forecast is confirmed rather than corrected — the survivor set matches by name, not just by count.

Two sentences in the first version of this comment are corrected in place rather than left standing: “at exit 1? no: exit 0” superseded — the measured :40 arm prints Tests 22 passed (22) at exit 0; and “my measured arms could not separate” for :252 superseded — no :252 arm was re-run this round, so that row rests on the forecast plus the new report printing it, not on a fresh local measurement. :40 survives because ArrayDeclaration replaces an empty array literal with an empty array literal: the mutated file is byte-identical to the original (sha256-16 4154162d9c72d047 on both sides) and the suite prints Tests 22 passed (22) at exit 0. It is equivalent by construction, so no assertion can reach it. :252 is the empty-template StringLiteral in the same expression. Stated narrowly: this round I re-ran the :249 (both spellings), :250, :391 and :393 arms and did not re-run a :252 arm, so that row rests on the forecast plus the new report printing it, not on a fresh local measurement from me.

The two extension figures, stated as what they are. The extension row still prints 282 changed executable lines against the gate’s 500-line cap, unchanged from the 9b6dc1a639db report, and unchanged because of what the push touched: both files are specs, and the gate’s own path rules give __tests__ paths no charge (packageForPath returns undefined for them, and they are in EXCLUDED_PATH_SEGMENTS), so spec paths add nothing to the executable-line count. The extension 12-minute overrun is still standing whatever the specs do: the mutation-diff job ran 23:32:02Z → 23:49:34Z, which is 17m32s, and the advisory line is identical in both reports. Splitting or a maintainer-reviewed narrow exclusion remains the only route that changes that row.

Arms, read once, filter GET /repos/Zoo-Code-Org/Zoo-Code/commits/6b5956473168e9459b737724ce838f28f2d91a93/check-runs?per_page=100&filter=all at 2026-11-10T00:22:21Z (23 check runs, total_count 23, tally 22 completed/success and 1 completed/skipped). Each context separately, with its own conclusion:

• check-translations — check run 129515335371: completed/success, 23:33:19→23:33:55.
• platform-unit-test (ubuntu-latest) — check run 129515335406: completed/success, 23:33:37→23:42:02.
• platform-unit-test (windows-latest) — check run 129515335732: completed/success, 23:32:10→23:46:45.
• compile — check run 129515335419: completed/success, 23:33:35→23:37:55.
• knip — check run 129515334808: completed/success, 23:32:06→23:33:28.
• e2e-mock — check run 129515329707: completed/success, 23:32:02→23:37:02.
• mutation-diff — check run 129515329886: completed/success, 23:32:02→23:49:34 (17m32s), 4 warning annotations. Success means the gate posted an advisory report, not that the mutation pass is clean: the extension leg is incomplete and the webview leg still reports 2 surviving mutants.
• CodeRabbit — check run 129523582524: completed/success, 23:50:14→23:59:49. (the run that posted review body)
• CodeRabbit — check run 129515359592: completed/skipped, 23:32:04→23:32:04. (the earlier run in the page, skipped)

Of the six contexts the branch ruleset requires, all six report a conclusion on this head: the two named above and compile (completed/success), knip (completed/success), e2e-mock (completed/success), check-translations (completed/success). The two platform-unit-test legs are named platform-unit-test (ubuntu-latest) (completed/success) and platform-unit-test (windows-latest) (completed/success) in the check-run listing, with the matrix value in parentheses.

What is still not measured. Extension mutation behaviour (0 valid because the leg times out), whether the eight newly killed webview rows stay killed on a re-run — answered below at head 504611564b61: all 31 valid webview mutants killed, none surviving — and requestedReviewers, which I did not query. The 2 remaining webview rows are the ones I forecast and the :40 one is provably equivalent, so the open decision on this PR is the extension split-or-exclusion conversation, not more webview assertions.

Delta at head 504611564b61 — both named rows now killed, read from the artifact first, then priced, then read on CI (added in place 2026-11-10T02:41Z)

Row set re-read from the artifact, not from this comment. Artifact 13721721618 (changed-code-mutation-report, 210447 B, sha256/16 9284fe2754dc0338, created 2026-11-09T23:49:25Z, run 42721208263, job 129515329886, four entries: pr-comment.md 2309 B baaf687571fcabac, webview/mutation.json 211822 B 7c286452fea58417, webview/mutation.html 446912 B 3bbe8c02e6b1d9ad, webview/vitest-related.json 628696 B 43773c216019844e). Its pr-comment.md carries head:6b5956473168e9459b737724ce838f28f2d91a93 blocking:2 advisories:1 and the rows quoted above. Counting webview/mutation.json with the gate own counters prints valid 31 / killed 29 / timeout 0 / survived 2 / noCoverage 0 / ignored 0 and two blocking records. Slicing the report column ranges out of the head source confirms the two records are source-position rows, and gives the replacement text: :40 columns 68-89 replace rawSkillDiagnostics]) with [] (ArrayDeclaration, dependency array emptied, 21 bytes removed / 2 added); :252 columns 21-81 replace the template `${diagnostic.path}-${diagnostic.line}-${diagnostic.column}` with `` (StringLiteral, 60 bytes removed / 2 added).

Both arms reproduced in the declaring package. Interpreter C:\work\Zoo-Code-fork\.tmp-node22\node_modules\node\bin\node.exe (v22.23.4), directory C:\work\Zoo-Code-fork\wt-r130-1321\webview-ui, entry node node_modules/vitest/vitest.mjs run src/components/settings/__tests__/SkillsSettings.spec.tsx, env CI=1, VITEST_MAX_THREADS=1. Clean control: Tests 22 passed (22) exit 0. Arm :40 (mutant file sha256/16 8e8dfdc0639e5f07): Tests 22 passed (22) exit 0, no failing test name — it survived the suite that already existed. Arm :252 (mutant sha256/16 4d5e79091dba6cac): Tests 22 passed (22) exit 0, likewise. After each arm the source was rewritten from the captured preimage and git hash-object -w webview-ui/src/components/settings/SkillsSettings.tsx printed 9c250e85149fbb2dfd72ec0e71a31625ce0b9d4c, the committed blob for that head at that path, with git status --porcelain empty. No blob was left moved between arms.

Classification, done before anything was written. Both rows are killable, and the mechanism is stated for each. :40: rendering with skillDiagnostics: [] and then pushing a non-empty list through the mocked extension state and rerendering the same tree shows the warning on clean source and fails on the mutant with Unable to find an accessible element with the role "alert" — the memo frozen at first render never sees the pushed value. :252: with three distinct diagnostics the rendered rows are byte-identical under clean and mutant (three rows, outerHTML lengths 121/121/121 on both, same text in both orders), so nothing about the text changes; what changes is list identity. Tagging each row on first render and reordering the list shows the row now reading broken-4 is the node first created for broken-4 (carried tag 2) on clean source, and the node first created for broken-2 (carried tag 0) under the mutant, because all three rows then carry the same key and React matches them by position; React 18.3.1 also logs 4 duplicate-key errors for the mutant (captured console entries 5, against 1 on clean, the extra one being the same react-i18next warning both sides). The shipped assertions name those visible consequences; no weaker assertion was written to manufacture a kill, and no directive or suppression was added.

Where the assertions went, and what ran. webview-ui/src/components/settings/__tests__/SkillsSettings.spec.tsx — the spec that owns the behaviour, package-local per AGENTS.md, with no source change: SkillsSettings.tsx is the same blob 9c250e85149fbb2dfd72ec0e71a31625ce0b9d4c at the new head. The spec went 22 to 24 it blocks and 16401 to 19609 bytes, committed blob 4452155988164676a1bdcc8bbe5d3af08beb4d9f (parent 6abaa3d3fa5984f8d6da2b0d5ae1c347aaa83206). Gates run in this span, each named with directory, entry and printed number: whole package node node_modules/vitest/vitest.mjs run in webview-ui → Test Files 188 passed (188), Tests 2498 passed (2498), 48.25 s; node ../node_modules/typescript/bin/tsc --noEmit -p tsconfig.json in webview-ui → exit 0, empty output; node_modules\.bin\eslint.cmd --max-warnings=0 src\components\settings\__tests__\SkillsSettings.spec.tsx in webview-ui → exit 0; node node_modules/prettier/bin/prettier.cjs --check webview-ui/src/components/settings/__tests__/SkillsSettings.spec.tsx at the repo root → exit 0, All matched files use Prettier code style!, and the --write run printed (unchanged). src/eslint-suppressions.json was not touched because no file under src was touched, and webview-ui carries no suppressions file.

Spec paths charge nothing. The gate path rule reads EXCLUDED_PATH_SEGMENTS containing /__tests__/ among the excluded segments, and packageForPath("webview-ui/src/components/settings/__tests__/SkillsSettings.spec.tsx") returns undefined where the same call on webview-ui/src/components/settings/SkillsSettings.tsx returns the webview package entry. Re-pricing at the delivered head lists 2 webview files (SkillsSettings.tsx, ExtensionStateContext.tsx) at 37 changed executable lines; the spec is not among them.

Push, hooks, head, price. One push. Commit 504611564b612b7869ca93e8f529700243031588 on parent 6b5956473168e9459b737724ce838f28f2d91a93, 1 file changed, 95 insertions(+). The pre-commit hook ran lint-staged for the staged file, including pnpm --dir webview-ui exec eslint --max-warnings=0 --no-warn-ignored, and left the file unformatted-forward: worktree, index and committed blob are all 4452155988164676a1bdcc8bbe5d3af08beb4d9f, so the committed blob is what I measured. The pre-push hook printed @roo-code/vscode-webview:check-types: cache miss, executing 688cc4404a100b93, Tasks: 11 successful, 11 total, Cached: 10 cached, 11 total, Time: 5.047s, and the push line printed 6b59564731..504611564b 504611564b612b7869ca93e8f529700243031588 -> fix/skill-frontmatter-yaml-859. HEAD from GET at 02:18:53Z: 504611564b61…, base main / 5319fe91fb0caf4aa4ec66aac68fa03744ebbc23, head repo the fork, mergeable true, mergeable_state blocked, labels awaiting-author, 56 commits, +5249/-528. My own git merge-base for the new head printed 5319fe91fb0caf4aa4ec66aac68fa03744ebbc23 against 5319fe91fb0c…, e487cd7b2a49…, a7157157239a… and 18861b2e3572… — one merge base for all four, so the tip move tonight changes nothing for this PR. selectFromGit at the delivered head printed extension 282 changed executable lines over 3 files and webview 37 over 2 files, against caps of 500 lines and 400 selectors, with no advisories.

Arms, read once. Filter GET /repos/Zoo-Code-Org/Zoo-Code/commits/504611564b612b7869ca93e8f529700243031588/check-runs?per_page=100, moment 2026-11-10T02:39:44Z, page 1 returned 22 rows and total_count 22. The six contexts the branch ruleset requires, each with its own conclusion: check-translations 129584516965 completed/success 02:18:35→02:19:13; platform-unit-test (ubuntu-latest) 129584517094 completed/success 02:19:31→02:30:18; platform-unit-test (windows-latest) 129584517076 completed/success 02:18:32→02:28:15; compile 129584517017 completed/success 02:18:40→02:21:56; knip 129584517067 completed/success 02:19:39→02:20:51; e2e-mock 129584520175 completed/success 02:19:13→02:24:05. mutation-diff from its own leg: job 129584520364 in run 42741838888, completed/success 02:20:02→02:37:11, artifact 13730661199 (213073 B, 30ba8e3d3871a8da, created 02:37:02Z). CodeRabbit separately: check run 129589529196 completed/success 02:31:30→02:39:23, which is the run that evaluated this head; the push-moment run 129584534546 was completed/skipped. Also on the page and all completed/success: webview-visual 129584520075, extension-host-visual 129584520072, theme-fixtures 129584519668, Build test VSIX 129584516959, dependency-review 129584516807, invisible-chars 129584516540, CodeQL 129585324112, Analyze (javascript-typescript) 129584515313, codecov/patch 129589301210, codecov/patch/webview-patch 129589314575, and the two reconcile rows.

Delivered versus forecast. The leg priced a merge candidate, not the raw head: its own log prints Merge 504611564b612b7869ca93e8f529700243031588 into 18861b2e357203e6cb4d93930aa2ed122b1eecea at checkout e225aa8c4ea6b11ff9731d6208e6075abec7c41a, so the second parent is what CI had for main (lead reports that tip as current — heard, not read by me with my own command in this span). The artifact for this head exists and is dated after the merge, which is what makes it pricing evidence rather than a green row. Its pr-comment.md (1420 B) prints head:504611564b612b7869ca93e8f529700243031588 blocking:0 advisories:1; the table reads webview 37 changed executable lines / 31 valid / 31 killed / 0 timeout / 0 survived / 0 no coverage / Passed, and extension 282 / 0 / 0 / 0 / 0 / 0 / Advisory incomplete. The gate counters over webview/mutation.json (228671 B) print valid 31 / killed 31 / timeout 0 / survived 0 / noCoverage 0 / ignored 0 with blocking: [], and the per-file view shows SkillsSettings.tsx at 19 mutants, all Killed, and ExtensionStateContext.tsx at 12, all Killed. My forecast before the push was 31 valid / 31 killed / 0 surviving / blocking 0 on webview; CI printed those figures, so the difference between forecast and printout is zero on every webview figure. Extension: 282 changed executable lines and 0 valid, unchanged from the 6b5956473168 report, its mutation behaviour unmeasured — which is what I have claimed all along, and the two packages are named separately on every figure above.

Corrected in place. In the paragraph above, the clause “whether the eight newly killed webview rows stay killed on a re-run, and” superseded — measured at this head: all 31 valid webview mutants killed, none surviving. The clause “The 2 remaining webview rows are the ones I forecast and the :40 one is provably …” is superseded by the same measurement: neither row survives at 504611564b61, and the :40 arm was re-run here and died. The line about check run 129515329886 is left standing because it is scoped to head 6b5956473168 and remains true there.

Residual classes, by name. (1) Extension mutation behaviour: 282 changed executable lines, 0 valid, leg incomplete — CI own log prints core Stryker preflight could not start: spawnSync /tmp/stryker-launch-…/stryker ENOENT and Could not write the job summary: EISDIR; I ran no local Stryker preflight on this leg because the gate reads working-tree files, so a merge candidate cannot be priced locally. (2) packages/core/src/value.ts:1 Survived BooleanLiteral, an advisory outside the changed scope. (3) Review state, measured after the push: 87 reviews (COMMENTED 57, DISMISSED 4, CHANGES_REQUESTED 26), 0 APPROVED before and 0 after, so the dismiss-on-push rule had nothing to dismiss on this PR; the human zero-byte CHANGES_REQUESTED 5580793451 (2026-10-20T04:34:10Z) and the CodeRabbit CHANGES_REQUESTED 5834596925 (2026-11-09T23:59:40Z, 11464 B) both still stand, because a push does not clear CHANGES_REQUESTED; 38 review threads, 35 resolved, 21 outdated. I resolved no thread, approved nothing, dismissed nothing and requested no reviewer; the label is still awaiting-author and mergeable_state is still blocked. (4) requestedReviewers still not queried. (5) The routing question owed in comment 6753382426 for the delayed-body test on src/api/providers/utils/error-handler.ts is still open, with the 1311 leg budget measured at 449 of 500.

Answering review body row 5836803142 (CHANGES_REQUESTED, 2026-11-10T04:35:48Z), whose whole statement is the 111 bytes "Pre-merge checks failed. Please resolve the failing checks before merging."; GET /pulls/1321/reviews/5836803142/comments returns 0 attached comments, so the row carries no detail beyond that sentence.

Measured on head 0637006 at 2026-11-10T05:09Z, one read per head, base repository.

The six contexts that Protect Main (ruleset id 15494382) requires, read from GET /rulesets/15494382 at rules[type=required_status_checks].parameters.required_status_checks - exactly these six, all integration_id 15368, with strict_required_status_checks_policy false:

Both advisory gates are green on this head too: mutation-diff 129621049316 completed success at 04:23:20Z, extension-host-visual 129621049258 completed success at 04:18:16Z.

The only non-green row on this head is not a check run and is not in that list. GET /commits/063700625b2eccee39ca178b2047ad2e31a79eae/status returns exactly 1 legacy status: id 58494460483, context "Zoo Code / PR review gate", state pending, updated_at 2026-11-10T04:36:08Z, pointing at Actions run 42756871245. It is not among the six contexts Protect Main requires.

I am not going to push to make that row move. The same arms read shows 26 check-run rows attached to this head today (20 distinct names, 26 distinct run ids) against the 18-to-22 range I quoted earlier: the difference is 8 rows named "Zoo Code / reconcile PR review state (1321)" and "Zoo Code / resolve PRs to reconcile", all completed success, appended by the reconcile workflow between 04:07Z and 04:59Z. A push would re-queue all 26 to answer one legacy status that the ruleset does not ask about.

Whether the merge button reads that legacy status is a platform question for a maintainer, not something this campaign can settle by pushing: nothing in the required list is failing on this revision, and the campaign will not add a commit that changes nothing but the queue.

Recorded rather than acted on: labels [awaiting-author] at 05:09Z with updated_at 04:38:05Z, mergeable true and blocked, 39 review threads with 37 resolved and 2 open. Both open roots, 4516247309 and 4516247317, are answered in-thread in this unit. No push, no label change, no review request, no thread resolution, no dismissal.

1321: priced split plan, published before anything opens

Marker: plan head 0637006; blocking mutation rows 37; advisories 0; proposed units 13; nothing opened.

Every number below is a measurement of one diff against the merge base named on its own row, produced by CI's own counting code (scripts/stryker-diff.mjs as it exists at head 0637006, copied verbatim to C:/work/Zoo-Code-fork/.tmp-b1-1404/r176-head-stryker-diff.mjs and run locally with node v22.23.4): caps MAX_CHANGED_LINES = 500 (line 11) and MAX_MUTANTS = 400 (line 12), per package; executable lines are the full span of every changed expression/statement found by walking the TypeScript AST, with __tests__, __mocks__, fixtures, test-utils, *.test|spec.*, *.d.ts and *config.* excluded. Selector ranges are contiguous ranges fed to Stryker, not mutant counts.

1. What 1321 costs at its own merge base 5319fe9

Advisories 0. Whole PR: 56 files, +5,248 / -528 = 5776 added+deleted lines, 57 commits.

Read that carefully: at this base neither the 500-line gate nor the 400-mutant gate is breached. The Out of Scope Changes finding is therefore not a gate failure; it is a contract failure - the PR carries sidecar-lifecycle machinery (sweep, retry, cross-instance queue, atomic publish, dispose guards, write-target containment) that the YAML-diagnostics unit never needed, and 2,746 a+d of settings.json translation edits of which only 41 a+d across 18 files carries diagnostic-labelled strings. A fourth explanatory comment cannot fix that; a split can.

2. The 13 units, each priced against its own merge base

Chain (single parent chain, U1 on top of 5319fe9 ... T4): U1=70eb304a654e, U2=085d3817acbf, U3=c2cd5be5d40d, U4=ee6a2885e974, U5=ca148c436d3e, U6=bacffad08e78, U7=2f67e1c6b536, U8=ba1be89fba78, U9=4db919edc398, T1=8fe66339ac9e, T2=d6feaa36908c, T3=d9afb48975c8, T4=3e16e0606f30.
Chain end 3e16e0606f30 reproduces 1321 exactly: git diff --name-only 3e16e0606f30 063700625b2eccee39ca178b2047ad2e31a79eae prints nothing (the screenshot PNG is materialised by checkout, not by text surgery).

3. Where the boundary is drawn, and why

Content that touches the publish path moves out: staging a temp sidecar, the per-directory queue, the pre-rename validateTarget, the fs.stat(filePath) mode-preserve read, the chmod, the rename, the EEXIST cleanup, dispose(), sweepStaleSkillTempFiles, and the write-target path/symlink containment. Content that parses, serializes or reports frontmatter stays: getFrontmatterLine, the frontmatter branch of loadSkillMetadata, recordDiagnostic, the diagnostics field, SKILL_YAML_DUMP_OPTIONS, discovery-time symlink accounting, and the diagnostics UI. Consequence for the reviewer's list: sidecar cleanup/retry, cross-instance locking, disposal and permission handling, write-target safeguards, unrelated settings translations and the auto-approval label test all leave the retained unit; YAML parsing diagnostics, YAML-safe serialization, the diagnostics UI and directly related tests/labels stay in it.

4. The 37 mutation rows: killable inside the retained unit, or moving with the machinery

All 37 live in src/services/skills/SkillsManager.ts (27 Survived + 10 No coverage). Unit ownership under the boundary above, using head line numbers:

• U1 keeps 15 rows killable with a test row inside the retained unit: 32, 33, 39 (getFrontmatterLine), 102 (diagnostics field), 452, 461, 462, 462, 467 (loadSkillMetadata frontmatter/description handling), 563, 563, 563, 563, 564, 566 (recordDiagnostic). Their kill rows are frontmatter/YAML/diagnostic tests that already exist or belong in U1's own spec parts (U2/U3).
• U6 takes 12 rows: 154, 171, 171 (Survived, initialize guard and sweepStaleSkillTempFiles) and 178, 178, 178, 178, 178, 179, 186 (No coverage, the stale-sidecar Dirent/unlink branch) plus 1402, 1442 (setupFileWatchers/watchDirectory disposal guards).
• U7 takes 5 rows: 740 (realpath lock key), 755, 755, 755, 756 (queue identity check) - the cross-instance directory lock.
• U8 takes 5 rows: 789, 793, 797 (No coverage, staged-write disposal throws and EEXIST cleanup) and 817, 826 (mode-preserve stat, ownership/EEXIST condition).
• U2, U3, U4, U5, U9, T1-T4 take 0 rows.

The 178/179/186 No-coverage rows are a fixture artifact and must not be read as untested code: the existing sweep fixture (SkillsManager.spec.ts describe at 4305, sidecarAgeMs 4347, mockReaddir 4348-4350, mockStat 4351-4352) returns [] on the withFileTypes path, so the Dirent branch is never executed. A kill row for those three drives a Dirent entry through the sweep, and that row belongs to U6 - it is not killable inside U1 at all.

That 15/22 answer is the split boundary: the retained unit can prove its own change; 22 of the 37 rows can only be killed by tests of machinery that the reviewer asked to move out.

5. Deviations, stated rather than hidden

• The 13 chain commits sum to 5772 a+d against the PR's 5776 (delta -4). The end tree is identical; the difference is identical lines re-added inside split segments. Not hidden.
• Per-unit executable-line sums add to 285 versus 282 for the whole PR, and selector sums to 52 versus 54, because CI marks the full span of a changed statement and merges contiguous ranges: one line can therefore be counted in two units, and two separate hunks can merge into one range. Direction and size disclosed.
• 8 of 13 units are over the 400 a+d soft line (U1 463, U2 414, U4 641, U8 477, T1 774, T2 799, T3 800, T4 501); 0 breach the 1,000 hard cap. T1-T3 sit at 774-800 by design: each batch is five locales of one file with one contract point, and cutting further would split one locale file across two units.
• U2 and U3 exist only because U1's own diagnostics test additions are 680 a+d on their own; they are spec-only, so the gate sees them as packages=[] (0 executable lines). They are a size cut, not a contract point, and they are flagged as such.
• 41 a+d across 18 locale files is the diagnostic-labelled share of the 2,746 a+d of settings.json translation edits; the rest moves to T1-T4. Mixed hunks inside a locale file go with U5 rather than being cut mid-line.

6. Conditions every unit owes before its own push

1. Cold full check-types with TURBO_FORCE=1 at the unit's own head, output quoted (the 2226 lesson: a unit that does not compile at its own head is not a unit).
2. Its own extension mutation run must complete and be read: completion is stated from a named arm row (head_sha, status, conclusion, completed_at), measurement from a named table row (Valid N / Killed N / Survived N / No coverage N). If the leg prints "The mutation run was incomplete; selected test file names were not recorded", the unit reports incomplete. Job completion is not a measurement, and a green row is never banked from it (issue 2410 class: a spawn ENOENT produced an empty packages list with pass true).
3. Instrument versus measurement, kept apart: the 12-minute and 5-minute figures are constants in the instrument (tip-stryker-diff.mjs line 424 timeout: 12 * 60 * 1_000 and line 379 timeout: 5 * 60 * 1_000). The 17m32s figure is a job-level measurement (extension arm completed 2026-11-09T23:49:34Z after 17m32s, check run 129515329886 inside run 42721208263) and inside it the leg printed Valid 0 / Killed 0 / Survived 0 / No coverage 0 at both 17b2689 and 6b59564. So 37 rows is the first extension-leg measurement this campaign holds, not a regression from 2.
4. Known defect of this pricing chain that the executor must remove while re-deriving the units: under line-level porting, U1's own head does not parse (src/services/skills/__tests__/SkillsManager.spec.ts:2228, "await is only allowed within async functions") and the retained-unit end fails at src/services/skills/SkillsManager.ts:126 (a method header deferred to a later unit while its body stayed). The cause is mid-block line sets. Porting must be whole-method / whole-test-block; after re-derivation U1 owes condition 1 and the focused SkillsManager.spec.ts run at its own head before anything is pushed.
5. Cold eslint --max-warnings=0 on each changed file with src/eslint-suppressions.json restored to its head blob (30,910 B, sha512 prefix 5ECFDB0DB4B1F9E8) - --prune-suppressions rewrites it to 33,294 B (95A8918A0E5DC809) and that edit does not belong in any unit.

7. Review-finding dispositions against head blobs (all five findings verified, not argued)

8. Drift to close before the first push

main tip 18861b2 is 8 commits ahead of 1321's head (measured: git rev-list --count 063700625b2eccee39ca178b2047ad2e31a79eae..18861b2e357203e6cb4d93930aa2ed122b1eecea = 8; git merge-base = 5319fe9 = this PR's own base). Close it with one upstream merge under the campaign's no-verify-off rules before the first split unit is pushed. That merge re-anchors every price above: each unit must be re-measured against its new merge base, and the numbers in this comment are the pre-merge anchor, not a promise about the post-merge tree.

9. Where this was searched before choosing a home (repo-scoped, counts as returned)

• is:issue 1321 repo:Zoo-Code-Org/Zoo-Code -> total 4: skills: descriptor-relative SKILL.md publication to close the publish-time path race #2203 Tracking fws U1: atomic text publish primitive (safeWriteText) #2016 [Tracking] guarded-write chain (epic 1375): rules, port sources, chain-level rows #1991 Pushes dismiss stale approvals while a maintainer CHANGES_REQUESTED survives; should label-pr-review-state treat that asymmetry as intended? #2407
• "1321" in:body repo:Zoo-Code-Org/Zoo-Code -> total 6: skills: descriptor-relative SKILL.md publication to close the publish-time path race #2203 Pushes dismiss stale approvals while a maintainer CHANGES_REQUESTED survives; should label-pr-review-state treat that asymmetry as intended? #2407 chore(deps): update dependency axios to v1.16.0 [security] #400[closed] chore(deps): update build, lint, and test tooling #804[closed] [Tracking] guarded-write chain (epic 1375): rules, port sources, chain-level rows #1991 Update dependency axios to v1.15.2 [SECURITY] #177[closed]
• skills diagnostics yaml repo:Zoo-Code-Org/Zoo-Code -> total 11: fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely #1321 fix(skills): safely serialize skill frontmatter #934[closed] fix(deps): update ai sdks and providers #2276[closed] test(webview): ClineProvider parallelMode suite with viewStates pruning edges #1921 [Feat] Support custom MCP OAuth callback paths and WWW-Authenticate discovery #2219[closed] fix(webview): isolate parallel mode and provider profile writes (vps2 F3) #1560
• "Tracking" skills repo:Zoo-Code-Org/Zoo-Code -> total 19: [Tracking] Dynamic MCP Tool Loading — Implementation Roadmap #584 fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely #1321 feat(webview): view-local buffer reads and setValue persistence (N3, split 3/5 of #1546) #2235 U9 re cut, rung 2 of 4: guarded write core and its recovery readers (416 counted lines, 57 selector ranges, headroom 84) #1917 feat(editor): route the diff-view save through the guard (U8, #1375) #1916 test(webview): ClineProvider parallelMode suite with viewStates pruning edges #1921
• sidecar repo:Zoo-Code-Org/Zoo-Code -> total 8: skills: descriptor-relative SKILL.md publication to close the publish-time path race #2203 fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely #1321 [Chore] Add conservative change-based CI validation with audit gates #2325[closed] Support for persistent long-term memory (Mem0, Zep, etc.) #221 [BUG] HTTP proxy is ignored on Anthropic provider #438[closed] [BUG] HTTP proxy is ignored on Anthropic provider #439[closed]
• "write delay" repo:Zoo-Code-Org/Zoo-Code -> total 51: Tracking #1381: writeDelayMs default is 0 (DEFAULT_WRITE_DELAY_MS) #1386 [BUG] No Streaming Feedback During File Writes and Long Delays Between Multi-File Writes #1189 feat(tools): wire guarded writes into the diff-view save paths (S4b, #1375) #1408 perf(write-path): remove artificial write delays by default (part of #1375) #1381[closed] [EPIC] File Write Safety Prevent Concurrent Write Races Data Corruption #1375 feat(editor): async post-save diagnostics on chat-diff save path (L1, #1375) #1403
• "934" repo:Zoo-Code-Org/Zoo-Code -> total 29: fix(skills): safely serialize skill frontmatter #934[closed] fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely #1321 [ENHANCEMENT] Show progress for inline code editing #120 perf(webview): drop date-fns and react-icons from the webview bundle #2262 [ENHANCEMENT] Restore bounded multi-file reading in a single native tool call #1948 [BUG] Label workflow strips has-conflicts when mergeability is unknown #2025[closed]
• frontmatter repo:Zoo-Code-Org/Zoo-Code -> total 18: fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely #1321 [Bug] SKILL.md YAML parsing fails silently when description contains unescaped double quotes #859 Feature Request: Per-Mode Skill Whitelist Configuration #556 fix(skills): safely serialize skill frontmatter #934[closed] [BUG] Slash command mode frontmatter clobbers orchestrator task mode and API profile #1776[closed] fix(task): slash command mode frontmatter must not overwrite orchestrator task mode #1784[closed]
• "skill" "diagnostic" repo:Zoo-Code-Org/Zoo-Code -> total 21: fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely #1321 fix(skills): safely serialize skill frontmatter #934[closed] fix(deps): update ai sdks and providers #2276[closed] test(webview): ClineProvider parallelMode suite with viewStates pruning edges #1921 fix(vertex): Gemini 3.x on Vertex and dynamic Gemini/Vertex model catalogs #1753[closed] [Feat] Support custom MCP OAuth callback paths and WWW-Authenticate discovery #2219[closed]
• skills.json repo:Zoo-Code-Org/Zoo-Code -> total 3: Fix skill discovery inside a symlinked container directory #1843[closed] feat: add experiment-gated self-improving learning, bounded memory, background curation, and auto-create/update skill #252[closed] fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely #1321
• skills in:title repo:Zoo-Code-Org/Zoo-Code -> total 5: skills: descriptor-relative SKILL.md publication to close the publish-time path race #2203 fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely #1321 [ENHANCEMENT] Having skills being active globally #380 [BUG] Skills inside a symlinked container directory are not discovered #1842[closed] fix(skills): safely serialize skill frontmatter #934[closed]
• Also fetched records: [Tracking] Dynamic MCP Tool Loading — Implementation Roadmap #584 open, 1 comment, "[Tracking] Dynamic MCP Tool Loading - Implementation Roadmap" (MCP, not this unit); [Bug] SKILL.md YAML parsing fails silently when description contains unescaped double quotes #859 open, 0 comments, "[Bug] SKILL.md YAML parsing fails silently when description contains unescaped double quotes" (the bug this PR says Fixes #859); Tracking fws U1: atomic text publish primitive (safeWriteText) #2016 open, 2 comments, "Tracking fws U1: atomic text publish primitive (safeWriteText)"; [Tracking] guarded-write chain (epic 1375): rules, port sources, chain-level rows #1991 open, 32 comments, "[Tracking] guarded-write chain (epic 1375)"; Pushes dismiss stale approvals while a maintainer CHANGES_REQUESTED survives; should label-pr-review-state treat that asymmetry as intended? #2407 open, 0 comments (label semantics); skills: descriptor-relative SKILL.md publication to close the publish-time path race #2203 open, 0 comments, "skills: descriptor-relative SKILL.md publication to close the publish-time path race".
• Conclusion, with the searches behind it: no open tracking issue names this unit. The closest subjects are [Bug] SKILL.md YAML parsing fails silently when description contains unescaped double quotes #859 (the bug being fixed) and skills: descriptor-relative SKILL.md publication to close the publish-time path race #2203 (the publish-time path race, which is what U8/U9 answer). So the plan lives here, on the PR, and skills: descriptor-relative SKILL.md publication to close the publish-time path race #2203 will carry the publish unit's own plan when that unit is opened.

10. State of the ground right now

• Nothing opened, nothing pushed, nothing branched: no split PR exists, 1321 is untouched (head 0637006), no labels changed by me, no changeset or CHANGELOG edits.
• The 13 commits above are local scratch objects pinned at refs/recut1321/U1 ... refs/recut1321/T4 in C:/work/Zoo-Code-Fork/wt-r159-price; that worktree is clean and parked back at 0637006.
• Measurement environment: git 2.51.1 (C:/Program Files/Git/cmd/git.exe) and node v22.23.4 (C:/work/Zoo-Code-fork/.tmp-node22/node_modules/node/bin/node.exe); the verification test run quoted in the conditions section used C:/work/Zoo-Code-Fork/wt-r178-ua1/src with the node_modules junction borrowed from C:/work/Zoo-Code-Fork/wt-r130-1321 (read-only borrow; no file in that worktree was modified).
• Awaiting review before anything opens: if the maintainers want a different boundary (for example atomic publish kept with the diagnostics unit, or the locale batches cut by language family instead of five-per-batch), say so and I will re-price at the new boundary - each unit's figure moves with its own merge base.

1321: drift closed with a merge, prices re-anchored at the whole-PR level, per-unit figures held back

Marker: re-anchor head 7449df1; new merge base 18861b2; whole-PR mutation counter unchanged; per-unit re-pricing pending; nothing else opened.

1. The drift merge that was pushed

2. The whole PR, priced against the new merge base 18861b2

Counter: CI's own scripts/stryker-diff.mjs at head, copied verbatim, run locally with node v22.23.4; caps MAX_CHANGED_LINES = 500 (line 11) and MAX_MUTANTS = 400 (line 12), per package.

Advisories 0. Against the old base 5319fe9 the same counter printed 282 / 54 / 3 and 37 / 6 / 2 with advisories 0, and the PR-side delta is unchanged at 56 files, +5,248 / -528 = 5,776 added+deleted lines. Closing 8 commits of drift moved the anchor and nothing else: the main-side 8 commits touch 121 files, 56 of which overlap this PR's own 56, and none of them adds or removes a line the counter counts.

3. Which unit carries which member of SkillsManager.ts (whole-method ownership, re-derived from the merged tree with the TypeScript parser, head line numbers)

Segment tally from that derivation: U1 53, U6 10, U7 2, U8 6, U9 3 (74 segments). The boundary is the one the Out of Scope finding asked for: everything on the publish path (initialize, sweepStaleSkillTempFiles, withDirectoryLock, writeSkillFileAtomically, updateSkillModes, publishSkills, getAvailableModes, setupFileWatchers, watchDirectory, dispose) plus the write-target containment members travel with the machinery; parsing, serializing and diagnostics reporting stay in the retained unit.

4. What is not re-anchored yet, and the measurements that say so

• The re-derivation is not finished, so no per-unit line or mutant figure in this comment is new. I will not publish a per-unit number that has not been measured on the chain it describes.
• Measured state of the new cut: the retained unit head produced by the current whole-member cut fails to parse at src/services/skills/SkillsManager.ts:1128 (TypeScript parse diagnostic 1128, "Declaration or statement expected"). That is the same defect class as the two heads named in the first plan, and it is not yet removed.
• Measured state of the test file: the AST walk over src/services/skills/__tests__/SkillsManager.spec.ts at the merged tree reports 21 top-level statements (5 import declarations, 9 const declarations, 7 expression statements) and zero describe/test/it call nodes at any depth, while a string scan of the same blob finds 22 lines that open a describe/test/it/beforeEach at indent 0 or 1. Those two counts disagree, so the test-block classifier is not trustworthy yet and no test-file ownership figure from it is published here. Whole-test ownership will be stated only after the classifier's counts agree with the blob.
• Consequence for the 13 prices: the pre-merge per-unit figures in comment 6773606980 stay valid for the pre-merge chain they were measured on; they are not restated here as post-merge numbers.

5. The reviewability row, kept visible

On the chain that comment published, 8 of 13 units are over the 400 added+deleted reviewability budget (U1 463, U2 414, U4 641, U8 477, T1 774, T2 799, T3 800, T4 501); none is over the 1,000 hard cap, and no unit touches either mutation gate (500 executable lines, 400 selector ranges). That distinction is unchanged by the merge and it still holds: this is a size-compliant split, not a review-compliant one.

6. The row that must be read with every unit number

Every unit's extension mutation run may complete and still record nothing. That is measured behaviour on this PR: at head 6b59564 the extension arm completed in 17m32s (check run 129515329886 inside run 42721208263, completed 2026-11-09T23:49:34Z) and the leg printed Valid 0 / Killed 0 / Survived 0 / No coverage 0 with "The mutation run was incomplete; selected test file names were not recorded". Completion is read from an arm row; measurement is read from a table row; if the leg prints the incomplete note the unit reports incomplete. The 12-minute and 5-minute figures are constants in tip-stryker-diff.mjs (line 424 timeout: 12 * 60 * 1_000, line 379 timeout: 5 * 60 * 1_000), not measurements.

7. The required contexts at the merge commit, from one read

Source: one read of /commits/7449df18b939/check-runs (page 1, 18 of 18 runs returned, taken 2026-11-10T17:22Z) plus /pulls/1321 in the same batch. The required-context list itself came from GET /rulesets/15494382 (Protect Main): its parameters carried 0 required_status_checks entries, so the six contexts named in the campaign brief are not enumerable from that endpoint and every row below is reported as read rather than as required-or-not.

PR state from the same single read: head 7449df1, mergeable undefined, mergeable_state blocked, 58 commits, undefined changed files, +5248/-528. The labels array in that read was empty; the campaign's own earlier read (17:01:33Z) recorded awaiting-author still on the PR. I took no label action either way, and the discrepancy is reported rather than smoothed.

Nothing above was re-read a second time; rows still in progress are named as in progress at read time.

8. Ground state

8. Ground state

• Nothing else was opened, branched, pushed or relabelled. 1321 itself carries one new commit: the merge.
• Scratch chains are pinned so their objects survive: refs/recut1321/U1..T4 hold the pre-merge pricing chain (tree-identical to 0637006), refs/recut1321b/U1 holds the first whole-member attempt (commit 119d38d062d3, the one whose head parses badly). The pricing worktree is parked with no uncommitted change of its own.
• Next, in this order and nothing before it: fix the whole-test classifier until its counts agree with the blob, rebuild the 13 commits whole-method / whole-test, prove cold check-types with TURBO_FORCE=1 and the focused SkillsManager.spec at the retained unit head, re-price each unit against its own new base with the caps printed beside each figure, then publish one comment with those per-unit numbers. A fourth explanatory comment is not on this list.

9. Re-cut round: two classifier attempts, both invariants, cold proofs (head 7449df18b939)

9.1 Test classifier: the parser limitation is the finding

• Blob src/services/skills/__tests__/SkillsManager.spec.ts at 7449df18b939: 4,506 rows, 0 parse diagnostics, 21 top-level statements (5 import, 9 first-statement/const, 7 expression statements; the last expression statement is line 164 describe("SkillsManager", () => {).
• Direct node census of that source file: CallExpression 2,589; ExpressionStatement 1,490; ArrowFunction 754; PropertyAccess 1,988.
• Two attempts spent. Walks gated on isExpressionStatement(x) && isCallExpression(x) report 0 describe/test nodes against that census; setParentNodes variants do not change the result; a probe calling x.expression.getText(sf) threw and left its samples empty.
• Keyword openings counted from the blob: 22 at indent 0-1, 125 at indent >= 2, 144 total.
• Finding, recorded rather than chased: for this file the AST cannot be the authority. The authority used below is string-anchored whole-test spans from a brace/quote/template/comment-aware line scanner (span = start line through the line that returns depth to the start depth; multi-line signatures advance to the first line carrying {). No third attempt was spent.

9.2 Whole-member / whole-test ownership derived from the merged tree (string authority)

Segment tallies (every row of both blobs is owned by exactly one unit; coverage 1,509/1,509 and 4,506/4,506): SkillsManager.ts U1 67 seg / 993 rows, U6 17 seg / 209 rows, U7 2 seg / 4 rows, U8 10 seg / 241 rows, U9 6 seg / 62 rows. Spec: U1 2 blocks / 181 rows, U2 5 blocks / 1689 rows, U3 8 blocks / 407 rows, U6 19 blocks / 927 rows, U7 2 blocks / 34 rows, U8 6 blocks / 1268 rows over 21 whole-test blocks.

9.3 The two invariants, measured on two instruments

Instrument 1 - whole-member / whole-test transplant (honours the published boundary exactly):

• Parse gate FAILED. First unit head: 24 parse diagnostics, first at line 10 (TS1005 ';' expected). Sixth unit head: 42 diagnostics, first at line 6 (TS1128).
• Correction to section 4 of this comment: the earlier failed cut's defect is at src/services/skills/SkillsManager.ts line 65, and 1128 there was the TypeScript error code (TS1128, "Declaration or statement expected"), not a line number. Measured cause: a duplicated isNotFoundError body - lines 59-62 define it and lines 63-65 repeat its last two lines plus the closing brace.

Instrument 2 - whole-hunk chain (every hunk assigned to exactly one unit, applied cumulatively from 18861b2e3572):

• Parse gate PASSED: clean TS parse of SkillsManager.ts and SkillsManager.spec.ts at all 13 unit heads, so TS1128 exists nowhere in this chain.
• Tree gate FAILED: git diff --name-only 60b885ffe3a9 7449df18b939 is not empty - 6 files differ. Two mechanisms, both measured: my line-level hunk applier loses exactly one line at end-of-file on 5 files (apps/vscode-e2e/src/suite/skills-diagnostics.test.ts 138 vs 139 rows, src/extension/__tests__/api-get-skills-state.spec.ts 63 vs 64, src/i18n/__tests__/skills-error-locales.spec.ts 33 vs 34, webview-ui/src/components/settings/__tests__/SkillsSettings.visual.tsx 14 vs 15, webview-ui/src/i18n/__tests__/write-delay-labels.spec.ts 21 vs 22) and refuses the spec's 218-added-line EOF hunk (@@ -3271 +4282 @@); git apply on the same filtered patch files refuses all 13 with error: new file apps/vscode-e2e/src/suite/skills-diagnostics.test.ts depends on old contents, exit 128. 49 of the 55 text files reproduce byte-identically under the line-level applier; the binary screenshot is carried by checkout, not by patch.
• Boundary fidelity cost, measured: 450 hunks over 56 files, of which 26 hunks contain added lines belonging to 2 or 3 units and must be assigned to one. Examples: SkillsManager.spec.ts @@ +2281 adds 938 lines owned 641/239/58 by U6/U3/U8; SkillsManager.ts @@ +112 adds 78 owned 74/4 by U6/U7; SkillsManager.ts @@ +732 adds 99 owned 33/66 by U1/U8. Four units receive no hunk at all (U3, U7, T2, T3), so this instrument cannot carry the published 13-unit contract, and the mutation-row-to-unit mapping recorded above (15 rows in U1, 12 in U6, 5 in U7, 5 in U8) is not reproduced - U7 gets nothing.

9.4 Whole-hunk chain widths (a+d from git diff --cached --numstat against each row's own parent; these are added+deleted rows, NOT the CI executable-line counter, and the CI counter was not re-run against these bases this round)

Sum of the rows: 3007 a+d against 5,776 a+d for the whole PR (+5,248/-528), because the tree gate failed - the missing 2,769 rows are the six files above plus the 938-line spec hunk that my applier applied to a shorter result. Over the 400 a+d reviewability soft budget: 2 of 13 rows (U1 1,450, U6 934); over the 1,000 a+d hard cap: 1 of 13 (U1 1,450). Neither unit touches either mutation cap (500 changed executable lines, 400 mutants per package). The 8-of-13-over-400 figure in section 5 was measured on the published boundary; this 2-of-13 figure is measured on the whole-hunk boundary - different denominators, not a correction of each other.

9.5 Cold proofs at the retained first unit head

Worktree C:/work/Zoo-Code-fork/wt-r159-price, head 1c262ef1b01a (= refs/recut1321d/U1), node v22.23.4 (C:/work/Zoo-Code-fork/.tmp-node22/node_modules/node/bin/node.exe), TypeScript from the donor tree C:/work/Zoo-Code-Fork/wt-r130-1321.

• tsc --noEmit -p src/tsconfig.json (the command behind pnpm --dir src run check-types): exit 2, 24 lines matching error TS.
• Same command at this unit's own base 18861b2e3572: exit 2, 23 lines matching error TS. The base is itself red locally, so a cold type-check cannot be quoted as clean at any head built on it; the honest number is the delta.
• Delta, line by line: the head has 2 error lines the base does not - services/skills/SkillsManager.ts(147,9): error TS2741: Property 'diagnostics' is missing in type '{ skills: Map<any, any>; containerSkillPaths: Set<string>; containers: Map<any, any>; failedPaths: never[]; }' and services/skills/SkillsManager.ts(743,15): error TS2339: Property 'writeSkillFileAtomically' does not exist on type 'SkillsManager' - and loses 1 (extension/api.ts(43,14): error TS2420). Net 24 against 23.
• tsc --noEmit -p packages/types/tsconfig.json at the unit head: exit 0, 0 errors.
• Focused spec, node src/node_modules/vitest/vitest.mjs run --reporter=dot SkillsManager from wt/src: Test Files 1 failed (1), Tests 4 failed | 90 passed (94), all four failures TypeError: this.writeSkillFileAtomically is not a function - the source calls this.writeSkillFileAtomically(...) at SkillsManager.ts:743 while the method belongs to a later unit. This is the same defect the type delta reports, seen from the test side.
• check-types under TURBO_FORCE=1 could not execute here: turbo 2.11.10 (resolved globally, the worktree's own node_modules junctions were removed by git clean -fd during the chain builds) printed x Unable to find package manager binary: cannot find binary path and 0 error TS lines, so it is not evidence of anything. The narrower tsc set above is labelled as its substitute.
• Not contradicted by CI: compile was measured green on 7449df18b939 in the single check-run read at 17:22Z. That check builds the shipped pipeline; the numbers above are tsc -p src/tsconfig.json on a Windows worktree. Different checks, both quoted as measured.

9.6 Verdict: no unit may open

No unit head exists that both satisfies the two invariants and passes the cold proofs, so nothing in this chain is opened, and no per-unit price table is offered as if it were ready:

1. the instrument that honours the published boundary does not parse at its intermediate heads (24 to 54 parse diagnostics);
2. the instrument that parses does not reproduce the PR tree (6 files), cannot honour the 13-unit contract (26 mixed hunks, 4 empty units, mutation-row mapping lost), and puts 1,450 a+d into U1 against the 1,000 hard cap;
3. at the retained first unit head the focused spec is red with 4 failures and the type-check is worse than its own base by 2 error lines, because the boundary separates writeSkillFileAtomically (later unit) from its caller and from the diagnostics state field (line 147).
   The reason this is reported as a blocker instead of a table of numbers: a table whose proof leg is missing is what cost 2414 a round of re-work.
   What would change the verdict, in order of cost: (a) a boundary decision that keeps each method with its callers and its state field, which puts the atomic-write cluster earlier than the published plan assumed and moves reviewer-owned rows between units; (b) a chain builder that can cut inside a hunk without desynchronising the file - git apply refuses the filtered new-file patches, and my line-level applier mishandles end-of-file context; (c) a cold tsc baseline that is green, which the merge base is not locally. This needs the stack owner's call on (a); I am not opening any unit and not pushing anything on this chain.
   Standing disclosures still apply to every number above: the extension mutation run may complete and still record nothing (measured at 6b5956473168: the arm completed in 17m32s and printed Valid 0), so job completion is not a mutation measurement; and no label action was taken here, because the label write upstream is refused (POST of a label already on 1321 returned HTTP 403 at 17:01:33Z).

9.7 Required contexts, cited not claimed

The six required contexts named in section 7 (check-translations, platform-unit-test (ubuntu-latest), platform-unit-test (windows-latest), compile, knip, e2e-mock) are quoted from another lane's artifact: e3-ruleset-codeowners.json, read 2026-11-10T16:28:53.126Z by t12-1291 (ruleset 15494382 Protect Main, require_code_owner_review true, merge queue SQUASH/ALLGREEN). The 18 rows in section 7 are check runs as read at 17:22Z - a different denominator, and the ruleset enumerates 0 check contexts through the field that read exposed. The missing gate item is one code-owner approval, not a green check: the per-unit split is what makes a reviewer willing to approve, not what lets the merge queue accept.

10. Dependency-closure re-cut (recut-e), the installed control, and the boundary question

Whole-hunk is off the table on the measured contract cost, not on taste, so this round cut by the closure rule: a unit owns, together or not at all, a state field, the methods that write it, the call sites that make those methods reachable, the whole-test rows that kill mutants in those methods, and the fixtures those rows consume. Applied to every member, not to the two examples.

10.1 What the rule actually produces on this file pair

• Nodes taken from the string-anchored whole-member segments of src/services/skills/SkillsManager.ts at 7449df18b939: 51 (50 whole members plus top-level statements), 1,167 rows of changed code.
• Reference closure over this.X, top-level names and class-qualified SkillsManager.X references yields one component: 50 members / 1,167 rows (a second listing showed directoryQueues alone, but that is an artifact of my segment list: withDirectoryLock never became a node in it, and 4 of the 5 head-blob lines naming directoryQueues are SkillsManager.directoryQueues inside that method).
• That component spans SkillYamlDumpOptions@51, the four top-level guards, the state fields skills, diagnostics, containers, rootRealPaths, discoveryQueue, isDisposed, constructor@125, initialization@129, sweepStaleSkillTempFiles@165, the discovery path (discoverSkills, runDiscovery, scanSkillsDirectory, scanSkillContainer), the frontmatter/diagnostics path (loadSkillMetadata@412, recordDiagnostic@552, getSkillDiagnostics@682, getSkillsMetadata@686), the publish/rollback path (writeSkillFileAtomically@769, createSkill@844, deleteSkill@939, moveSkill@966, moveDirectory@1074), the containment guards (isSymlink@391, findEscapingRelativeSymlink@1194, assertTargetWithinSkillsRoots@1220), the mode path (updateSkillModes@1241, getAvailableModes@1380) and the watchers plus dispose@1492. Under the rule as written those are one unit: every candidate seam cuts a reference a method needs from a field or method in the other half.
• Whole-test rows bind to it the same way: 18 of the 21 whole-test blocks in SkillsManager.spec.ts name symbols from that component - 4,262 of 4,506 rows; 3 blocks (44 rows) name none. The largest single whole-test block is 1,669 rows (discoverSkills), then 980 (moveSkill), 733 (createSkill), so owning part of those paths means carrying the whole block.

10.2 recut-e U1, and the control that makes its numbers quotable

Withdrawal, stated plainly: the 24-error and 23-error tsc figures in section 9.5 were environment-bound - that worktree had lost its node_modules to git clean -fd during the chain builds and five junctions had been re-made by hand, and turbo there failed with Unable to find package manager binary. In the re-installed worktree both ends are exit 0, so those two numbers must not be used as a type-check verdict.

10.3 Mutation duty

• The sanctioned local preflight is node zdt.mjs mutation preflight --worktree <wt> --base <sha> --head <sha>, which writes .zdt-mutation-preflight.log and .zdt-mutation-preflight.json. Searched for it and found nothing: entries of Zoo-Code/scripts/ matching stryker|muta|zdt - only stryker-diff.mjs (the gate counter, not the runner); repo probes zdt.mjs, scripts/zdt.mjs, src/stryker.jsonc, src/stryker.mjs, stryker.jsonc - none present; names under C:/work/Zoo-Code-fork matching zdt|tool|muta|stryker - none; Get-Command zdt - no source. The preflight runner produced no artifact because it is not installed in this tree; CI at the first pushed unit is the only proof of the mutation gate.
• Substitute measurement, which is stronger than an argument: the three named rows were applied by hand onto the unit head and the focused suite was run each time. Baseline (candidate rows present, no mutant): 134 passed. Row 102 ArrayDeclaration private diagnostics: SkillDiagnostic[] = [] to = [""]: 134 passed, survived. Row 461 OptionalChaining descriptionLine?.line.includes('"') && to descriptionLine.line.includes('"') &&: 134 passed, survived. Row 563 EqualityOperator typeof yamlError.reason === "string" ? yamlError.reason : undefined to !==: 134 passed, survived. The digest's own labels for these rows were 102S, 461S and 563S, so the three survivals reproduce CI three for three.
• Kept separate from the measurement, as reading: row 461 looks equivalent because line 463 of the same condition dereferences descriptionLine.lineNumber without ?.; row 563 changes only a reason value that no assertion in this suite reads; row 102 initialises a field no test asserts empty. That argument belongs with a reasoned Stryker disable review, not with a green run, and no directive was added here.
• Standing caveat unchanged: an extension mutation run may complete and still record nothing (measured at 6b5956473168: the arm completed in 17m32s and printed Valid 0), so job completion is never a measurement.

10.4 The boundary question, with the seam choices and what each costs a reviewer

The rule as written produces exactly one closure-clean unit and it is 1,727 a+d against a 1,000 hard cap, so it cannot open either. The choices, with costs measured on this tree:

Two things I cannot settle alone, and they are why this is a question rather than a proposal: (1) whether the code owner would rather review one unit of 1,727 added+deleted rows that is green at every proof leg, or two units near 2,200 each whose seam deliberately cuts references; (2) whether the 1,000 figure is meant to bind added+deleted rows at all - the CI gate measures 282 changed executable lines and 54 selectors for this delta, both far inside its own caps (500 and 400), so the same change is compliant on the gate's denominator and non-compliant on the reviewability denominator, and that difference decides whether option A may open.
Not claimed: recut-e carries only the two skills blobs. The other 54 files of 1321 (18 locale settings.json, the webview components and screenshot, the e2e and i18n specs) are not assigned into this chain yet, so no unit is proposed for opening, nothing was pushed, and no label was touched. The blocker for 1321 remains a code-owner approval under ruleset 15494382 with require_code_owner_review true; this split exists to make that approval willing, not to let the merge queue accept.

11. Correction to 10.3: the local preflight runner is present, and the preflight is now running

The search sentence in 10.3 was wrong in two ways, and a wrong absence claim is a wrong measurement, so it is corrected rather than quietly refined.

• Zoo-Code/scripts/ entries matching stryker|muta|zdt are three, not one: stryker-diff.mjs (the gate counter), stryker-diff.test.mjs, .scratch-stryker-diff.mjs. None of them is the Stryker runner.
• Get-Command zdt returns no source, and C:/work/Zoo-Code-fork/tools holds only node_modules, package-lock.json and package.json - which is where I stopped looking, and that was the defect. Get-ChildItem -LiteralPath C:\work -Recurse -Depth 2 -Filter zdt.mjs finds it: C:\work\zoo-dev-tools\zdt.mjs. The claim that the runner is not installed is withdrawn.
• Consequence, executed: the preflight the protocol asks for is running on exactly the delta named in the ruling - node zdt.mjs mutation preflight --worktree C:\work\Zoo-Code-fork\wt-r159-price --base 18861b2e357203e6cb4d93930aa2ed122b1eecea --head dddcda7c2dc8 (default --timeout-min 20; the tool writes .zdt-mutation-preflight.log and .zdt-mutation-preflight.json in the worktree). Its output is being captured to .tmp-b1-1404/r203-zdt-preflight.txt and its verdict will be appended into this comment when it lands. If it produces no artifact, the absence will be recorded in those words and CI at the first pushed unit stands as the only proof of the gate.
• What does not change: the three measured survivals. They were produced by applying each mutant by hand onto the unit head and running the focused suite - baseline 134 passed, 102 survived, 461 survived, 563 survived - which reproduces the digest's own 102S, 461S, 563S labels three for three, and does not depend on this runner.

12. The local preflight ran and produced an artifact that records nothing, plus a correction to the parent in 10.2

12.1 Preflight artifact, quoted

zdt mutation preflight --worktree wt-r159-price --base 18861b2e357203e6cb4d93930aa2ed122b1eecea --head dddcda7c2dc8 exited 0 in about a minute and wrote .zdt-mutation-preflight.json (470 B, packages: [], pass: true) and .zdt-mutation-preflight.log (443 B). The log and the generated pr-comment.md say, verbatim:

Mutation-testing 2 package(s) from merge base 18861b2e3572: extension (282 lines), webview (37 lines)
::warning title=Mutation test advisory::extension Stryker preflight could not start: spawnSync <wt>\node_modules\.bin\stryker ENOENT
::warning title=Mutation test advisory::webview related-test discovery could not start: spawnSync <wt>\webview-ui\node_modules\.bin\vitest ENOENT
...
| extension | 282 | 0 | 0 | 0 | 0 | 0 | Advisory incomplete |
| webview   |  37 | 0 | 0 | 0 | 0 | 0 | Advisory incomplete |
The mutation run was incomplete; selected test file names were not recorded.
=> PASS (local gate green)

So: the runner is present and does produce an artifact, and the artifact is the same shape that cost 2414 a round of re-work - blocking: 0, advisories: 2, every count 0, Valid 0, and a green verdict computed over an empty measurement. The changed-line counts it reports (282 and 37) match the CI figures for this delta, so the counting leg is sound; only the measurement leg is missing, and it is missing for a named reason: node_modules/.bin/stryker and webview-ui/node_modules/.bin/vitest are not in this worktree's install. Direct check: <wt>/node_modules/.bin and <wt>/src/node_modules/.bin and <wt>/webview-ui/node_modules/.bin contain none of stryker/vitest under those names [stryker, stryker.CMD, stryker.ps1 | vitest, vitest.CMD, vitest.ps1 | vitest, vitest.CMD, vitest.ps1]. A PASS whose digest holds no package is not evidence that a mutant dies. The three hand-applied mutants in 10.3 - 102, 461, 563, all surviving the focused suite - remain the only kill evidence on this tree, and CI at the first pushed unit remains the only proof of the mutation gate.

12.2 Correction to the parent named in 10.2

What this does and does not change. It does not change the closure result (one component, 50 members, 1,167 rows; 18 of 21 whole-test blocks bound to it) or the seam costs, and it does not change the cold check-types legs, which ran at the commit as checked out and are green at both ends in the installed worktree. It does change one row I printed as a width: the honest width of the component's two files against the merge base is 1900 a+d (src/services/skills/SkillsManager.ts 396+41-, src/services/skills/tests/SkillsManager.spec.ts 1346+117-), and 1,727 is the width of the last step against an unrelated pricing commit that happens to carry the same two blobs' content one step back. I mislabelled the parent, and the number has to carry its parent with it. The verdict stands unchanged: no unit may open, and the two questions in 10.4 - one unit of this width or two units near 2,200 each with a seam that deliberately cuts references, and whether the 1,000 figure binds added+deleted rows or the gate's 282-changed-executable-lines denominator - are still the ones I cannot settle without the code owner. Nothing was pushed, no label was touched, and 1321 still waits on a code-owner approval under ruleset 15494382 with require_code_owner_review true.

13. Two sentences in 12 contradicted the numbers printed next to them

13.1 The binaries are installed; the failure is the Win32 shim shape, not a missing dependency

Section 12.1 says none of the three .bin directories holds stryker or vitest. The lists printed inside that same sentence say otherwise, and the lists are the measurement:

<wt>/node_modules/.bin            -> stryker, stryker.CMD, stryker.ps1
<wt>/src/node_modules/.bin       -> vitest, vitest.CMD, vitest.ps1
<wt>/webview-ui/node_modules/.bin -> vitest, vitest.CMD, vitest.ps1

So spawnSync <wt>\node_modules\.bin\stryker ENOENT is not a missing Stryker: it is the runner spawning the extensionless POSIX shim through spawnSync without a shell, which Windows cannot exec - the pattern the mutation-gate protocol already catalogues as Win32 shim loss. Quoted from that protocol:

assertion only as a last resort, with the reason in a comment. The count must come out
flat or negative.
self-explanatory.
E. Win32 shim loss (Stryker preflight). Signature: zdt mutation preflight
fails within SECONDS (not minutes) with related-test discovery could not the pipeline, .bin\stryker ENOENT`. The gate never actually ran. Cause: the

The consequence for this PR's record is the one 12.1 already stated and is unchanged - the artifact holds Valid 0, Killed 0, Survived 0, No coverage 0 for both packages, blocking: 0, and a => PASS (local gate green) verdict computed over an empty measurement - but the cause is corrected: a local preflight on this machine can pass while measuring nothing because the runner cannot exec its own shim, not because Stryker is absent. A green local preflight is therefore not evidence here, in either direction, and CI at the first pushed unit stays the only proof of the mutation gate. The three hand-applied mutants (102, 461, 563, all surviving the focused suite) remain the only kill evidence produced on this tree.

13.2 The unit tree is not the PR head tree, and the component's width against the merge base is 1,900 a+d

12.2 prints 93faa70e803a vs 7f36f5e756fa and calls them identical in the sentence beside it. They are different. So dddcda7c2dc8 does not reproduce the PR head tree: it carries the merge base plus the two skills blobs at head content on top of 1c262ef1b01a, and the 56-file / 5,776 a+d figure beside it is that lineage's delta from the merge base, not the unit's step. The measurement that matters for the split decision, labelled with its parent:

git diff --numstat 18861b2e3572 7449df18b939 -- SkillsManager.ts SkillsManager.spec.ts
  src/services/skills/SkillsManager.ts                     396+ 41-    = 437
  src/services/skills/__tests__/SkillsManager.spec.ts   1,346+ 117-    = 1,463
  two files together                                     1,900 a+d

So carrying the closure component whole costs 1,900 added+deleted rows against the merge base for these two files - 900 over the 1,000 reviewability figure - not 1,727 (that was the step from a leftover pricing commit) and not 280 (that was the source file alone). The code side is small; 1,463 of the 1,900 is test rows that whole-test granularity refuses to split.

13.3 Standing verdict

Unchanged, and now carried by measurements whose parents and denominators are named: the closure rule produces one component, one closure-clean unit, that unit is 1,900 a+d against the merge base for the two files it owns, parse-clean, cold turbo run check-types green at both ends in a correctly installed worktree, and 134 focused tests green; no local mutation measurement exists because the local runner cannot exec its own shim on this machine; therefore no unit may open, and the two questions in 10.4 belong to the code owner. task-166 is released (completed at rev 3). Nothing was pushed, no label was touched, no changeset or CHANGELOG was edited; the only artifact written on 1321 is this comment.

14. What the shim failure actually is, where its carrier is, and why I did not re-apply it here

13.1's quote block was mis-chopped: its first three lines come from the bullet above the pattern in the mutation-gate protocol, not from the pattern. The pattern's own words, quoted from where they start:

E. Win32 shim loss (Stryker preflight). Signature: zdt mutation preflight
fails within SECONDS (not minutes) with related-test discovery could not the pipeline, .bin\stryker ENOENT. The gate never actually ran. Cause: the win32 shim is an UNCOMMITTED local edit to scripts/stryker-diff.mjs discoverRelatedTests: false; Stryker launched via process.execPath +
node_modules/@stryker-mutator/core/bin/stryker.js; spawn timeout raised to
45 min). It is uncommitted BY DESIGN (never commit it), so a merge, git checkout, or reset of the worktree silently wipes it. Diagnosis in one

That matches this worktree exactly. The shim is not in the committed file: scripts/stryker-diff.mjs at 7449df18b939 versus the local backup C:/work/Zoo-Code-fork/.tmp-zdt-shim-stryker-diff.mjs.bak differs by +770 / -831 lines, and the added side contains the launch through process.execPath plus @stryker-mutator/core/bin/stryker.js and the discoverRelatedTests: false discovery switch:

+#!/usr/bin/env node
+
+import { execFileSync, spawnSync } from "node:child_process"
+import fs from "node:fs"
+import path from "node:path"
+import process from "node:process"
+import { fileURLToPath } from "node:url"
+

Because the shim is an uncommitted local edit, every git checkout and git clean -fd this worktree went through while building the chains wiped it, which is precisely why the preflight failed within about a minute with .bin\stryker ENOENT and related-test discovery could not start, and why the digest printed PASS over an empty measurement. The gate never ran.

Not done here, and why: the protocol's fix is to re-apply the shim and rerun the preflight from scratch, and the carrier is sitting right there (C:/work/Zoo-Code-fork/.tmp-zdt-shim-stryker-diff.mjs.bak, and .tmp-zdt-shim-stryker-diff.test.mjs.bak for the matching test file). Re-applying it is a two-line copy; running the preflight from scratch is the 45-minute spawn the protocol raised the timeout to, which this lane's stop condition - one dependency-closure re-cut plus one correctly installed control, then propose or ask - does not stretch to cover, and a rerun would not change any number already published: it would either confirm the three measured survivals or add survivors, and the two seam questions in 10.4 are the code owner's either way. So the mutation leg stays unmeasured, CI at the first pushed unit stays the only proof of the gate, and the named carrier is left for whoever runs the first real unit.

Everything else in sections 10-13 stands: one closure component of 50 members / 1,167 rows; the component's two files at 1,900 added+deleted rows against the merge base (SkillsManager.ts 396+41-, SkillsManager.spec.ts 1346+117-); parse-clean heads; cold turbo run check-types exit 0 at both the unit and its merge base in a correctly installed worktree (12/12 tasks, 28.645s and 33.825s, 0 error TS); 134 focused tests green; three hand-applied mutants all surviving that suite, matching the digest's own 102S/461S/563S; and no unit may open. task-166 is released. Nothing was pushed, no label was touched, no changeset or CHANGELOG was edited; this comment is the only artifact written on 1321.

15. task-173 measurement: is the 1,463-line spec one uncuttable test row, or a grouping problem?

Answer: a grouping problem. Measured on this machine (Windows) with local git plus my own segment and statement-span engine over git diff --unified=0; no API was used for the measurement. Toolchain: node v22.23.4, git 2.54.0.windows.1. Both price bases named throughout: merge base 18861b2 (main tip at re-anchor time) and the PR head 7449df1.

Gate checks that make the segment list trustworthy, run three times with identical results: all 1463 changed spec lines folded together reproduce the PR head blob byte for byte, and folding none of them reproduces the merge-base blob byte for byte.

Structure of src/services/skills/__tests__/SkillsManager.spec.ts at the PR head: 1463 changed lines sit in a file of 4,505 content lines (base 3,276), organised as 128 test rows inside 14 describe wrappers. The four regions named in the plan are wrappers, not test rows: describe("SkillsManager") spans 164-4505 and contains every other row; discoverSkills 200-1868, createSkill 2234-2966 and skill diagnostics 2968-3205 are the three largest of the remaining 13. Cutting inside a wrapper is allowed by the ruling (a wrapper is not a test row), and every line inside a test row must travel with that row.

Per-row accounting, each changed line counted exactly once, whole rows only (a row owns the head-side additions inside its own head span plus the deletions inside the same-titled base-side span of the same row):

Widest rows after that one: 72 a+d (should allow creating a .roo skill when the duplicate lives in the lower-priority .agents root 2935-2965), 63, 62, 59, 59, 57, 48. The hard cap is 1,000 a+d per unit; the largest indivisible unit of work in this file is 84 a+d, twelve times below it. Question 10.4 is therefore not triggered in the form it was framed: no single uncuttable test row exceeds 1,000.

Indivisible pieces: the 39 hunks partition into 439 atoms when a hunk may only be cut where the accumulated add-side brace delta and the accumulated del-side brace delta are both back at entry depth and the last changed line on each side ends a statement (semicolon, opening brace, closing brace or closing paren). Largest atom 59 a+d. 11 atoms are not independently brace-neutral and must travel with a neighbour atom that restores the balance; that constraint, not the cap, is what shapes the boundaries below.

16. The cap-compliant partition, with each unit's own parent named

Width is git diff --numstat <parent> <head> additions plus deletions for that unit against its own parent commit, the unit before it; the first unit's parent is the merge base 18861b2. These are a+d widths, not the CI axis: the gate counts changed executable lines and valid mutants per package (scripts/stryker-diff.mjs line 11 MAX_CHANGED_LINES = 500, line 12 MAX_MUTANTS = 400 on the blob at 7449df1), and open upstream issue #1923 (mutation-diff charges a stacked unit the whole unmerged chain) means a unit's CI number is not its own width. Per-file add and del splits for every row below are in the ledger. Chain: C:/work/Zoo-Code-fork/wt205, refs/recut-n/*.

Read-out: 30 units, widths summing to 1929 a+d, largest U1 at 554 a+d, 1 unit over the 400 soft cap and 0 over the 1,000 hard cap. The cap part of the ruling is satisfiable today. It is not green yet: 24 of these 30 heads fail the focused spec and 9 fail to parse, because a unit that takes an atom from the middle of the createSkill region takes a closing brace away from a row whose opening brace is still in a later unit. Those failures are section 17. The table is published because the widths and the named parents are the measurement that was asked for, not because this chain is openable.

Two green endpoints measured in the same worktree on the same content, both with SkillsManager.ts identical to its PR head blob and the spec equal to the head blob plus the 37 lines of the two owed assertions:

• refs/recut-o, two units: U1 eff53a332c09 at 491 a+d against merge base 18861b2 (brace -1/-1, unusable: it takes only the deletion side of an atom, which the brace rule forbids), then 0f2ff1a7f758 at 1446 a+d against eff53a332c09, green, Tests 136 passed (136).
• refs/recut-p, five units, chain end 51d423b7ee17, green Tests 136 passed (136), its last three units at 191, 146 and 65 a+d against their own named parents, all three brace-balanced 0/0.

17. Why no unit that carries the manager whole is green yet, measured

This decides whether 1321's first unit can open, and it is not a cap problem. The manager and two spec rows are coupled through mock state declared once for the whole file.

At that same head, running the one failing row by itself gives Tests 1 passed, 95 skipped (96): it passes in isolation and fails in the full run, so it reads mock state left dirty by an earlier row rather than missing any behaviour of the manager. The other row, should create a new global skill, fails in isolation, the opposite failure: it reads harness declarations that live inside other rows' pieces. Counting the changed lines in this file that mention the identifiers that row reads (mock-prefixed names, globalSkillsDir, matter, constants) gives 918 of the 1,346 added lines, so the createSkill harness is shared file-wide; folding on that criterion drove the first unit to 1,436 a+d and left the file with two unclosed braces.

Consequence, stated as a number rather than a mood: the smallest head that carries SkillsManager.ts whole and passes the focused spec is the unit that also carries the remaining spec, measured at 1,446 a+d (0f2ff1a7f758 against eff53a332c09), over the 1,000 hard cap. The cap-compliant partition in section 16 stays under 1,000 everywhere but is red at 24 of its heads for the coupling reason above. Both facts are measured on the same three trees.

18. Recommendation, and the answer to question 10.4

Question 10.4 was framed as one honest over-cap unit with the rationale written in the unit body, versus splitting again, to be opened only if a single uncuttable test row exceeded 1,000. Its trigger did not fire: the largest row is 84 a+d. I do not recommend opening it in that form. What did appear is a variant the framing did not anticipate, an uncuttable green unit at 1,446 a+d, and my recommendation is to remove the cause rather than waive the cap. Order the re-anchor as: (1) a preparatory spec unit that factors the shared createSkill mock harness out of the spec file, the mock declarations and the reset wiring the two invalidated rows read; (2) re-measure, and the 84 a+d row ceiling then partitions the remaining spec into units under the 400 soft cap without cutting inside any row; (3) keep SkillsManager.ts whole in the first unit at 437 a+d against merge base 18861b2 with the rationale in its body, because the file is 1,509 rows and the gate is per package, so cutting inside it buys nothing on the gate axis. If the harness factoring is refused, take the other branch: one honest over-cap unit at 1,446 a+d with this section as its stated rationale, and say in the body that the campaign rule wants a split the file's harness will not currently allow.

Mutant rulings that bind where spec blocks may be placed: mutation-diff row 461 descriptionLine?.line.includes('"') is equivalent (the guard dereferences the same object the surrounding condition already dereferences plainly) and needs an explanation only, no directive; rows 102 and 563 are unasserted and each owes one discriminating assertion, and those assertions belong in the unit that carries the diagnostics block, which is the skill diagnostics wrapper 2968-3205. That is why the 4a and 4b spec-block distribution matters, it is where you get to place a killing assertion rather than argue that one could exist.

19. Mutation leg: evidence, and the Windows runner report that already exists upstream

Three independent runs of the same mutations, at three different unit heads, each with the manager identical to its PR head blob and the spec equal to the head blob plus the 37 owed lines:

Both owed assertions kill exactly one row each and nothing else, the control mutation stays green at every head, and every mutant was reverted before the next run (git status --porcelain empty after each). The two sentences stand as written and they are the honest state of the mutation leg on this machine:

a green local preflight over Valid 0, Killed 0, Survived 0 and No coverage 0 with blocking 0 and advisories 2 is not evidence in either direction on this machine

the mutation leg is unmeasured, a PASS over an empty digest is not evidence in either direction, and CI at the first pushed unit is the only proof of the gate

Search that decided whether to file anything, run with the campaign token and named here with its result: the qualifier repo:Zoo-Code-Org/Zoo-Code is:issue is:open combined separately with stryker (27 results), mutation (50), windows (103), spawn (17), ENOENT (18), stryker-diff (24) and "no tests" (37), plus a plain listing of open issues (26 open issues after excluding pull requests) scanned with the keyword set stryker, mutation, mutant, windows, spawn, ENOENT, .bin, shim, runner, gate, preflight.

All seven searches return the same open report: #2410 Mutation gate spawns the extensionless .bin/stryker shim without a shell, which cannot start on Windows, state open, author easonLiangWorldedtech, opened 2026-11-10, no labels, its body quoting blob 7dfe0006d2d5c7e4d2282f65f68b8ed5f16866bc line 421. On the blob at 7449df1 the same call is line 421, const result = spawnSync(path.join(repoRoot, "node_modules/.bin/stryker"), args, {, and that path is a POSIX shell script of 2,140 bytes beginning #!/bin/sh with .CMD and .ps1 siblings, which is the shape #2410 describes. An open report already exists, so nothing was posted, as instructed. Three reproductions are recorded for when a maintainer asks: mine, a local preflight that died on ENOENT for that shim in seconds and printed the incomplete advisory rather than a score; u7-1918's, a copy of the campaign-tip scripts/stryker-diff.mjs (30,108 B, byte-equal to the instrument task154/tip-stryker-diff.mjs) placed in a scratch worktree as scripts/stryker-diff-winspawn.mjs (30,155 B) with exactly one line changed to spawn node_modules/@stryker-mutator/core/bin/stryker.js through process.execPath, the entry the shim wraps; and the reproduction #2410 itself already credits, lane f0-1528 preflighting unit 2 of the #2226 split. The two failure modes are the two ways this runner fails on Windows: a seconds-long ENOENT death here, and u7-1918's extension package generating 373 mutants and then timing its initial test run after about five minutes inside a 12-minute CI allowance. A tree installed offline with --ignore-scripts and nothing built prints 0 0 0 0 with an incomplete advisory instead of a score, which is the same evidentiary class as a pass over an empty digest, not proof in either direction.

20. What this changes, and what it does not

PR 1321 itself is unchanged by this round and still sits in the human code-owner leg: open, head 7449df1, mergeable true with mergeable_state blocked, 56 files, 5,776 a+d, and the only hard gate being ruleset 15494382 Protect Main (one approving review plus required code-owner review, six required contexts, merge queue). Nothing was pushed and no label was touched this round.

The verdict the campaign was watching for, whether the first unit may now open, does not flip on this measurement. A first unit at 437 a+d (manager whole, spec deferred) is red on two rows; a first unit at 461 or 554 a+d is red on one of them for the reason in section 17; the first unit that is green while carrying the manager whole measures 1,446 a+d, over the 1,000 hard cap. The honest verdict is that no unit of the skills pair may open from this re-anchor until either the shared harness is factored into a preparatory unit or one unit is accepted at 1,446 a+d with this section as its stated rationale, and I recommend the first.

Not run this round, deliberately: the cold pnpm exec turbo run check-types sweep and the eslint --prune-suppressions check over the table above, because no unit in that table is openable and the gate list travels with the unit that eventually opens. src/eslint-suppressions.json is untouched at its head blob and no changeset or CHANGELOG file was edited.

Artifacts: chains refs/recut-n (the table), refs/recut-o and refs/recut-p (the two green endpoints) in C:/work/Zoo-Code-fork/wt205. Superseded refs/recut-j, recut-k, recut-l and recut-m were deleted after their numbers were recorded here, with the recut-k mutant head 18cbcac75c89 kept as a sha. The published chain contract refs/recut1321/U1..T4 is untouched.

Plan of record for the skills split (continuation of comment 6774239651)

This comment is the plan of record for the skills-family split of this pull request. It continues
comment 6774239651, which is the contract comment and stands at 65,418 B of the 65,536 B per-comment
limit with 118 B of headroom; the plan lives here because there is no room to append it there, and
6774239651's split-plan section is superseded by this comment while its contract sections still stand.

Two conventions used below, stated once:

• Every cap figure names the base it was taken against. The mutation-diff job prices with --base =
  the tip of main and --head = a merge candidate, while a local preflight is taken against the merge
  base with the unit's parent. The two readings coincide only when main changed nothing in the counted
  set, so one is never reported as the other.
• A digest comment is reused per pull request and re-keyed to the head it measured, so its comment id
  is not a run identifier; the head marker inside the digest names the run.