fix(mt#4622): Exempt the markdown compile SOURCES from Prettier, not just their outputs - #3381
Merged
Merged
Conversation
…just their outputs
`.prettierignore` has exempted `.claude/skills/` and `.claude/agents/` since
mt#2555, on the stated grounds that "the compiler emits the source markdown
verbatim". That reasoning constrains BOTH sides of the source==output equality
and only the output side was exempted, so lint-staged went on reformatting the
SOURCE — reproducing the same deadlock mt#2555 set out to end, from the other
direction:
Step 1 lint-staged runs `prettier --write` on the staged `.minsky/skills/**`
markdown, rewriting `*italic*` -> `_italic_` and padding table cells
Step 9b runCompileCheck compiles from the NOW-formatted source, compares
against output built from the PRE-format source, and blocks the commit
It fires at most once per artifact — on the first commit that stages its source,
since Prettier is idempotent thereafter — which is why it presented as a defect
that only affected NEWLY-ADDED skills. Reproduced under control in an isolated
workspace; a recompile DOES clear it, so the spec's inherited "no sequence of
compiles clears it" was false for this mechanism.
Fix: exempt the markdown sources, mirroring the mt#2555 block directly above.
Markdown ONLY — `skill.ts`/`agent.ts` are code and stay formatted, and Prettier
does not reformat template-literal contents, so formatting them cannot perturb
an emitted body.
Covers agents too, not just skills: `loadMarkdown` reads `prompt.md` verbatim
and `buildAgentMd` emits it verbatim, so `.minsky/agents/*/prompt.md` is the
same defect class. `.minsky/rules/` needs no entry — `.mdc` has no Prettier
parser AND does not match .lintstagedrc's `*.{json,md}` glob.
The permanent-wedge version of this symptom is a DIFFERENT bug and is not
touched here: `listOutputFiles` keys on the directory name while `compile` keys
on the definition's `name`, so a mismatch yields a stale path no compile can
produce. `claude-agents` guards it; `claude-skills` does not. mt#1118 has owned
that since 2026-04-22 and carries a vendored-skill complication this does not.
The regression test is two halves, both load-bearing: an in-memory half pinning
the COUPLING (reformat the source, nothing else, output goes stale) and a
real-corpus census pinning the FIX, asked of Prettier's own resolver rather than
re-implementing gitignore anchoring. Neither alone protects the invariant.
…e skills/agents half
Success Criterion 3 asks whether rules and agents are affected. Agents are, and
the fix covers them. Rules are NOT — for two independent reasons, and this
asserts both rather than leaving the claim in prose where a later pass could
"helpfully" widen the exemption to cover `.minsky/rules/` (inert at best; if
written as a directory pattern, it would stop formatting real markdown).
Reason 1: Prettier infers no parser for `.mdc`.
Reason 2: lint-staged's glob is `*.{json,md}`, which `.mdc` does not match.
Reason 2 is asserted against the PARSED config's glob keys rather than a
substring scan of the file text, and is paired with a positive assertion that
the markdown glob it reasons about is actually present — so a lint-staged
reconfiguration fails this loudly instead of passing vacuously.
Minsky Reviewer StatusVerdict: APPROVED — no blocking findings Commands
|
![minsky-reviewer[bot]](https://avatars.githubusercontent.com/u/52695?s=60&v=4){kind=small}
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
The PR addresses the Prettier-vs-compile staleness race by ignoring markdown sources and adds a two-part regression test. However, there are two issues that must be fixed before merge: (1) the new test misuses Prettier’s API by passing an array to getFileInfo’s ignorePath option, which expects a single string path — this risks false assertions and future breakage; and (2) Success Criterion 2 is not satisfied — there is no test or script that adds a real on-disk throwaway skill and verifies compile --check passes post-fix. Non-blocking notes: the broad ignore patterns could unintentionally exempt future non-source markdown under .minsky/skills/** and .minsky/agents/**; and Part 2’s test is brittle due to dependence on repo layout and Prettier’s resolution semantics. Once the Prettier API usage is corrected in both places and SC2 is demonstrated with an actual on-disk add-and-check, the change looks solid.
Findings
- [BLOCKING] packages/domain/src/compile/prettier-source-exemption.test.ts:72 — Misuse of Prettier API:
getFileInfocalled withignorePathas an array, which Prettier does not support
prettier.getFileInfo(absPath, { ignorePath: [GITIGNORE, PRETTIERIGNORE] })passes an array forignorePath(seepackages/domain/src/compile/prettier-source-exemption.test.ts, theprettierIgnoreshelper). Prettier's API expectsignorePathto be a single string (path to one ignore file), not an array. Depending on Prettier's argument validation, this can silently fall back to defaults (making the test assert the wrong semantics) or break when versions change. It also undermines the test's stated intent to pin resolution against the repo's root ignore files.
Fix one of:
- Omit
ignorePathentirely and rely on Prettier's documented default of consulting both root.prettierignoreand.gitignore. - Or invoke
getFileInfotwice (once per ignore file) if you must force a specific path, or combine the contents into a temporary ignore file and pass that single path.
There is a second occurrence with the same misuse later in the file (the rules test’s getFileInfo call) — please correct both.
- [NON-BLOCKING] .prettierignore:58 — Ignore patterns may unintentionally exempt future non-source markdown (e.g., README.md) under
.minsky/skills/**or.minsky/agents/**
The new patterns/.minsky/skills/**/*.mdand/.minsky/agents/**/*.mdwill ignore every markdown file anywhere under those trees. Today all.mdthere are compile sources (per the comment), but if a README.md or other authoring doc is later added in those directories, it will silently stop being formatted by Prettier. The comment notes this caveat, but consider strengthening guardrails: e.g., use more targeted patterns (SKILL.md,prompt.md,content.md) if the set is stable, or add a repo policy/test that forbids non-source.mdin those trees to prevent accidental exemptions. - [NON-BLOCKING] packages/domain/src/compile/prettier-source-exemption.test.ts:47 — Test’s Part 2 relies on actual repo layout and Prettier resolution — can be brittle across moves or tooling changes
Part 2 asserts the real.prettierignorecovers the real source population by reading.minsky/**from disk and asking Prettier’s resolver. This is high-signal now, but it couples the test to repo layout and Prettier’s ignore semantics; unrelated reorganizations (e.g., moving source trees or renaming files) will fail this test even when the underlying invariant (sources are ignored) still holds. Consider mitigating brittleness: centralize the source-tree roots (import from a single config), narrow the census to explicit expected files (SKILL.md,prompt.md,content.md), or gate with a helper that resolves roots the same way compile targets do. Keep the data-read edge hack (./) but isolate layout assumptions.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
The cause is identified and named: compile non-idempotency, or a --check comparison that reads something the writer does not emit. |
Met | Cause is recorded and enforced via tests: packages/domain/src/compile/prettier-source-exemption.test.ts Part 1 demonstrates that reformatting only the source produces staleness without a recompile (the SOURCE changed between compile and check), and the fix is implemented in .prettierignore to prevent Prettier from rewriting sources. |
Adding a new skill, then running compile --target claude-skills, leaves compile --check passing — demonstrated by adding a throwaway skill in a test or a script, not by assertion. |
Not Met | No test or script in the diff adds a throwaway skill under .minsky/skills/** and then asserts compile --check passes. The new test exercises an in-memory FS coupling (Part 1) and a Prettier ignore census (Part 2), but it does not create a real on-disk skill and run compile/check over it. Please add a test or script that authors a temporary skill in a temp workspace or fixture directory and proves --check passes post-fix, per the criterion. |
| Whether the same defect affects a new RULE and a new AGENT is checked and stated. | Met | packages/domain/src/compile/prettier-source-exemption.test.ts Part 2 asserts .minsky/rules/*.mdc is unaffected (no Prettier parser and not matched by lint-staged glob) and that agent .minsky/agents/*/prompt.md sources are ignored by Prettier after the change. |
| Staging a skill source whose markdown Prettier would reformat, then committing, does not block on a stale compile output — demonstrated by exercising the format-then-check sequence, not by assertion. | Met | The fix is implemented in .prettierignore (added patterns for .minsky/skills/**/*.md and .minsky/agents/**/*.md), which prevents Prettier from rewriting those sources during lint-staged. The test’s Part 1 shows the pre-fix hazard (format-then-check without recompile yields stale), and Part 2 verifies that Prettier would ignore all markdown sources under the two trees, eliminating the sequence. Together they demonstrate that the commit path would not block on this class of change. |
| The committed OUTPUT matches a compile of the committed SOURCE. The check reads the working tree while the commit ships the index, so a regenerate that is not re-staged still ships drift; state explicitly whether the chosen fix closes this or leaves it open. | Met | The PR description’s “SC5” section explicitly states the index-vs-working-tree gap remains and that this fix removes the cause (Prettier rewriting sources) but does not close the divergence’s ability to exist. No code changes attempt to alter check semantics. This satisfies the “state explicitly whether” portion of the criterion. |
Documentation impact
- no-update-needed — Change is configuration and tests only: adds Prettier ignore patterns for markdown sources and introduces a regression test. No user-facing CLI, API, or compile semantics changed (targets and behavior unchanged). Searched for docs covering Prettier ignore policy; no dedicated doc found. No invalidated statements detected in
CONTRIBUTING.mdor README about formatting source markdown under.minsky/skills/**or.minsky/agents/**.
…rd the retirement condition
Reviewer non-blocking, and it was right: `**/*.md` exempted markdown nobody has
written yet. A README added under `.minsky/skills/` or `.minsky/agents/` would
have silently stopped being formatted — no error, no failing test, since the
census only asserted that sources ARE ignored and never that non-sources are
not. Patterns are now the exact filenames the targets read verbatim:
.minsky/skills/**/SKILL.md
.minsky/skills/**/content.md
.minsky/agents/**/prompt.md
Narrowing shifts the failure mode from silent to loud, so the test grew the
other direction of the sync requirement:
- `no unrecognised markdown ...` fails and NAMES a new source filename that was
added to the trees but not to the pattern list — the case the narrow pattern
would otherwise let regress.
- `a NON-source .md ... would still be formatted` asserts the reviewer's case
directly, against hypothetical paths (`getFileInfo` resolves patterns without
touching disk), with a control asserting the sibling SOURCE at the same
non-existent depth IS exempt — without which a typo'd root would make the
whole test vacuous.
Deliberately NOT reusing `SKILL_SOURCE_FILENAMES`: that constant answers "does
the repo own this skill" for the budget cap and is `["SKILL.md", "skill.ts"]` —
includes TypeScript, which must stay formatted, and omits `content.md`, which
must not. Same shape, different question.
Also records what this exemption is: INTERIM, retiring under mt#4513 (the
principal's ask#9976 decision to stop committing compiled trees). With no
committed output there is no staleness check, at which point this stops being
load-bearing and merely leaves 46 files unformatted. Escalation date noted at
the patterns per work-completion.mdc §Temporary mechanism budget.
The BLOCKING finding — "getFileInfo called with ignorePath as an array, which
Prettier does not support" — is a false positive, and NOT changed. prettier
3.5.3 declares `ignorePath?: string | URL | (string | URL)[]` at
node_modules/prettier/index.d.ts:802. Discriminating probe: `[GITIGNORE]` alone
returns ignored:false for an exempted source, `[GITIGNORE, PRETTIERIGNORE]`
returns true — if the array form were being dropped, both would be false. Both
the citation and the probe are now in the helper's docblock so a re-review does
not have to re-derive it.
Negative control re-run against the NARROWED patterns: full revert of the block
fails 2 tests (the census, and the new test's own control), 8 pass; restored,
10 pass.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
The R2 verification confirms the prior BLOCKING finding is addressed. The ignore patterns were narrowed to the exact source filenames (SKILL.md, content.md, prompt.md) in .prettierignore, eliminating the risk flagged in R1 while preserving the intended exemption. The added test prettier-source-exemption.test.ts robustly covers both the coupling (source reformat induces staleness) and the fix (Prettier’s resolver reports sources ignored; non-sources unaffected). Rules remain unaffected for two independent reasons and are pinned in tests. No new defects or regressions are evident; no public surface changed. Documentation updates are not required. Verdict: APPROVE.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
The cause is identified and named: compile non-idempotency, or a --check comparison that reads something the writer does not emit. |
Met | packages/domain/src/compile/prettier-source-exemption.test.ts:1-44 — test header and comments explicitly name the cause (Prettier reformats SOURCE between compile and check); Part 1 tests at lines ~120-186 reproduce the coupling and show a recompile clears it. |
Adding a new skill, then running compile --target claude-skills, leaves compile --check passing — demonstrated by adding a throwaway skill in a test or a script, not by assertion. |
Met | packages/domain/src/compile/prettier-source-exemption.test.ts:146-184 — in-memory fs creates a new skill source, compiles, and asserts checkStaleness returns stale: false (control) and remains non-stale on recompile after formatting; .prettierignore change at .prettierignore:50-92 ensures real-world --check pass by exempting sources. |
| Whether the same defect affects a new RULE and a new AGENT is checked and stated. | Met | packages/domain/src/compile/prettier-source-exemption.test.ts:232-309 — Part 2 tests assert agents are covered (sources ignored) and rules are unaffected for two independent reasons: Prettier infers no parser for .mdc and lint-staged does not target .mdc (see reads of .lintstagedrc.json). .prettierignore adds agents’ prompt.md explicitly at .prettierignore:88-92. |
| Staging a skill source whose markdown Prettier would reformat, then committing, does not block on a stale compile output — demonstrated by exercising the format-then-check sequence, not by assertion. | Met | packages/domain/src/compile/prettier-source-exemption.test.ts:166-184 — Part 1 simulates lint-staged rewriting the source without recompile and shows stale: true, then demonstrates stale: false after recompile; combined with source exemptions in .prettierignore:74-92, the pre-commit formatter becomes a no-op for sources, preventing the block. |
| The committed OUTPUT matches a compile of the committed SOURCE. The check reads the working tree while the commit ships the index, so a regenerate that is not re-staged still ships drift; state explicitly whether the chosen fix closes this or leaves it open. | Met | PR description and test header in packages/domain/src/compile/prettier-source-exemption.test.ts:1-44 state explicitly that the fix does NOT close the index-vs-worktree gap (SC5) — it removes the cause (Prettier rewriting sources) but does not change --check semantics. The .prettierignore comments at .prettierignore:54-72 reiterate this is an interim exemption. |
Documentation impact
- no-update-needed — Behavioral change is limited to Prettier ignore patterns and added tests. No public API, CLI contract, or user-facing docs are changed. The explanatory comments in
.prettierignoredocument the rationale inline; no docs indocs/or CONTRIBUTING appear to describe Prettier source-ignore patterns specifically.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
R3 verification: The prior BLOCKING concern (Prettier getFileInfo ignorePath usage) is addressed; the test now documents and confirms array support. The fix narrows .prettierignore to exact markdown source filenames under .minsky/skills/** and .minsky/agents/**, matching the spec and preventing lint-staged from reformatting sources. The new test prettier-source-exemption.test.ts robustly covers both the source→output staleness coupling (in-memory fs) and the real ignore semantics (via Prettier’s resolver), including controls for non-sources and rules. I found no new defects introduced by this change; compile targets remain untouched functionally and no public surface changed. Documentation updates are not required. Verdict: APPROVE.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
The cause is identified and named: compile non-idempotency, or a --check comparison that reads something the writer does not emit. |
Met | packages/domain/src/compile/prettier-source-exemption.test.ts:22-53 documents and tests the source-reformat → stale-output coupling; Part 1 tests at lines ~121-189 reproduce that a formatted source without recompile yields stale, and a recompile clears it. |
Adding a new skill, then running compile --target claude-skills, leaves compile --check passing — demonstrated by adding a throwaway skill in a test or a script, not by assertion. |
Met | packages/domain/src/compile/prettier-source-exemption.test.ts:151-170 — control test compiles from source and asserts subsequent check is not stale; the Prettier exemption in .prettierignore prevents Step 1 from reformatting sources, preserving the pass. |
| Whether the same defect affects a new RULE and a new AGENT is checked and stated. | Met | packages/domain/src/compile/prettier-source-exemption.test.ts:245-317 — tests assert agents' prompt.md sources are ignored (covered) and rule sources .mdc are unaffected for two independent reasons (no parser; lint-staged glob excludes .mdc). .prettierignore adds .minsky/agents/**/prompt.md (lines 79-87). |
| Staging a skill source whose markdown Prettier would reformat, then committing, does not block on a stale compile output — demonstrated by exercising the format-then-check sequence, not by assertion. | Met | packages/domain/src/compile/prettier-source-exemption.test.ts:171-188 — Part 1 simulates lint-staged’s reformat (formatted source without recompile) and shows it would create staleness; combined with .prettierignore additions at lines 74-87 ensures Prettier ignores these sources so the commit path is unblocked. |
| The committed OUTPUT matches a compile of the committed SOURCE. The check reads the working tree while the commit ships the index, so a regenerate that is not re-staged still ships drift; state explicitly whether the chosen fix closes this or leaves it open. | Met | .prettierignore:58-100 comment block explicitly records that the exemption is INTERIM and does not close the index-vs-worktree gap; the test suite focuses on preventing source reformat drift. The spec’s requested statement is present in the PR description and encoded in comments. |
Documentation impact
- no-update-needed — Internal behavior change only: expands .prettierignore to cover markdown sources and adds tests. No user-facing commands, flags, or compile semantics changed. Checked CONTRIBUTING.md and related compile target docs indirectly via inline comments; no documented behavior is invalidated.
edobry
added a commit
that referenced
this pull request
Aug 30, 2026
…on, distinct from /handoff
## Summary
`/handoff` writes a durable record for the **next agent**. Nothing addressed the principal who is
**still here** and has lost the thread — stepped away, context-switched, or reading a scrollback
they can no longer see the shape of.
Asking `/handoff` to cover that produces a technically-correct summary answering the least useful
question. It opens with *"shipped mt#X, mt#Y; queue is mt#Z"* — which the principal can already read
off a task list. **What they cannot reconstruct is the chain from what they originally asked for to
whatever is on the screen now**, especially when the work drifted, which is exactly when they ask.
So the skill mandates four parts in order — what / **why** / current state / next — and requires the
WHY to name the originating request and say plainly if the work departed from it.
| | `/handoff` | `/catch-up` |
| -------- | ---------------------------------------- | ---------------------------------- |
| Audience | the NEXT agent, or future-you | the principal, now |
| Timing | conversation end; compaction approaching | any time the thread is lost |
| Output | a durable memory record + a chat pointer | chat prose only, nothing persisted |
| Assumes | the reader shares the motivation | the reader has LOST the motivation |
## Key changes
- **`.minsky/skills/catch-up/SKILL.md`** + its compiled output. Two disciplines it inherits rather
than invents:
- **Re-derive statuses via `refs_status`; never recall them.** Not diligence theater — in the
conversation that produced this skill, mt#4541 was filed mid-session and went TODO → DONE under
another actor before the catch-up was written. A recalled status would have reported finished
work as pending.
- **Lead with the correction.** If something the principal was told earlier turned out wrong, the
catch-up is where it gets fixed, near the top, not buried.
- **Explicitly NOT auto-triggered.** Unlike `/handoff`, nothing about conversation shape reveals
that the principal has lost the thread — only they know. And it persists nothing; `/handoff` owns
the durable path (mt#2911). Doing both is called out as the anti-pattern.
- **`LISTING_TOTAL_TARGET_CHARS` 18_000 → 18_500**, on an explicit principal decision. The old value
left 47 chars of headroom at 60 skills, so the next skill of any kind overflowed it. Three options
were put to him — truncate the five vendored over-cap descriptions at compile time, trim an owned
sibling, or raise the cap — and he chose to raise it. The docblock records the rejected options and
tells the next reader to **read the headroom, not the number**: 18_500 buys roughly one more skill,
so raising it again is not the reflex to reach for.
## Testing
Execution evidence:
**SC1 — skill exists, `user-invocable: true`, description names real trigger phrasings.** Verified
against the COMPILED output, not the source:
```
$ grep -n 'user-invocable' .claude/skills/catch-up/SKILL.md
10:user-invocable: true
```
Its `description` carries "catch me up", "what were we doing", "where are we", "remind me why", and
"a summary request while work is in flight".
**SC2 / SC3 / SC4 — skill body content.** Prose criteria, satisfied by the shipped file and visible
in this diff: the four-part answer with WHY named as the part `/handoff` omits (§The four parts);
the re-derive-don't-recall requirement with mt#4541 as its worked example (§Re-derive statuses);
and the explicit boundary against `/handoff`, `/retrospective` and `/incident-memo` (§Why this is
not `/handoff`).
**SC5 — compiled output regenerated and committed, verified rather than trusted.**
```
$ git status --porcelain
# empty — nothing left uncommitted
$ head -3 .claude/skills/catch-up/SKILL.md
---
# Generated by minsky compile. Do not edit directly.
name: catch-up
$ git show --stat --oneline HEAD
69d117d feat(mt#4611): Add /catch-up — re-orient the principal mid-conversation
.claude/skills/catch-up/SKILL.md | 90 ++++++++++++++
.minsky/skills/catch-up/SKILL.md | 89 ++++++++++++
packages/domain/src/compile/skill-listing-budget.ts | 20 ++++-
```
**One deviation from SC5's stated method, stated rather than papered over.** The criterion asks for
the per-target `Target "<name>": N file(s) written` line. This CLI invocation does not emit it — it
prints a JSON result object plus two `[compile]` report lines, and the human-readable per-target line
belongs to a different output mode. Rather than fabricate it, the stronger available check is used:
```
$ bun run src/cli.ts compile --check --target claude-skills | jq -r '{stale, staleFile}'
{ "stale": false, "staleFile": null }
```
`--check` compares output CONTENT against a fresh compile of the source. A write count only asserts
that something happened; this asserts the committed output equals what the committed source
produces, which is what SC5 is actually for.
**The budget, measured after the change** — the criterion the cap raise exists for:
```
[compile] skill listing: 18263 chars across 61 skills (target <= 18500)
[compile] vendored over cap (upstream-owned, not enforced): impeccable (895),
tailwind-v4-shadcn (484), motion-framer (472), seo-skill (470), plan-design-review (425)
```
18,263 is **over the old 18,000 cap**, so this change is load-bearing rather than cosmetic: without
it the compile reports `EXCEEDS SKILL LISTING BUDGET`, and an over-budget listing silently drops
descriptions — which makes a skill listed name-only unroutable. The second line is the declined
alternative, still on the table: those five vendored descriptions total ~2,746 chars.
**Nothing pinned the old constant:**
```
$ bun scripts/run-related-tests.ts packages/domain/src/compile/skill-listing-budget.ts
46 pass 0 fail 106 expect() calls (3 related test files)
```
**This commit was blocked for five days, and the unblocking is itself verified.** mt#4622 (merged
2026-08-26, PR #3381) fixed the cause: lint-staged reformatted the staged `SKILL.md` **source** at
pre-commit step 1, and the staleness check at step 9b then rejected output built from the pre-format
source. Confirmed here after `session_update` brought the fix into this 5-day-old session:
```
$ grep -c 'minsky/skills/\*\*/SKILL.md\|minsky/agents/\*\*/prompt.md' .prettierignore
2 # the mt#4622 exemption is present
```
and the commit that had failed repeatedly now passes pre-commit and pushes clean. That is end-to-end
evidence for mt#4622's fix as much as for this task.
No new test file is added — this ships a skill definition and a constant, and the constant's effect
is a property of the live 61-skill corpus rather than of a fixture, so the compile output above is
the assertion.
## Deploy verification
Deploy surface — verified by running the predicate rather than assuming:
```
$ bun -e 'import { isDeploySurfaceFile } from "./packages/domain/src/deployment/deploy-surface.ts";
for (const f of process.argv.slice(1)) console.log(isDeploySurfaceFile(f), f);' \
.minsky/skills/catch-up/SKILL.md .claude/skills/catch-up/SKILL.md \
packages/domain/src/compile/skill-listing-budget.ts
false .minsky/skills/catch-up/SKILL.md
false .claude/skills/catch-up/SKILL.md
true packages/domain/src/compile/skill-listing-budget.ts
```
The constant lives under `packages/domain/**`, so this IS deploy surface and no `[no-deploy-impact]`
tag is claimed anywhere in this PR or its commit message. Post-merge deploy verification runs per
§10 against the merge timestamp; it is not waived.
## Live verification
The compile runs above are the live exercise — the real CLI against the real 61-skill corpus in the
session workspace. The skill's own behaviour is prose instruction to an agent, so there is no runtime
surface to probe beyond its presence in the compiled listing, which the 61-skill count confirms.
Co-Authored-By: minsky-ai[bot] <minsky-ai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adding a new skill could not be committed:
compile --checkcalled its output stale immediatelyafter compiling it. The cause is not in the compiler.
.prettierignorehas exempted the compiled OUTPUT (.claude/skills/,.claude/agents/) sincemt#2555, on the stated grounds that "the compiler emits the source markdown verbatim." That
reasoning constrains both sides of the source==output equality and only the output side was
exempted — so lint-staged went on reformatting the SOURCE, reproducing the same deadlock mt#2555
set out to end, from the other direction:
prettier --writeon the staged.minsky/skills/**markdown(
.lintstagedrc.jsonmatches*.{json,md}), rewriting*italic*→_italic_and padding tablecells.
runCompileCheckcompiles from the now-formatted source, compares against outputbuilt from the pre-format source, finds it stale, and blocks the commit.
It fires at most once per artifact — on the first commit that stages its source, since Prettier is
idempotent thereafter. That is why it presented as a defect affecting only newly-added skills.
The spec's inherited claim that "no sequence of compiles clears it" is false for this mechanism —
a recompile does clear it, measured. The permanent-wedge version of the same symptom is a different
bug:
listOutputFileskeys on the directory name whilecompilekeys on the definition'sname, soa mismatch yields a stale path no compile can produce.
claude-agentsguards that explicitly(
claude-agents.ts:218, with a comment naming this exact symptom);claude-skillsdoes not.mt#1118 has owned it since 2026-04-22 and carries a vendored-skill complication this PR does not
— so it is scoped out, not fixed here.
Key changes
.prettierignore— exempt the markdown sources by exact filename, mirroring the mt#2555block directly above:
skill.ts/agent.tsare code and stay formatted; Prettier does notreformat template-literal contents, so formatting them cannot perturb an emitted body.
**/*.md— see R1 below. A directory-wide glob also exempts markdown nobodyhas written yet.
loadMarkdownreadsprompt.mdverbatim andbuildAgentMdemits it verbatim — the same defect class. This is the class-not-instance scandone at authoring time rather than after a reviewer points at one site.
.minsky/rules/needs no entry —.mdchas no Prettier parser and does not matchlint-staged's glob. Two independent reasons, both asserted.
ask#9976 decision to stop committing compiled trees). With no committed output there is no
staleness check, at which point this exemption stops being load-bearing and merely leaves 46
files unformatted. Escalation date noted inline per
work-completion.mdc §Temporary mechanism budget.packages/domain/src/compile/prettier-source-exemption.test.ts— the regression test, in twohalves that are deliberately different in kind:
compiled output goes stale. If someone later decides the exemption is unnecessary, this is the
test that says why it is not.
getFileInfo)rather than by re-implementing gitignore anchoring — semantics that file's own mt#3880 comment
records as easy to misread.
would still pass if the compiler stopped emitting verbatim.
One deliberate oddity, flagged so it is not "tidied" away. The path constants are written
"./.prettierignore"and"./.lintstagedrc.json". The related-test selector's data-read edge(mt#4224) is extracted by a regex requiring a
/and a dotted extension; a bare".prettierignore"has neither. Measured — before the./,bun scripts/run-related-tests.ts .prettierignoreselected zero tests, so CI would have been the first signal for a change to thevery file this PR is about (the mt#4367 shape). With it, the selector names this file.
Review round 1
BLOCKING — "
getFileInfocalled withignorePathas an array, which Prettier does notsupport." Verified false positive; not changed. prettier 3.5.3 declares
ignorePath?: string | URL | (string | URL)[]atnode_modules/prettier/index.d.ts:802.Discriminating probe, since the type alone is not proof the runtime honours it:
If the array form were dropped, the first two would both be false. The citation and the probe are
now in the helper's docblock so a re-review need not re-derive them.
NON-BLOCKING — patterns may exempt future non-source markdown. Correct, and fixed. The original
**/*.mdwould have silently stopped formatting a README added under either tree — no error, nofailing test, because the census only asserted that sources ARE ignored and never that non-sources
are not. Patterns narrowed to exact filenames, and the test grew both directions of the
resulting sync requirement: one test fails and NAMES a new source filename added to the trees but
not to the pattern list; another asserts a hypothetical
README.md/NOTES.mdunder those trees isstill formatted, with a control asserting the sibling
SKILL.mdat the same non-existent depth ISexempt (without which a typo'd root would make the whole test vacuous).
NON-BLOCKING — Part 2 relies on real repo layout, brittle across moves. Acknowledged and kept
by design. A census over the real tree is the only thing that can answer "does the real
.prettierignorecover the real sources"; an injected fs would assert something about the mock.This follows the existing precedent at
packages/domain/src/persistence/capability-guard-idiom.test.ts,which carries the same file-level
no-real-fs-in-testsexemption for the same reason. Thebrittleness is bounded: a layout move fails the
source trees are actually populatedguard loudlyrather than passing over zero files.
Testing
Execution evidence:
SC1 — cause identified and named. Neither of the spec's two candidates. Compile IS idempotent for
a fixed source and
--checkcompares only what the writer emits; the SOURCE changed underneath theoutput, between the compile and the check, from inside pre-commit itself. Reproduced under control in
an isolated scratch workspace (no repo or session mutated), driving the target and
checkStalenessdirectly so the CLI and MCP surfaces could not confound it:
SC2 / SC4 / AT1 / AT2 — live, against the real toolchain. A throwaway skill with
Prettier-divergent markdown, run through the same commands pre-commit runs, then deleted:
Re-run after R1's narrowing, against a REAL committed source rather than a throwaway:
The verdict is read as a field, not by grepping the output:
--checkdumps every compiled skillbody, and several of those bodies contain the literal string
is STALEin their prose. A regex overthat output reports a stale match on a passing check — the mt#4121 filter trap, hit and corrected
during this work.
SC3 / AT3 — rules and agents, measured then pinned. Agents affected and covered by the fix; rules
not affected, for two independent reasons (no Prettier parser for
.mdc, and lint-staged's glob doesnot match it). Via
prettier --file-infoafter the change:SC5 — stated, NOT closed, and no follow-up task is owed. The criterion asks the implementation to
say whether the fix closes the index-vs-worktree gap. It does not —
--checkreads the workingtree while the commit ships the index, so an operator who regenerates without re-staging still
commits drift. The planning pass said a follow-up was owed; that was wrong, and the correction is
the more useful finding. mt#4513 (principal decision, ask#9976) stops committing the compiled trees
at all, so with no committed output there is no index copy to diverge — the gap dissolves rather
than needing a fix. mt#4513's own SC6 already retires mt#2977 and mt#3134 on exactly this reasoning,
and
runCompileCheckis in the same class. Filing a regenerate-and-re-stage task would have built amechanism a decided direction removes.
Unit tests:
That last run is itself the check on the data-read edge described above — it returned "no related
test files" before the
./prefix.Negative control — the
.prettierignoreexemption, re-run against the NARROWED patterns:Reverted the entire added block (all three patterns and the comment), not just one line — per
mt#4512, a partial revert leaves a state that is neither pre-fix nor post-fix and can pass for the
wrong reason:
Two tests flip, and both assert the fix — the census, and the new test's own control assertion
(the sibling
SKILL.mdis no longer exempt once the patterns are gone). Part 1 keeps passing underthe revert by design: it pins the coupling, which the revert does not change.
What this control does and does not buy. It proves the census can fail and is observing the real
.prettierignore. It does not prove the pattern set is right for a source layout other than today's— which is precisely why R1's narrowing added the
no unrecognised markdowntest, so a NEW sourcefilename fails loudly instead of silently regressing this defect.
validate_typecheck: 0 errors across 8 projects (session workspace;infra/skipped with itsdocumented reason, covered by CI).
validate_lint: 0 errors / 0 warnings across 4092 files.format:checkexit 0.Deploy verification
This PR is deploy surface — verified by running the predicate rather than assuming, which is what
the guidance asks for and which my own intuition would have gotten wrong here:
A test file under
packages/domain/**is application source to the predicate, so no[no-deploy-impact]tag is claimed anywhere in this PR or its commit messages. Post-merge deployverification will run per §10 against the merge timestamp; it is not waived.
Live verification
The SC2/SC4 probes above are the live exercise — real Prettier, the real CLI compile, and the
real
--check, run in the session workspace against the real skill corpus. No external-systemintegration is added, so there is no credentialed surface to exercise.
One thing surfaced by it, worth knowing independently of this PR: the skill-listing budget is at
17953 / 18000 across 60 skills. A single throwaway probe skill pushed it to 18007 and the compile
printed
EXCEEDS SKILL LISTING BUDGET. mt#3803 owns that; mt#4611's raise to 18500 is uncommittedwork sitting in another session. Anyone adding a skill before either lands will trip it.