From a1383c0d951b9a57c149d6236528620978be00c0 Mon Sep 17 00:00:00 2001 From: Xinyi Lin Date: Tue, 8 Sep 2026 00:53:05 -0400 Subject: [PATCH 1/2] fix(setup): honor provider roots and calibrate agent workflows --- .github/pull_request_template.md | 9 + AGENTS.md | 33 ++ CLAUDE.md | 4 + CONTINUITY.md | 31 +- INIT.md | 8 + README.md | 77 +++++ docs/engineering/git-workflow.md | 38 ++- docs/guidance-loading-smoke.md | 71 +++++ docs/provider-calibration-2026-09-07.md | 145 +++++++++ .../.github/pull_request_template.md | 9 + project-templates/AGENTS.md | 26 +- project-templates/CLAUDE.md | 8 +- project-templates/TEMPLATE-USAGE.md | 19 ++ .../docs/engineering/git-workflow.md | 41 ++- scripts/install-skills.mjs | 80 ++++- scripts/test-bootstrap.mjs | 299 ++++++++++++++++++ 16 files changed, 862 insertions(+), 36 deletions(-) create mode 100644 docs/provider-calibration-2026-09-07.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 2dc35b4..c2d883a 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -13,6 +13,15 @@ Keep this receipt factual. If a check was not run, say why. - Related product / engineering / continuity docs: - Breaking change, migration, or rollout note: None / describe +## PR size + +- Counted additions + deletions against the PR base: +- Excluded generated/lockfile/pure-rename/binary paths and reasons: None / describe +- Above 500 lines: why keeping this change together improves review: +- Above 1,000 lines: split plan, or explicit independent-reviewer exception + (reviewed head, rationale, and review approach): + + ## Verification - Focused checks: diff --git a/AGENTS.md b/AGENTS.md index 9fd2516..46a947b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,6 +56,32 @@ facts. It is not an agent-policy source. - Sandbox authorization to access the parent registry or a sibling target does not load that sibling's instructions into this session. +## Product-delivery hook + +The installed `~/.agents/workflows/product-delivery/` contract applies to this +repository, not only to the projects it seeds. In addition to its explicit +role, user-request, and active-artifact triggers, its complete flow activates +when a change alters any of these contracts: + +- installation ownership, target boundaries, overwrite or recovery behavior; +- skill acquisition sources, reviewed revisions/hashes, or integrity validation; +- provider adapter fields, discovery locations, or generated instruction meaning; +- workflow activation, approval, delegation, or required review policy; +- the hard invariants above, or guidance propagated into sibling projects. + +Check the resulting behavior and contract diff in `workflows/`, templates, +manifests, scripts, and owning guidance. A path match alone does not activate +the flow. Typo/link corrections, reflow, tests of unchanged behavior, and +internal refactors preserving these contracts are maintenance. Record the +classification and evidence; any listed contract change activates the full +flow even if most of the diff is maintenance. Continuity-only updates remain +outside it unless they continue an active workflow artifact. + +Independent review means a fresh reviewer that did not make the change: the +installed Verifier role unless the user names another. When a listed condition +matches and the workflow or its review is not run, say so and record it; an +unrecorded skip is a process failure, not a judgment call. + ## Working method - Keep changes scoped to the requested behavior and its necessary cleanup. @@ -83,3 +109,10 @@ The full regression suite is the primary gate. Guidance changes also require path, import, byte-budget, and internal-consistency checks. Line targets are readability warnings, not CI gates. Report skipped host-capability checks and unverified external behavior explicitly. + +Every implementation, including maintenance outside the full workflow, needs +implementer verification and one fresh independent review before local +completion. Delegate it to the installed Verifier (`mb_verifier` in Codex, +`mb-verifier` in Claude), unless the user names another reviewer. Only the user +may waive review for a specific change; record the waiver without calling it +passed. This requirement does not activate the full workflow by itself. diff --git a/CLAUDE.md b/CLAUDE.md index 1bf89a8..5cfee7f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,3 +5,7 @@ Use Claude Code's current official loading and configuration documentation when changing project starter guidance or generated adapters. Keep provider-specific tool mechanics here; shared policy belongs in `AGENTS.md`. + +The installed Verifier role is the generated `mb-verifier` subagent. When a +project also defines its own `verifier`, name which one satisfies independent +review before relying on it. diff --git a/CONTINUITY.md b/CONTINUITY.md index 29638ea..7702898 100644 --- a/CONTINUITY.md +++ b/CONTINUITY.md @@ -12,6 +12,12 @@ is read only when a task needs it. ## Snapshot +- 2026-09-08 [USER+CODE] PR size policy: 500-line target, 1,000-line review gate; + counting and exceptions live in each Git contract and PR template. + +- 2026-09-07 [USER+CODE] Codex-first calibration: custom provider roots and + maintenance/review policy (D025). Source findings, verification and recorded + process deviation: `docs/provider-calibration-2026-09-07.md`. - 2026-08-14 [USER+CODE] Sibling-only ownership replaces the workspace router (D005, D017, D021): the parent has optional inert `MACHINE.md` but no live `AGENTS.md`, `CLAUDE.md`, or `_templates/`. Machine-bootstrap and every project @@ -52,6 +58,10 @@ is read only when a task needs it. project work. Standalone Verifier selection activates only independent review. - 2026-08-13 [USER] Every implementation defaults to implementer verification plus one fresh independent review; only the user may waive it per change. +- 2026-08-19 [USER+CODE] Workflow activation must be stated as checkable + conditions, the independent reviewer is named rather than implied, an + unrecorded skip is a process failure, and this repository applies the + workflow to itself (D024). - 2026-08-13 [USER] Dependency versions come from current official sources. Prefer the latest compatible stable release, preserve package-manager/range policy, update tracked lockfiles, and explain older or prerelease choices. @@ -119,16 +129,22 @@ Milestones — detail in `docs/continuity/2026-07.md`: - 2026-08-14 [USER] D023 ACTIVE: document agent-assisted setup as a normal interface. Natural-language requests select the target and outcome; audited scripts remain the execution and verification layer. +- 2026-08-19 [USER+CODE] D024 ACTIVE: workflow triggers are checkable conditions, not + self-assessed judgment; the satisfying reviewer is named; skips are recorded; + and machine-bootstrap is in scope for its own workflow. Provider adapters own + harness mechanics; README and shared reviewer guidance name provider roles. + +- 2026-09-07 [USER+CODE] D025 ACTIVE: Codex desktop is the primary documented + entry; Claude retains native loading. Workflow triggers name changed contracts; + maintenance keeps fresh review and existing user authorization carries forward. ## Working Set - `AGENTS.md`, `CLAUDE.md`, `README.md`, `INIT.md`, `CONTINUITY.md` - `.github/{pull_request_template.md,workflows/bootstrap.yml}` -- `machine-templates/` -- `project-templates/` +- `machine-templates/`, `project-templates/` - `docs/{engineering/git-workflow.md,guidance-loading-smoke.md,continuity/}` -- `scripts/{init-workspace,init-project,test-bootstrap}.mjs` -- `scripts/{install-skills,install-workflows}.mjs` +- `scripts/{init-workspace,init-project,test-bootstrap,install-skills,install-workflows}.mjs` - `scripts/lib/{fs-safety,portable-path,skill-*,workflow-*}.mjs` - `skills.json`, `workflows/` @@ -136,8 +152,5 @@ Milestones — detail in `docs/continuity/2026-07.md`: - Keep scratch optional and disposable; promote durable work into its own sibling repository with project guidance. -- Add a private remote only with explicit user authorization. - -## Open Questions - -- 2026-07-27 [USER] UNCONFIRMED which private remote will host this repository. +- Impeccable 4.0.2 restored with explicit user authorization and a recoverable + 4.0.4 backup; retain the pin until a freshly reviewed upgrade recommendation. diff --git a/INIT.md b/INIT.md index 92d44f9..a269245 100644 --- a/INIT.md +++ b/INIT.md @@ -67,3 +67,11 @@ access does not load sibling instructions. Use `--skip-skills` or `--skip-workflows` only when the user explicitly wants that shared layer left untouched. Installing `product-delivery` makes its roles available but does not activate the workflow for ordinary work. + +Codex desktop is the primary interactive entry; use the task prompts in +`README.md`. CLI profiles configure new CLI sessions only. Verify the selected +provider roots: workflow adapters and Claude skill links honor +`CLAUDE_CONFIG_DIR`; Codex adapters and duplicate-skill checks honor `CODEX_HOME`. +Canonical shared skills remain under `~/.agents/skills/`. Use the expanded +default/custom-root and nested-loading checks in `docs/guidance-loading-smoke.md` +when establishing runtime support. Report unrun scenarios explicitly. diff --git a/README.md b/README.md index 8ed18f7..23fd6ba 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,22 @@ Verifier activates only independent verification for the supplied change; it does not retroactively create missing upstream gates or artifacts. Ordinary work creates neither by default. +For this repository, `AGENTS.md` names the activating contracts: installation +ownership and recovery, acquisition integrity, provider formats/discovery, +workflow policy, hard invariants, and propagation to siblings. Mechanical +maintenance that preserves those contracts stays outside the full flow. Every +implementation still receives self-verification and fresh independent review +before local completion. Record the classification rather than judging a task +by its apparent size. Existing project-owned starters are never synchronized +when these defaults change. + +The Git contracts and PR templates use a 500-line target and a 1,000-line +review gate, measured as additions plus deletions. Changes over 1,000 require a +split or an explicit reviewer exception; generated/lockfile/rename/binary +changes are reported separately. See `docs/engineering/git-workflow.md` for +counting, exceptions, and review requirements. These gates are guidance for +both providers, not an automated CI size limit. + ### The three roles | Role | Owns | Claude agent | Codex agent | @@ -244,6 +260,45 @@ it does not read or write `settings.json` in the Claude configuration root or ### Launching a role +#### Codex desktop (primary workspace) + +Open a task in the target repository, then state the outcome in ordinary +language. Root guidance routes routine work; inspect the nearest scoped guide +before editing a subtree. Use a fresh task starting in that scope when relying +on automatic nested instruction discovery. + +For ordinary implementation: + +> Implement the requested change within the existing contract. Run the affected +> checks, then delegate a fresh independent review to `mb_verifier`. Report the +> evidence and unresolved findings. Keep changes local. + +For a review without implementation: + +> Delegate review of the current changes to `mb_verifier`. Inspect the actual +> diff and checks; report findings without editing or publishing. Missing full +> workflow artifacts are a limitation only where that workflow was required. + +For the complete discovery-to-delivery workflow: + +> Use the complete `product-delivery` workflow for this task. Read its installed +> contract, begin Product Partner discovery, and follow its separate brief and +> plan approval gates. Keep the approved artifacts available for role handoffs. + +These are task instructions, not an app profile selector. The CLI profile +commands below configure a primary CLI session; do not assume they select the +role of an existing desktop task. Custom agents are delegated specialists. +Codex currently supports delegation explicitly requested by the user or by +applicable project/skill instructions. Leave model and reasoning settings +inherited unless the user chooses otherwise; bootstrap adapters pin neither. + +Confirm `mb_product_partner`, `mb_delivery_lead`, and `mb_verifier` are available +in a fresh task after installation. If the harness cannot expose an installed +role, report that limit and request a replacement reviewer or explicit waiver +when review is required. A new user-owned task is not needed for each subagent. + +#### Claude Code and Codex CLI + Launching Product Partner or Delivery Lead activates the complete workflow. Launching the Verifier activates only its independent verification portion for the supplied change. @@ -280,6 +335,10 @@ profile because it is not a primary-session role. Start a new session after installation so the provider rediscovers the definitions. +Claude retains its native `--agent` entry and `@AGENTS.md` shared-guidance +adapter. Its lazy nested `CLAUDE.md` loading differs from Codex's startup guide +chain; use the provider-specific checks in `docs/guidance-loading-smoke.md`. + ### Updating a workflow Edit the canonical contract or template in `workflows/`, bump @@ -308,6 +367,15 @@ not weaken exact user approval, scope-change escalation, or honest verification. ## Updating +Shared skill content lives in `~/.agents/skills/`, which Codex discovers +directly. Claude links live under `$CLAUDE_CONFIG_DIR/skills/` when set, or +`~/.claude/skills/` otherwise. Checks for redundant Codex-specific copies use +`$CODEX_HOME/skills/` when set, or `~/.codex/skills/` otherwise. Changing a +provider configuration root does not relocate canonical shared skills. Relative +override paths resolve from the command's working directory; empty overrides +use the defaults, matching workflow installation. Inactive default provider +directories and unrelated entries remain untouched. + The installer adds only missing skills and rejects content that differs from `skills.json`. To update a skill, review the upstream change at its repository, then revise its exact `sourceRevision`, `sourcePath` when needed, expected @@ -344,3 +412,12 @@ provider roots. Manual provider loading checks live in `docs/guidance-loading-smoke.md`. Static tests are not runtime loading evidence. + +Provider contracts checked against official documentation on 2026-09-07: +[Codex instructions](https://developers.openai.com/codex/guides/agents-md), +[Codex skills](https://developers.openai.com/codex/skills), +[Codex subagents](https://learn.chatgpt.com/docs/agent-configuration/subagents), +[Claude memory](https://code.claude.com/docs/en/memory), and +[Claude configuration directories](https://code.claude.com/docs/en/claude-directory). +These sources establish supported contracts, not successful loading on every +installed provider version. diff --git a/docs/engineering/git-workflow.md b/docs/engineering/git-workflow.md index 33194bc..1421adf 100644 --- a/docs/engineering/git-workflow.md +++ b/docs/engineering/git-workflow.md @@ -35,6 +35,38 @@ Keep commits coherent and reviewable. Inspect the complete staged diff and run the relevant local checks before committing. Never bypass hooks unless the user explicitly authorizes the exception. +## PR size + +Plan PR boundaries before a large implementation. Each PR should deliver one +coherent behavior change and pass its checks independently. Keep related code, +tests, and necessary documentation together; separate unrelated refactors. + +Count additions plus deletions in the proposed PR diff against its actual base +using the merge base, not the sum of individual commits. For stacked PRs, use +the declared parent branch. Include handwritten code, tests, and documentation. +Report generated output, lockfiles, pure renames, and binary files separately +with exact paths and reasons; exclude them from the numeric threshold but still +review them. Count edits within renamed files. Unknown or mixed-content files +remain counted; do not classify handwritten changes as generated to fit a limit. + +| Counted changed lines | Required action | +| :--- | :--- | +| Up to 500 | Normal target; keep the PR focused | +| 501–1,000 | Explain in the PR body why keeping the change together improves review | +| Over 1,000 | Split it, or obtain a documented exception from the fresh independent reviewer before calling it ready or merging | + +For an exception, record the counted size, excluded paths, why a safe split is +not useful, how the change can be reviewed, and the reviewer's explicit +acceptance for the reviewed head. The author cannot self-approve. A material +scope change or changed head requires renewed review and exception confirmation. +An exception does not waive tests, independent review, or publication authority. +If independent review is user-waived, only the user can explicitly grant the +size exception; the review waiver alone is insufficient. + +These are review-policy gates, not an automated CI size check. Start with the +500/1,000 defaults; any project-specific adjustment belongs in its owning Git +contract with rationale, rather than an ad hoc per-PR threshold change. + ## Pull requests and review Use `.github/pull_request_template.md`. A PR title should match the intended @@ -46,8 +78,10 @@ Before opening or updating a PR: 1. Confirm worktree, branch, base, and exact changed paths. 2. Run focused owner checks and affected consumer checks. 3. Run `git diff --check` and inspect the complete diff. -4. Complete implementer self-review and one fresh independent review unless the - user explicitly waives it for this change. +4. Complete implementer self-review, then one fresh independent review by a + reviewer that did not make the change — the installed Verifier role unless + the user names another. Only the user may waive that review for a change. + `AGENTS.md` also requires this before local completion, including maintenance. 5. Update affected guidance, engineering, and continuity owners. Before merge, confirm the PR head is the exact reviewed and verified commit. diff --git a/docs/guidance-loading-smoke.md b/docs/guidance-loading-smoke.md index 42c8d48..9085e4c 100644 --- a/docs/guidance-loading-smoke.md +++ b/docs/guidance-loading-smoke.md @@ -60,6 +60,73 @@ required. Each session should contain only its repository marker through that repository's `CLAUDE.md` adapter and single `@AGENTS.md` import. The workspace and scratch sessions should contain none of the three repository markers. +## Nested guidance and overrides + +Extend the disposable project A with `src/scoped/AGENTS.md` carrying +`PROJECT_A_SCOPED`, a sibling `CLAUDE.md` containing only `@AGENTS.md`, and +`example.txt` containing a harmless fixture marker. + +| Scenario | Codex expectation | Claude expectation | +| :--- | :--- | :--- | +| Start at project A root | Root guide loaded; scoped guide not automatically in startup chain | Root import loaded; scoped import loads when reading the scoped fixture | +| Start at `src/scoped/` | Root and scoped guides in startup chain | Root and scoped imports loaded | +| Add scoped `AGENTS.override.md` with `PROJECT_A_OVERRIDE` | A fresh scoped run selects override instead of scoped `AGENTS.md` | Explicit `@AGENTS.md` still imports the shared guide; no automatic override selection | +| Access project B from project A | Access does not switch the startup guide chain | Record actual sources; additional-directory options can affect loading | + +Remove the temporary override before testing shared-policy parity. Codex +overrides are a provider mechanism, not a portable way to change shared policy. +Do not rely on a root-started Codex task navigating into a subtree to rebuild +its startup chain; it must explicitly inspect the applicable shared guide. + +## Configuration roots and role discovery + +Use disposable homes and configuration directories for these checks. Never +copy real authentication, settings, global guidance, or session logs into them. +If provider authentication cannot be supplied through an approved isolated +runtime, perform static installation checks and mark runtime discovery +unverified. Do not weaken permissions or transplant credentials to get a pass. + +Repeat the checks with default roots, custom absolute roots, and relative roots +containing spaces. Supply environment overrides only to the fixture process; +do not change the user's shell or global defaults. + +Where directory symlinks are supported, also relocate a custom configuration +root to a target at a different depth. Confirm the installed skill is readable +through its link, not merely that the link text looks correct. A regular file +blocking any ancestor of a custom root must fail preflight before acquisition. + +1. Install audited skill fixtures and workflow adapters into the disposable + home. With custom roots selected, confirm the defaults are untouched. +2. Confirm canonical skill content remains at the fixture home's + `.agents/skills/`; Claude links appear under the selected + `CLAUDE_CONFIG_DIR/skills/`. A missing selected link must fail `--check` + even when the default Claude directory has a valid link. +3. Confirm custom Codex agents/profiles appear under `CODEX_HOME`. A redundant + skill under the selected `CODEX_HOME/skills/` must fail installation/check + without writing, while unrelated inactive default entries remain preserved. +4. In a fresh provider run, establish loaded skill paths and role discovery. + Both providers should find the three namespaced roles; Codex discovers the + canonical shared skills, while Claude uses its selected skill links. +5. Launch both primary CLI roles using the README commands. Record role + activation and instruction sources; launch is not approval of an artifact. + +The Node regression suite covers fixture installation, custom-root false +success, and invalid destinations. It does not launch a model or establish +runtime skill discovery. + +## Codex desktop + +In a fresh desktop task for the disposable project, follow the README's +ordinary-review prompt. Confirm `mb_verifier` actually runs as a separate +reviewer and returns evidence without requesting an unrelated full workflow. +Check the full-workflow prompt separately: the brief and plan remain separate +approval gates. A CLI profile command is not proof of desktop role activation. + +Where the app supports a separate worktree, repeat root/scoped discovery there +and verify the effective repository and instruction sources. Record the app +version separately from the CLI version; one does not prove the other's +behavior. Replacing generated adapters requires a new discovery run. + ## Reporting Runtime loading is `verified` only for the provider version and scenario @@ -67,3 +134,7 @@ actually observed. Otherwise report it as `unverified`; do not promote the static Node suite to runtime evidence. Record provider version, date, launch directory, exact command, observed sources, and ambiguity. Remove the fixture and logs after recording a non-sensitive summary. + +Record each scenario separately as passed, failed, unverified, or skipped. +Report role availability, successful delegation, profile activation, and nested +guidance as separate observations; none is a substitute for the others. diff --git a/docs/provider-calibration-2026-09-07.md b/docs/provider-calibration-2026-09-07.md new file mode 100644 index 0000000..c2bbf7c --- /dev/null +++ b/docs/provider-calibration-2026-09-07.md @@ -0,0 +1,145 @@ +# Codex and Claude setup calibration + +Date: 2026-09-07. Scope: machine-bootstrap and its canonical project starters. +This receipt contains portable source facts only, not machine inventory. + +## Authorization and workflow accounting + +The user requested a Codex-first review retaining Claude support, then said +"go with your recommendation" after receiving the prioritized findings. +That session authorization covers the recommended implementation. Existing +dirty guidance changes were preserved and integrated into this calibration. +The user subsequently authorized final setup polish, push, and merge when ready. +Sibling propagation remains outside scope. + +This change activates the full workflow under both the original path triggers +and the new contract triggers: it changes provider discovery and review policy. +The formal separate exact-version Product Brief and Delivery Plan approval +gates were not run. Execution followed the user's direct authorization and the +active session requirement to continue already-authorized work. This is a +recorded process deviation, not a claim that those two artifact approvals +occurred or that future tasks can omit them. The independent Verifier must +report that limitation. No waiver of independent review was requested. + +## Implementation and acceptance evidence + +| Recommendation | Result and evidence | +| :--- | :--- | +| Correct provider configuration roots | Skill installer uses `CLAUDE_CONFIG_DIR` and `CODEX_HOME` consistently with workflow installer; canonical content remains under the user's `.agents/skills/` | +| Prevent false successful verification | Disposable tests require missing custom Claude links to fail even when default links exist; selected Codex duplicates and malformed roots fail before acquisition | +| Preserve unrelated configuration | Tests cover inactive defaults, unrelated entries, relative paths with spaces, unset/empty fallback, relocated symlinks, and obstructed ancestors | +| Calibrate workflow activation | Root guidance names contract changes; mechanical maintenance preserving those contracts remains outside the full flow | +| Keep independent review for ordinary work | Root and starter completion guidance require self-verification and a fresh reviewer before local completion, including work without a PR | +| Avoid repeat permission loops | Starter respects instruction precedence and existing user authorization; unresolved decisions and new scope still require questions | +| Prioritize Codex desktop; retain Claude | README adds desktop task prompts and distinguishes CLI profile activation; Claude native roles and the single `@AGENTS.md` import remain | +| Expand runtime verification coverage | Smoke guide covers nested roots, overrides, custom provider directories, desktop delegation, and worktree scenarios; documented scenarios are not runtime passes | +| Reconcile Impeccable | Reviewed 4.0.2 restored with user authorization; the complete 4.0.4 copy remains recoverable outside skill discovery | + +The portable Product Delivery package and generated adapters remain at 1.3.0; +their source contracts did not change. These are project trigger and starter +changes, not a generated adapter format update. Seeded sibling guidance remains +project-owned and is not synchronized. + +## Impeccable source review and change request + +The existing manifest pins 4.0.2 at +`fc2e694afca1ac0cc384b4fe56bab3335fea7912`, content hash +`c882be322c5f25047191b0675f0dcd6000b0b0ac7a9ce3d7fba649bf58f6abe5`. +Materializing that public Git revision through the audited acquisition helper +reproduced its hash without executing upstream code. + +The previously installed 4.0.4 files match upstream commit +`e2761cae80ec90986c642739c7b42bc387e2d580` exactly by Git blob identities and +independent materialization through the audited acquisition helper. Both yield +the content hash +`1a05c01f46f896e103d2a9c9197ba8407cb82e15dc9deeb2b95ff7160b40ed6e`. +It is not the `skill-v4.0.4` tag (`9a949fb543d44cfb406f61bcab99d95d7f12cf1d`), +whose materialized hash is +`682acbf4650aefc64987290653b3eb0e467c27fb879c7bce991b7f98e355207b`. +Changing only the expected version or accepting an arbitrary local hash would +not provide reproducible acquisition. + +Independent static review found a new optional live-edit journal cleanup +boundary defect in `scripts/live/frameworks/journal.mjs`: lexical containment +does not reject an intermediate symlink leading outside the project. A crafted +created-file entry with a matching marker can remove a file outside the project; +patched entries can rewrite one. The tag and installed 4.0.4 share this code. +No upstream helper was executed to test the finding. Hooks remain opt-in. + +The user explicitly authorized restoring reviewed 4.0.2 on 2026-09-08 and +retaining it until a later reviewed upgrade recommendation. The audited Git +acquisition helper freshly reproduced the pinned hash without upstream code +execution. The complete 4.0.4 directory was moved to a recoverable backup +outside skill discovery, then replaced with verified 4.0.2. Both hashes were +verified afterward. The manifest and unrelated provider configuration remain +unchanged. This avoids the newly added helper; it does not certify all 4.0.2 +behavior. Recommending an upgrade requires fresh review of the cleanup fix and +any changed acquisition or runtime behavior. + +On 2026-09-08, the user asked whether a newer release addresses the finding. +The latest skill release is 4.2.2 at +`f64da20b07271b760e4e3133eef3b87942860f11`, using engine 0.1.3. Its launcher can +download and execute a platform binary on first use. Static review of +`crates/live/src/journal.rs` and `crates/live/src/util.rs` found the cleanup path +still uses lexical containment before file removal or rewriting. Current main +has no changes to those files relative to the release. The separate live-server +`/source` symlink fix does not establish that journal cleanup is fixed. +Independent review confirmed the same cleanup implementation in engine 0.1.3 +at `2abca8b472afa15dd5f0430ea5c5f86911a14806`, including reachable removal and +orphan-healing callers. Released binary correspondence and an end-to-end +reproduction remain unverified; no upstream executable was run. The user chose +restoration after this comparison. + +Public evidence: [4.0.2 source](https://github.com/pbakaus/impeccable/tree/fc2e694afca1ac0cc384b4fe56bab3335fea7912/.agents/skills/impeccable), +[matching 4.0.4 source](https://github.com/pbakaus/impeccable/tree/e2761cae80ec90986c642739c7b42bc387e2d580/.agents/skills/impeccable). +[4.2.2 release](https://github.com/pbakaus/impeccable/releases/tag/skill-v4.2.2) +and [cleanup source](https://github.com/pbakaus/impeccable/blob/f64da20b07271b760e4e3133eef3b87942860f11/crates/live/src/journal.rs). +Provider documentation sources are linked in README. + +## Verification + +- Full Node 24.19.0 regression suite after restoration: all 92 tests passed, + with no skips. This includes all nine provider-root groups and guidance + path/import, byte, and newline-inclusive continuity budget checks. Earlier + runs failed only on the now-reconciled installed Impeccable drift. +- Workspace check after restoration: passed for all eight shared skills and + the Product Delivery workflow with both providers' adapters. +- Workflow installation check: passed; installed package/adapters match source. +- Whitespace check: passed. +- External TOML parser: the 2026-09-08 full suite used bundled Python 3.12.14; + `tomllib` parsed both adversarial generated-adapter fixtures successfully. +- Codex desktop: installed `mb_verifier` successfully delegated during review. + Other desktop/CLI profile and nested-loading scenarios were not run. +- Claude: native runtime loading/delegation was not exercised. +- Cross-platform CI: not run in this local task. +- Independent review found and retested fixes for unreadable links through + relocated Claude directories and file-obstructed ancestors. The final full + diff review found no unresolved implementation or guidance defect. Its only + failed setup gate was the subsequently reconciled Impeccable drift. +- Separate brief/plan approval evidence remains absent as recorded above; + runtime discovery and CI claims require their own evidence. + +## 2026-09-08 follow-up: PR size policy + +The user approved the proposed 500-line target and 1,000-line review gate, +allowing the numbers to be adjusted if needed. Both defaults were retained. +This changes required review policy and therefore activates the full workflow. +As with the calibration above, execution follows direct session authorization; +separate exact-version brief/plan approval gates were not run. That deviation +is recorded, not treated as artifact approval or a future exemption. + +Both Git contracts now define additions-plus-deletions counting against the +actual PR base's merge base, with handwritten tests/docs included and generated +output, lockfiles, pure renames and binaries reported separately. Changes above +500 lines need an explanation; above 1,000 need a split or explicit fresh +reviewer exception for the reviewed head. A user waiver of independent review +does not itself grant a size exception. Both PR templates collect the evidence. +README and starter usage explain the policy and project-owned adjustments. +No size CI automation or sibling propagation was added. + +Verification of this follow-up: both policy sections and both PR templates +match exactly; continuity's 160-line budget and `git diff --check` pass. +A fresh independent Verifier reviewed the actual policy diff, counting and exception +semantics, matching copies, and continuity budget, and found no material defect. +Final publication review must cover any subsequent edits and the exact head. +The separate brief/plan approval limitation remains recorded above. diff --git a/project-templates/.github/pull_request_template.md b/project-templates/.github/pull_request_template.md index 2dc35b4..c2d883a 100644 --- a/project-templates/.github/pull_request_template.md +++ b/project-templates/.github/pull_request_template.md @@ -13,6 +13,15 @@ Keep this receipt factual. If a check was not run, say why. - Related product / engineering / continuity docs: - Breaking change, migration, or rollout note: None / describe +## PR size + +- Counted additions + deletions against the PR base: +- Excluded generated/lockfile/pure-rename/binary paths and reasons: None / describe +- Above 500 lines: why keeping this change together improves review: +- Above 1,000 lines: split plan, or explicit independent-reviewer exception + (reviewed head, rationale, and review approach): + + ## Verification - Focused checks: diff --git a/project-templates/AGENTS.md b/project-templates/AGENTS.md index f96186a..098d9fe 100644 --- a/project-templates/AGENTS.md +++ b/project-templates/AGENTS.md @@ -60,11 +60,22 @@ activates independent verification only and never creates missing upstream artif When active, follow the installed contract without copying its gates or artifacts here. Project-specific additions only: -- Named work that activates the workflow: TODO, or `none`. +- Named work that activates the workflow: TODO as checkable conditions — paths, + contracts, or artifact kinds a reader can evaluate — never a self-assessed + size or importance test; or `none`. +- Maintenance outside those conditions: typo/link corrections, reflow, tests + of unchanged behavior, and internal refactors preserving the named contracts. + A change to a named contract still activates the flow; record the classification. - Additional Change Request triggers: TODO, or `none`. - Required project checks or specialist review: TODO, or `the commands below`. +- Reviewer that satisfies independent review: TODO exact agent or command name, + or `the installed Verifier`; resolve any project agent that shares its name. - Artifact-retention policy: TODO, or `local while active`. +When a listed condition matches and the workflow or its review is not run, say +so in the response with the reason. An unrecorded skip is a process failure, +not a judgment call. + ## Commands Run from the repository root unless a command says otherwise. @@ -84,7 +95,9 @@ Run from the repository root unless a command says otherwise. - Implement the smallest maintainable change that satisfies the request. Avoid speculative features, abstractions, configuration, and drive-by cleanup. - Ask before changing dependencies, schemas, authentication, deployment, - destructive storage behavior, paid services, or public network exposure. + destructive storage behavior, paid services, or public network exposure + when the user's current request or prior authorization does not cover it. + Continue authorized work; ask only for an unresolved decision or new scope. - For dependencies, inspect compatibility and version policy, verify the latest compatible stable or maintainer-recommended release from official sources, preserve the package manager/range policy, and update a tracked lockfile. @@ -112,6 +125,15 @@ they are part of the changed contract. If no harness exists, run the strongest lightweight check and state the gap. Report passed, failed, unverified, and skipped evidence honestly; missing evidence is not a pass. +Every implementation, including ordinary work outside the full workflow, +requires implementer verification and one fresh independent review before +local completion. Delegate to the reviewer named above; the installed Verifier +is `mb_verifier` in Codex and `mb-verifier` in Claude. The reviewer must not have +implemented the change. Only the user may waive review for a specific change; +record the waiver and reason, never a pass. If that reviewer is unavailable, +report the gap and request a replacement or waiver. This review alone does not +activate the full workflow or require its upstream artifacts. + ## Git and existing work Read `docs/engineering/git-workflow.md` before any branch, commit, push, PR, diff --git a/project-templates/CLAUDE.md b/project-templates/CLAUDE.md index a217a92..3ea87e8 100644 --- a/project-templates/CLAUDE.md +++ b/project-templates/CLAUDE.md @@ -6,8 +6,12 @@ > then delete this note. Keep the import exactly once. `AGENTS.md` is the canonical shared policy. This adapter adds only Claude-specific -mechanics and must not duplicate or contradict it. Resolve conflicts in the -files or ask for direction; do not rely on load order. +mechanics and must not duplicate or contradict it. Follow applicable instruction +precedence, including higher-priority harness and session requirements. Explain +any resulting project-process limitation and continue authorized independent +work. Ask only for unresolved user decisions or new scope; an already resolved +precedence conflict does not require another approval. Do not silently skip a +required project process for convenience. ## Claude-specific mechanics diff --git a/project-templates/TEMPLATE-USAGE.md b/project-templates/TEMPLATE-USAGE.md index 031dae4..4c0a160 100644 --- a/project-templates/TEMPLATE-USAGE.md +++ b/project-templates/TEMPLATE-USAGE.md @@ -11,6 +11,12 @@ copied or renamed as one. - `docs/engineering/git-workflow.md`: project-owned Git/publication contract. - `.github/pull_request_template.md`: PR scope and verification receipt. +The Git contract defines a 500-line PR target and a 1,000-line review gate; +the PR template records size, exclusions, and any required explanation or +reviewer exception. Keep related code/tests/docs together. Projects may adjust +the defaults in their own contract with rationale; existing seeded projects +are not changed when this starter is updated. + The starter `.gitignore` keeps shared `.claude/` and `.codex/` configuration trackable. It excludes `CLAUDE.local.md`, `.claude/settings.local.json`, and active `.agent-work/` artifacts. Remove the last exclusion only when the @@ -85,5 +91,18 @@ explicitly requests the complete workflow, an active Product Brief or Delivery Plan is continued, or project guidance requires it for named work. Selecting the Verifier activates only independent verification for the supplied change. +Fill the project's activating conditions with checkable facts — paths, +contracts, or artifact kinds — and name the reviewer that satisfies independent +review. A self-assessed test such as "non-trivial work" is not a condition, and +an unrecorded skip is a process failure. + +Prefer contract changes over blanket directory matches when defining triggers. +For example, name changes to data retention, authorization, or installation +ownership; exclude mechanical maintenance only when those contracts remain +unchanged. Ordinary implementations still receive self-verification and one +fresh independent review before local completion. This standalone review does +not require the full workflow's brief and plan. Existing user authorization +continues to apply; do not repeat a permission question for the same scope. + Personal preferences may live in gitignored `CLAUDE.local.md`, but they must remain compatible with tracked project instructions. diff --git a/project-templates/docs/engineering/git-workflow.md b/project-templates/docs/engineering/git-workflow.md index 7529503..de6aec6 100644 --- a/project-templates/docs/engineering/git-workflow.md +++ b/project-templates/docs/engineering/git-workflow.md @@ -54,13 +54,43 @@ and keep its summary, scope, verification, risks, and skipped-checks sections accurate. If the project squash-merges by default, use the Conventional Commit subject as -the PR title so the resulting base-branch commit remains consistent. Prefer a -reviewable behavior slice; aim for 500 changed lines or fewer and document why -an exception cannot be split. +the PR title so the resulting base-branch commit remains consistent. For stacked PRs, state the dependency and base branch. Merge the base PR first, update the dependent branch onto the new base, and rerun affected checks. +## PR size + +Plan PR boundaries before a large implementation. Each PR should deliver one +coherent behavior change and pass its checks independently. Keep related code, +tests, and necessary documentation together; separate unrelated refactors. + +Count additions plus deletions in the proposed PR diff against its actual base +using the merge base, not the sum of individual commits. For stacked PRs, use +the declared parent branch. Include handwritten code, tests, and documentation. +Report generated output, lockfiles, pure renames, and binary files separately +with exact paths and reasons; exclude them from the numeric threshold but still +review them. Count edits within renamed files. Unknown or mixed-content files +remain counted; do not classify handwritten changes as generated to fit a limit. + +| Counted changed lines | Required action | +| :--- | :--- | +| Up to 500 | Normal target; keep the PR focused | +| 501–1,000 | Explain in the PR body why keeping the change together improves review | +| Over 1,000 | Split it, or obtain a documented exception from the fresh independent reviewer before calling it ready or merging | + +For an exception, record the counted size, excluded paths, why a safe split is +not useful, how the change can be reviewed, and the reviewer's explicit +acceptance for the reviewed head. The author cannot self-approve. A material +scope change or changed head requires renewed review and exception confirmation. +An exception does not waive tests, independent review, or publication authority. +If independent review is user-waived, only the user can explicitly grant the +size exception; the review waiver alone is insufficient. + +These are review-policy gates, not an automated CI size check. Start with the +500/1,000 defaults; any project-specific adjustment belongs in its owning Git +contract with rationale, rather than an ad hoc per-PR threshold change. + ## Verification and review Before opening or updating a PR: @@ -69,8 +99,11 @@ Before opening or updating a PR: 2. Run the narrowest checks for the changed surface and affected consumers. 3. Run `git diff --check` and inspect the complete diff for scope, regressions, stale paths, and secrets. -4. Complete self-review and obtain one fresh independent review by default. +4. Complete self-review, then obtain one fresh independent review by default + from a reviewer that did not make the change; `AGENTS.md` names which one. Only the user may waive that review for a specific change. + The root guide requires the same review before local completion, even when + no PR or full product-delivery workflow is requested. 5. Update product, engineering, guidance, and continuity docs when behavior, ownership, commands, or durable state changed. diff --git a/scripts/install-skills.mjs b/scripts/install-skills.mjs index 4cc5d10..f07d75f 100644 --- a/scripts/install-skills.mjs +++ b/scripts/install-skills.mjs @@ -9,6 +9,7 @@ import { readdirSync, readFileSync, readlinkSync, + realpathSync, rmSync, statSync, symlinkSync @@ -33,8 +34,14 @@ const preflightOnly = args.has("--preflight"); const printHashes = args.has("--print-hashes"); const userHome = homedir(); const canonicalRoot = join(userHome, ".agents", "skills"); -const claudeRoot = join(userHome, ".claude", "skills"); -const codexRoot = join(userHome, ".codex", "skills"); +const claudeConfigRoot = process.env.CLAUDE_CONFIG_DIR + ? resolve(process.env.CLAUDE_CONFIG_DIR) + : join(userHome, ".claude"); +const codexHome = process.env.CODEX_HOME + ? resolve(process.env.CODEX_HOME) + : join(userHome, ".codex"); +const claudeRoot = join(claudeConfigRoot, "skills"); +const codexRoot = join(codexHome, "skills"); const failures = []; function failSetup(message) { @@ -82,6 +89,18 @@ function hasSkill(name) { return existsSync(join(canonicalSkillPath(name), "SKILL.md")); } +function resolveMissingTarget(path) { + let current = path; + while (true) { + try { + return resolve(realpathSync(current), relative(current, path)); + } catch (error) { + if (error?.code !== "ENOENT" || entryExists(current)) throw error; + current = dirname(current); + } + } +} + function validateClaudeDestination(name) { const source = canonicalSkillPath(name); const destination = join(claudeRoot, name); @@ -94,9 +113,22 @@ function validateClaudeDestination(name) { return; } - const actual = resolve(claudeRoot, readlinkSync(destination)); - if (comparablePath(actual) !== comparablePath(source)) { - failures.push(`${name}: Claude symlink points to ${actual}`); + try { + const expected = resolveMissingTarget(source); + let actual; + try { + actual = realpathSync(destination); + } catch (error) { + // A correct existing link may await installation of its canonical skill. + // Resolve its target from the effective directory, never the lexical root. + if (error?.code !== "ENOENT" || entryExists(source)) throw error; + actual = resolveMissingTarget(resolve(realpathSync(claudeRoot), readlinkSync(destination))); + } + if (comparablePath(actual) !== comparablePath(expected)) { + failures.push(`${name}: Claude symlink points to ${actual}`); + } + } catch { + failures.push(`${name}: Claude symlink cannot resolve to its canonical skill`); } } @@ -115,15 +147,19 @@ function ensureClaudeLinks(skills) { const createdLinks = []; try { mkdirSync(claudeRoot, { recursive: true }); + const effectiveClaudeRoot = realpathSync(claudeRoot); for (const skill of missingLinks) { - const source = canonicalSkillPath(skill.name); + const source = realpathSync(canonicalSkillPath(skill.name)); const destination = join(claudeRoot, skill.name); if (process.platform === "win32") { symlinkSync(source, destination, "junction"); } else { - symlinkSync(relative(claudeRoot, source), destination); + symlinkSync(relative(effectiveClaudeRoot, source), destination); } createdLinks.push(destination); + if (comparablePath(realpathSync(destination)) !== comparablePath(source)) { + throw new Error(`${skill.name}: Claude symlink does not resolve to its canonical skill`); + } } } catch (error) { for (const destination of createdLinks) { @@ -303,19 +339,29 @@ function installMissingSkills(skills) { for (const [label, root] of [ ["canonical skill root", canonicalRoot], + ["Claude configuration root", claudeConfigRoot], + ["Codex configuration root", codexHome], ["Claude skill root", claudeRoot], ["Codex skill root", codexRoot] ]) { - if (!entryExists(root)) continue; - // Followed, so a root relocated through a symlink stays supported. - let rootStat; - try { - rootStat = statSync(root); - } catch { - failSetup(`${label} cannot be read: ${root}`); - } - if (!rootStat.isDirectory()) { - failSetup(`${label} must be a directory: ${root}`); + // Follow relocated roots, but distinguish missing directories from a file + // or dangling symlink anywhere in their ancestors before acquisition. + let current = root; + while (true) { + let rootStat; + try { + rootStat = statSync(current); + } catch (error) { + if (error?.code === "ENOENT" && !entryExists(current)) { + current = dirname(current); + continue; + } + failSetup(`${label} cannot be read as a directory: ${root}`); + } + if (!rootStat.isDirectory()) { + failSetup(`${label} must be a directory: ${root}`); + } + break; } } diff --git a/scripts/test-bootstrap.mjs b/scripts/test-bootstrap.mjs index 1c92153..5bb97f2 100644 --- a/scripts/test-bootstrap.mjs +++ b/scripts/test-bootstrap.mjs @@ -176,6 +176,50 @@ function runScript(script, scriptArgs, testHome, environmentOverrides = {}) { return result; } +async function createSkillInstallerFixture() { + const testHome = createTestHome("skill-provider-roots"); + const checkout = join(testHome, "installer"); + const fixtureScriptDir = join(checkout, "scripts"); + mkdirSync(fixtureScriptDir, { recursive: true }); + copyFileSync( + join(scriptDir, "install-skills.mjs"), + join(fixtureScriptDir, "install-skills.mjs") + ); + cpSync(join(scriptDir, "lib"), join(fixtureScriptDir, "lib"), { + recursive: true + }); + const name = "fixture-skill"; + const canonical = join(testHome, ".agents", "skills", name); + mkdirSync(canonical, { recursive: true }); + writeFileSync(join(canonical, "SKILL.md"), "# Disposable test skill\n"); + const { hashDirectory } = await import("./lib/skill-integrity.mjs"); + writeFileSync(join(checkout, "skills.json"), JSON.stringify({ + schemaVersion: 3, + skills: [{ + name, + repository: "https://github.com/example/fixture.git", + sourcePath: "skills/fixture-skill", + sourceRevision: "a".repeat(40), + contentSha256: hashDirectory(canonical) + }] + })); + return { + testHome, + name, + canonical, + run(args = [], overrides = {}) { + return spawnSync(process.execPath, [ + join(fixtureScriptDir, "install-skills.mjs"), ...args + ], { + cwd: testHome, + encoding: "utf8", + env: isolatedEnvironment(testHome, { PATH: "", ...overrides }), + shell: false + }); + } + }; +} + async function test( name, callback, @@ -468,6 +512,261 @@ await test("incompatible Claude state fails before skill installation", () => { } }); +await test("skill links and checks use custom provider roots without changing defaults", async () => { + const fixture = await createSkillInstallerFixture(); + const { testHome, name, canonical } = fixture; + try { + // Relative overrides resolve from the invocation directory, as workflows do. + const overrides = { + CLAUDE_CONFIG_DIR: "providers/claude config", + CODEX_HOME: "providers/codex config" + }; + const customClaude = join(testHome, overrides.CLAUDE_CONFIG_DIR, "skills"); + const defaultClaude = join(testHome, ".claude", "skills"); + const defaultCodex = join(testHome, ".codex", "skills", name); + mkdirSync(defaultClaude, { recursive: true }); + symlinkSync(canonical, join(defaultClaude, name), + process.platform === "win32" ? "junction" : "dir"); + const originalLink = readlinkSync(join(defaultClaude, name)); + mkdirSync(customClaude, { recursive: true }); + writeFileSync(join(customClaude, "keep.txt"), "unrelated Claude entry\n"); + + const missing = fixture.run(["--check"], overrides); + assert.notEqual(missing.status, 0); + assert.match(missing.stderr, /fixture-skill: Claude symlink is missing/); + assert.doesNotMatch(missing.stderr, /redundant Codex-specific copy/); + assert.equal(existsSync(join(customClaude, name)), false); + + mkdirSync(defaultCodex, { recursive: true }); + writeFileSync(join(defaultCodex, "keep.txt"), "unrelated inactive copy\n"); + const installed = fixture.run([], overrides); + assert.equal(installed.status, 0, installed.stderr); + const destination = join(customClaude, name); + assert.equal(lstatSync(destination).isSymbolicLink(), true); + assert.equal(readFileSync(join(destination, "SKILL.md"), "utf8"), + readFileSync(join(canonical, "SKILL.md"), "utf8")); + const checked = fixture.run(["--check"], overrides); + assert.equal(checked.status, 0, checked.stderr); + assert.equal(readlinkSync(join(defaultClaude, name)), originalLink); + assert.deepEqual(readdirSync(defaultClaude), [name]); + assert.equal(readFileSync(join(defaultCodex, "keep.txt"), "utf8"), + "unrelated inactive copy\n"); + assert.equal(readFileSync(join(customClaude, "keep.txt"), "utf8"), + "unrelated Claude entry\n"); + assert.equal(existsSync(join(testHome, overrides.CODEX_HOME)), false); + } finally { + rmSync(testHome, { recursive: true, force: true }); + } +}, { needsSymlinks: true }); + +await test("unset and empty provider overrides retain default skill roots", async () => { + for (const overrides of [{}, { CLAUDE_CONFIG_DIR: "", CODEX_HOME: "" }]) { + const fixture = await createSkillInstallerFixture(); + const { testHome, name } = fixture; + try { + const installed = fixture.run([], overrides); + assert.equal(installed.status, 0, installed.stderr); + assert.equal(lstatSync(join(testHome, ".claude", "skills", name)) + .isSymbolicLink(), true); + const checked = fixture.run(["--check"], overrides); + assert.equal(checked.status, 0, checked.stderr); + const duplicate = join(testHome, ".codex", "skills", name); + mkdirSync(duplicate, { recursive: true }); + const rejected = fixture.run(["--check"], overrides); + assert.notEqual(rejected.status, 0); + assert.match(rejected.stderr, /redundant Codex-specific copy exists/); + } finally { + rmSync(testHome, { recursive: true, force: true }); + } + } +}, { needsSymlinks: true }); + +await test("relocated Claude roots create readable links and reject broken lexical links", async () => { + for (const relocatedPart of ["configuration", "skills"]) { + const fixture = await createSkillInstallerFixture(); + const { testHome, name, canonical } = fixture; + try { + const config = join(testHome, "config"); + const effectiveRoot = join(testHome, "deeper", "relocated", relocatedPart); + mkdirSync(effectiveRoot, { recursive: true }); + const relocated = relocatedPart === "configuration" ? config : join(config, "skills"); + mkdirSync(dirname(relocated), { recursive: true }); + symlinkSync(effectiveRoot, relocated, + process.platform === "win32" ? "junction" : "dir"); + const overrides = { CLAUDE_CONFIG_DIR: config }; + const result = fixture.run([], overrides); + assert.equal(result.status, 0, result.stderr); + const link = join(config, "skills", name); + assert.equal(readFileSync(join(link, "SKILL.md"), "utf8"), + readFileSync(join(canonical, "SKILL.md"), "utf8")); + const checked = fixture.run(["--check"], overrides); + assert.equal(checked.status, 0, checked.stderr); + + // Reproduce the previously accepted link based on the lexical root depth. + rmSync(link); + symlinkSync("../../.agents/skills/fixture-skill", link, "dir"); + const broken = fixture.run(["--check"], overrides); + assert.notEqual(broken.status, 0); + assert.match(broken.stderr, /Claude symlink cannot resolve to its canonical skill/); + assert.doesNotMatch(broken.stderr, /node:internal|file:\/\//); + } finally { + rmSync(testHome, { recursive: true, force: true }); + } + } +}, { needsSymlinks: true }); + +await test("a correct Claude link can await its missing canonical skill", async () => { + const fixture = await createSkillInstallerFixture(); + const { testHome, name, canonical } = fixture; + try { + const claudeRoot = join(testHome, "custom-claude", "skills"); + mkdirSync(claudeRoot, { recursive: true }); + symlinkSync(canonical, join(claudeRoot, name), + process.platform === "win32" ? "junction" : "dir"); + rmSync(canonical, { recursive: true }); + const overrides = { CLAUDE_CONFIG_DIR: dirname(claudeRoot) }; + const preflight = fixture.run(["--preflight"], overrides); + assert.equal(preflight.status, 0, preflight.stderr); + const checked = fixture.run(["--check"], overrides); + assert.notEqual(checked.status, 0); + assert.match(checked.stderr, /canonical skill is missing/); + assert.doesNotMatch(checked.stderr, /Claude symlink/); + const install = fixture.run([], overrides); + assert.notEqual(install.status, 0); + assert.match(install.stdout, /Fetching reviewed source/); + assert.doesNotMatch(install.stderr, /Claude symlink/); + assert.equal(existsSync(canonical), false); + } finally { + rmSync(testHome, { recursive: true, force: true }); + } +}, { needsSymlinks: true }); + +await test("provider roots beneath files fail preflight before acquisition", async () => { + for (const variable of ["CLAUDE_CONFIG_DIR", "CODEX_HOME"]) { + const fixture = await createSkillInstallerFixture(); + const { testHome, canonical } = fixture; + try { + const ancestor = join(testHome, "ancestor"); + writeFileSync(ancestor, "preserve ancestor\n"); + rmSync(canonical, { recursive: true }); + for (const args of [[], ["--check"], ["--preflight"]]) { + const result = fixture.run(args, { [variable]: join(ancestor, "nested") }); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /configuration root cannot be read as a directory/); + assert.doesNotMatch(result.stderr, /ENOTDIR|node:internal/); + assert.doesNotMatch(result.stdout, /Fetching reviewed source/); + assert.equal(existsSync(canonical), false); + assert.equal(readFileSync(ancestor, "utf8"), "preserve ancestor\n"); + } + } finally { + rmSync(testHome, { recursive: true, force: true }); + } + } +}); + +await test("dangling provider roots and ancestors fail before acquisition", async () => { + for (const variable of ["CLAUDE_CONFIG_DIR", "CODEX_HOME"]) { + const fixture = await createSkillInstallerFixture(); + const { testHome, canonical } = fixture; + try { + const dangling = join(testHome, "dangling"); + const missing = join(testHome, "missing"); + symlinkSync(missing, dangling, "dir"); + rmSync(canonical, { recursive: true }); + for (const configRoot of [dangling, join(dangling, "nested")]) { + for (const args of [[], ["--check"], ["--preflight"]]) { + const result = fixture.run(args, { [variable]: configRoot }); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /configuration root cannot be read as a directory/); + assert.doesNotMatch(result.stderr, /node:internal/); + assert.doesNotMatch(result.stdout, /Fetching reviewed source/); + assert.equal(existsSync(canonical), false); + assert.equal(existsSync(missing), false); + assert.equal(readlinkSync(dangling), missing); + } + } + } finally { + rmSync(testHome, { recursive: true, force: true }); + } + } +}, { needsSymlinks: true }); + +await test("custom Codex duplicates fail all setup modes before skill acquisition", async () => { + const fixture = await createSkillInstallerFixture(); + const { testHome, name, canonical } = fixture; + try { + const codexHome = join(testHome, "custom-codex"); + const duplicate = join(codexHome, "skills", name); + mkdirSync(duplicate, { recursive: true }); + writeFileSync(join(duplicate, "keep.txt"), "preserve duplicate\n"); + rmSync(canonical, { recursive: true }); + for (const args of [[], ["--check"], ["--preflight"]]) { + const result = fixture.run(args, { CODEX_HOME: codexHome }); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /redundant Codex-specific copy exists/); + assert.doesNotMatch(result.stdout, /Fetching reviewed source/); + assert.equal(existsSync(canonical), false); + assert.equal(existsSync(join(testHome, ".claude")), false); + assert.equal(existsSync(join(testHome, ".codex")), false); + assert.equal(readFileSync(join(duplicate, "keep.txt"), "utf8"), + "preserve duplicate\n"); + } + } finally { + rmSync(testHome, { recursive: true, force: true }); + } +}); + +await test("incompatible custom Claude destination stops before skill acquisition", async () => { + const fixture = await createSkillInstallerFixture(); + const { testHome, name, canonical } = fixture; + try { + const claudeConfig = join(testHome, "custom-claude"); + const destination = join(claudeConfig, "skills", name); + mkdirSync(dirname(destination), { recursive: true }); + writeFileSync(destination, "preserve existing destination\n"); + rmSync(canonical, { recursive: true }); + for (const args of [[], ["--preflight"]]) { + const result = fixture.run(args, { CLAUDE_CONFIG_DIR: claudeConfig }); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Claude destination exists but is not a symlink/); + assert.doesNotMatch(result.stdout, /Fetching reviewed source/); + assert.equal(existsSync(canonical), false); + assert.equal(existsSync(join(testHome, ".claude")), false); + assert.equal(readFileSync(destination, "utf8"), + "preserve existing destination\n"); + } + } finally { + rmSync(testHome, { recursive: true, force: true }); + } +}); + +await test("malformed custom provider roots stop before skill acquisition", async () => { + for (const [variable, provider] of [ + ["CLAUDE_CONFIG_DIR", "Claude"], ["CODEX_HOME", "Codex"] + ]) { + for (const leaf of ["configuration", "skill"]) { + const fixture = await createSkillInstallerFixture(); + const { testHome, canonical } = fixture; + try { + const configRoot = join(testHome, "custom-provider"); + const invalid = leaf === "configuration" ? configRoot : join(configRoot, "skills"); + mkdirSync(dirname(invalid), { recursive: true }); + writeFileSync(invalid, "preserve root file\n"); + rmSync(canonical, { recursive: true }); + const result = fixture.run([], { [variable]: configRoot }); + assert.notEqual(result.status, 0); + assert.match(result.stderr, new RegExp(`${provider} ${leaf} root must be a directory`)); + assert.doesNotMatch(result.stderr, /ENOTDIR|node:internal/); + assert.doesNotMatch(result.stdout, /Fetching reviewed source/); + assert.equal(existsSync(canonical), false); + assert.equal(readFileSync(invalid, "utf8"), "preserve root file\n"); + } finally { + rmSync(testHome, { recursive: true, force: true }); + } + } + } +}); + await test("stray OS metadata fails verification with a named remedy", () => { const testHome = mkdtempSync(join(tmpdir(), "machine-bootstrap-metadata-test-")); try { From 45b77cf0c9d0d71741786f0132ad1a22cf9bde5e Mon Sep 17 00:00:00 2001 From: Xinyi Lin Date: Tue, 8 Sep 2026 00:57:10 -0400 Subject: [PATCH 2/2] test(setup): accept portable root rejection diagnostics --- docs/provider-calibration-2026-09-07.md | 5 ++++- scripts/test-bootstrap.mjs | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/provider-calibration-2026-09-07.md b/docs/provider-calibration-2026-09-07.md index c2bbf7c..064ff2d 100644 --- a/docs/provider-calibration-2026-09-07.md +++ b/docs/provider-calibration-2026-09-07.md @@ -111,7 +111,10 @@ Provider documentation sources are linked in README. - Codex desktop: installed `mb_verifier` successfully delegated during review. Other desktop/CLI profile and nested-loading scenarios were not run. - Claude: native runtime loading/delegation was not exercised. -- Cross-platform CI: not run in this local task. +- Initial Linux/macOS CI passed. Windows rejected an obstructed root correctly + but exposed a platform-specific error-message assertion. The maintenance fix + accepts both valid rejection diagnostics while retaining acquisition and + preservation assertions. Final cross-platform results are recorded in the PR. - Independent review found and retested fixes for unreadable links through relocated Claude directories and file-obstructed ancestors. The final full diff review found no unresolved implementation or guidance defect. Its only diff --git a/scripts/test-bootstrap.mjs b/scripts/test-bootstrap.mjs index 5bb97f2..0b6e0c9 100644 --- a/scripts/test-bootstrap.mjs +++ b/scripts/test-bootstrap.mjs @@ -652,7 +652,9 @@ await test("provider roots beneath files fail preflight before acquisition", asy for (const args of [[], ["--check"], ["--preflight"]]) { const result = fixture.run(args, { [variable]: join(ancestor, "nested") }); assert.notEqual(result.status, 0); - assert.match(result.stderr, /configuration root cannot be read as a directory/); + // Windows can report ENOENT for the child and reach the file ancestor; + // POSIX reports ENOTDIR immediately. Both must reject before fetching. + assert.match(result.stderr, /configuration root (?:cannot be read as|must be) a directory/); assert.doesNotMatch(result.stderr, /ENOTDIR|node:internal/); assert.doesNotMatch(result.stdout, /Fetching reviewed source/); assert.equal(existsSync(canonical), false);