Repository navigation
refactor(skills)!: remove OpenSpec workflow and factory skills - #93
Conversation
The OpenSpec change store, its orchestration/proposal/review/cleanup skills, local .pi skill copies, semantic suites, and their gate tests are gone. pr-review-repair remains, without the OpenSpec finalization contract. - drop openspec/ changes, archive, specs, and config - drop openspec-review, openspec-propose-issue, orc-*, and cleanup-merged-issues skills - drop tests/semantic, the test:semantic script, and three gate suites - retire ADR-0001 with the semantic evidence it governed - update AGENTS.md, README, GLOSSARY, and issue-tracker docs
pr-review-repair is the only packaged skill and stays model-invocable, so humanOnlySkills had no members left. Assert the invocability directly instead of comparing against an empty set.
The gate now covers the single surviving skill, so the "factory" vocabulary was a dead leading word: rename the file, the packagedSkills map, the notice helper, the temp dir, and the hermetic PI_FACTORY_SKILLS_OUTPUT variable to PI_SKILLS_OUTPUT (test and nix check both sides).
- README: collapse the scattered toolkit headings into prerequisites, a round boundary, and one set of caveats; drop the one-skill "Applies to" table column and the duplicated authorisation/assurance statements; fold the acpx and OpenSpec skill retirements into one "Retired tooling" section; replace the stale `^0.99.1` codex-alias peer claim with a pointer to Requirements. - AGENTS.md: document that a published skill needs an entry in the skills-distribution gate's packagedSkills map, and correct the notify-me command list (`/notify-me-cancel` is a separate command, not a reserved argument). - skills/pr-review-repair: drop the leftover "change/store context" hand-off vocabulary from the removed OpenSpec procedures.
The first page carried ~270 lines of per-extension contracts (notify-me alone was 177). Keep README.md as the index, install path, and dev setup, and let each component document itself beside its code. - extensions/<name>/README.md now holds each extension's behavior, configuration, privacy, and retry contracts; links are rewritten to resolve from the new location - README keeps a one-line-per-extension table as the only pointer - Skill toolkit compressed to prerequisites + link; the round-boundary and budgeting text stays in SKILL.md where it already lives - Retired tooling history moved to docs/retired-tooling.md - tests/gates/readme-index.test.mjs guards the split: every extension owns a README, the index links all of them, and no relative documentation link dangles - AGENTS.md: document the per-extension README convention, repoint the notify-me documentation reference, and add the missing ollama-usage Repo Layout entry
Review complete — limited coverage, no repair-worthy findingsFresh Standards and Spec reviews covered the full PR diff at Report-only highlights:
Validation: fresh rebuilds of Scope: limited coverage — no tracked issue/spec; the PR body was used as stated intent. The downstream Reviewed: Full review reports, judge ledger, and audit detailsStandardsComplete original report: Standards axis report —
|
| ID | Provenance | Evidence | Impact | Correction |
|---|---|---|---|---|
| F-001 | Standards (a3) | tests/gates/skills-distribution.test.mjs:641 !packedPaths.has("skills/FACTORY-PROVENANCE.md") |
Vacuous: the file never existed in this repo (git log --all -- skills/FACTORY-PROVENANCE.md empty; absent at base 7a8166a). Not introduced here — carried through the rename. No runtime consequence. |
Drop the dead assertion when the file is next touched. |
| F-004 | Standards (a4) | readme-index.test.mjs:16,25-34 vs skills-distribution.test.mjs:63-76 |
Two divergent markdown-link extractors; grep shows 0 broken relative links across all 21 tracked .md files (judge scan). Maintenance smell only. |
Optional shared helper; no behavior change. |
| F-005 | Standards (a5) | GLOSSARY.md:31-33 (_Avoid_: Status bar) vs README.md:28-29, extensions/{permission,ollama-usage,opencode-usage}/README.md |
Wording violation of docs/agents/domain.md:16; but the phrase moved verbatim out of the old README, so nothing was newly invented. Cosmetic. |
Reword to "status slot" on next edit. |
| F-006 | Spec (a1) | PR body "399 pass, 1 skip" / "hermetic (400/400, 0 skipped)" vs measured npm test = 403 tests, 402 pass, 1 skip; hermetic derivation passes with the skip tier enabled |
Stale by exactly the 3 tests e3f55d4 added (readme-index.test.mjs). The substance ("all pass, 0 skipped hermetically") holds; only counts are wrong. Misleading evidence, no delivery risk. |
Correct the numbers in the PR body or in the round summary; do not repair code. |
| F-007 | Spec (a2) | GLOSSARY.md dropped **Escalation**; skills/pr-review-repair/SKILL.md:9,33,38,50 and references/pseudocode.md:16-77 use escalation as a load-bearing outcome |
One surviving concept has no canonical definition. GLOSSARY.md:3 explicitly narrows scope to "test suites and usage-reporting extensions", and the skill defines escalation operationally, so this is a documentation gap, not a contract breach. |
Optional one-line glossary re-entry if the author wants skill vocabulary centralized. |
| F-008 | Judge | readme-index.test.mjs:16 MARKDOWN_LINK = /\[[^\]]*\]\(([^)\s]+)\)/g; coverage limited to README.md, extensions/*/README.md, docs/retired-tooling.md (:14) |
Gate silently ignores links with titles and reference-style links, and never checks AGENTS.md/docs/agents/*. No dangling link exists today (verified), so the gate is weak, not wrong. |
Harden later; not a defect in this PR's claims. |
| F-009 | Judge (adversarial) | Deleted openspec/specs/{permission-yolo,notify-me,testing-harness}/spec.md |
Requirements docs for surviving capabilities now live only in AGENTS.md, the extension READMEs, and tests. Deleting the store is the explicit PR intent (PR body), so this is an accepted information loss, not a deviation. |
None within this PR; flag for the author's future spec source. |
Refuted
- F-002 (Standards a2,
externalSkills) — refuted.tests/gates/skills-distribution.test.mjs:38-42is a deny list of default skill names that must not be loaded; the same names are seeded as fake defaults at:663-664(seedDefaultSkill). The list's purpose is to proveincludeDefaults: falseisolation, not to document externals, so absence of prose aboutcode-review-project/-agentsis not a defect. - F-003 (Standards a3, domain docs) — refuted as stated.
docs/adr/still exists with0002-opencode-usage-reads-undocumented-inference-meters.md, sodocs/agents/domain.md:8is not dangling.cleanup-merged-issuesandorc-issue-to-prgenuinely routed the OpenSpec pipeline, so the grouping indocs/retired-tooling.md:6is defensible, and ADR-0001's governing feature (tests/semantic/) is removed with it — deletion, not silent supersession. - "Removed gates lost non-OpenSpec coverage" — refuted.
cleanup-merged-issues.test.mjs(305 lines) and bothopenspec-implement*gates asserted only removed skill/spec text plus the removedtests/semanticmaterializer's storage admission. No surviving contract depended on them. - "Renamed gate stopped enforcing its claims" — refuted.
packagedSkillsmatches the shipped tree exactly (git ls-files skills=LICENSE+pr-review-repair/*), reference presence, installed-link portability,npm packcontents, MIT notice, and isolated discovery all still assert (:560-714); thehumanOnlySkillsremoval is behavior-preserving because the set was already empty and onlypr-review-repairremains (4d2ada4). - "
pr-review-repairbroke its own contract" — refuted. The four edited files remove only OpenSpec/archival clauses;git grep -i archiv -- skillsreturns nothing,prepare.md/pseudocode.md/SKILL.mdstay mutually consistent (issue/spec "when one exists"), and the report-contract examples still match the contract text (report-contracts.mdtests pass).
Ledger (all findings)
F-001 Standards · report-only · open · skills-distribution.test.mjs:641. F-004 Standards · report-only · open. F-005 Standards · report-only · open. F-006 Spec · report-only · open (fix in PR prose/summary, not code). F-007 Spec · report-only · open. F-008 Judge · report-only · open. F-009 Judge · report-only · accepted-by-design. F-002, F-003 · refuted, resolution evidence above at e3f55d4.
Removal completeness (verified, no finding)
No live reference remains to openspec/, tests/semantic, test:semantic, docs/adr/0001, PI_FACTORY_SKILLS_OUTPUT, or the seven removed skills outside deliberate mentions (README.md:98, docs/retired-tooling.md:5-6). nix/modules/pi-package.nix:136 and the gate agree on PI_SKILLS_OUTPUT. Flake outputs unchanged in name: nix eval reports 8 packages (pi-acp, pi-codex-alias, pi-notify-me, pi-ollama-usage, pi-opencode-usage, pi-permission, pi-prompts, pi-skills) and 11 checks wiring every package plus biome-check/tsc-check/node-tests. Breaking-change claim for pi install consumers is supported: only skills/pr-review-repair is packed now.
Check status at reviewed head
- Fresh (judge):
npm test→ 403 tests, 402 pass, 1 skip, 0 fail;nix build --rebuildofbiome-checkandtsc-check→ exit 0; repo-wide relative-link scan → 0 dangling. - Fresh (review):
nix build --no-link --rebuildofbiome-check,tsc-check,node-tests→ exit 0 (node-tests 403/403); all 8 flake package outputs rebuilt → exit 0. - Cached but content-addressed to HEAD (flake src = clean git tree):
node-tests(/nix/store/2rq1z3…-node-tests, built earlier, all 403 pass withPI_SKILLS_OUTPUT), plus package outputs and thepi-acpcheck, which were evaluated but not force-rebuilt by the judge. nix flake check -Litself ran 0 checks (cache-only), so it is not counted as independent evidence.
Limitations
Requirements coverage is limited to the PR body (untrusted); its downstream patextreme/dotfiles claim is unverifiable here. No live-model execution of pr-review-repair, no GitHub markdown rendering check, and no test of pi install against a real Pi host. No escalation questions.
Audit record
- Run ID:
e3f55d4501cfa9b459d2567cb1fb2491aec6eeb5(reported ase3f55d4); round 1; kindsummary. - PR: refactor(skills)!: remove OpenSpec workflow and factory skills #93 (
patextreme/agent-mod#93), basemain, headcleanup, same repository. - Pinned base / merge-base:
7a8166a755da44b3036b6fd1a7aa18c7f21d0b81/ same. - Reviewed / local / remote head:
e3f55d4501cfa9b459d2567cb1fb2491aec6eeb5(all equal). Delivered head: not applicable — no commits or pushes. - Worktree:
/home/pat/Desktop/workspace/personal/agent-mod, clean before and after review/judgment. - Repairs used: 0/10. Repair-start / scope-amendment records: not applicable (no repair attempt).
- Checks:
nix flake check -Lexit 0 but cache-only (0 checks run); fresh rebuilds as listed above; directnpm test403 tests / 402 pass / 1 skip / 0 fail. No required check failed; no check was unavailable. - Coverage: limited coverage explicitly authorized by the user; no tracked issue/spec exists. Standards sources:
AGENTS.md,docs/agents/*,README.md,GLOSSARY.md,docs/adr/0002-*, gate tests,biome.json,tsconfig.json,package.json,.github/workflows/, plus thecode-reviewsmell baseline. - Reviewers: fresh Standards and Spec agents with no prior reports/history; independent judge re-verified the reports and adversarially swept the full 194-file diff. Prior-ID reconciliation: none (first round; 0 prior comments/reviews).
- Delivery state: no commit, no push, no merge; final PR decision recorded by this comment. Merge remains the user's decision.
Summary
Remove the OpenSpec workflow and everything it governed:
openspec/change store (active changes, archive, specs, config)openspec-review,openspec-propose-issue,orc-openspec-*,orc-issue-to-pr,cleanup-merged-issues, plus their local.picopiestests/semantic/, thetest:semanticscript, and the three gate tests that backed thempr-review-repairremains, without the OpenSpec finalization contract.AGENTS.md,README.md,GLOSSARY.md, anddocs/agents/*are updated to match, and.gitignorenow covers multi-output Nixresult-*symlinks.No flake outputs are removed or renamed —
pi-permission,pi-ollama-usage,pi-opencode-usage,pi-codex-alias,pi-notify-me,pi-prompts, andpi-skillsstill exist. The breaking part is the removed packaged skills and OpenSpec store forpi installconsumers.Verification
nix flake check— all checks pass, including the hermeticnode-teststier (400/400, 0 skipped)npm test— 399 pass, 1 Nix-only skip;biome check .andtsc --noEmitcleanpatextreme/dotfilesdevelop(the current config, agent-mod locked at6b4820b): it consumespi-acp,pi-permission,pi-ollama-usage,pi-opencode-usage,pi-codex-alias,pi-notify-me,pi-prompts/init.md, andpi-skills/pr-review-repair. None of the removed skills, prompts, or theopenspec/store are referenced; the one skill it takes frompi-skillsispr-review-repair, which this branch retains. (pkgs.llm-agents.openspecin its module list is the external CLI package, unrelated to this repository.)