fix(desktop): harden the renderer architecture ratchet base comparison

[chihumyum]

Summary

Three follow-ups to #4249's base-tree ratchet:

• CI uses --base <sha> --strict-base. Failure to materialize or analyze the base tree is an error instead of falling back to its committed ledger, which could reintroduce the stale-ledger failure demonstrated by CI red on main: "Check renderer architecture" fails from a stale renderer-architecture ledger #4250.
• When the checker changes, the base commit's generator measures both trees and the current checker compares those measurements. Under strict mode, an existing base checker that cannot be written, imported, or run, lacks the generator export, or produces an incompatible ledger shape fails the check. Local runs without the flag report these conditions and skip the cross-check.
• Real-Git CLI fixtures cover base-tree derivation, stale ledgers, weakened classification, schema transitions, missing exports, import failures, and non-writable script directories. The schema-transition fixture combines a ledger version bump, weaker classification, and a new legacy file; strict mode now rejects the change instead of returning success.

Refs #4582. renderer-architecture.json is unchanged.

Verification

Validated locally on Node 24 after rebasing onto main a49ba7544c3bdd1ef648ec90c69b8f39f3e881c0:

• Architecture fixture suite: 112/112, including 9 real-Git fixtures.
• CI planner/workflow/Windows-harness/workspace-runner suites: 132/132.
• node apps/desktop/scripts/check-renderer-architecture.mjs --base upstream/main --strict-base: passes with the base-checker cross-check exercised.
• npm run lint, npm run format:check, npm run build, npm run typecheck, and Knip for Desktop and UI: pass.
• npm run windows:inventory, npm run check:asf-headers, and git diff --check: pass.
• Before the follow-up fix, the schema-transition and missing-export regressions reproduced strict-mode false success, and the non-writable-directory regression reproduced lenient-mode failure. All three pass with the fix. The filesystem-permission fixture runs on non-root POSIX hosts and is skipped on Windows/root.

Review focus

• The cross-check protects measurement. Schema validation and comparison still belong to the current checker; changes to those rules require review. In particular, comparator-only changes to validateMonotonicDebt remain outside this PR's automatic protection, as agreed in the earlier scope correction.
• A base commit with no checker has no old measurement rules to execute and is skipped in both modes. An unchanged checker needs no cross-check. All incompatibility paths for an existing, changed checker fail under --strict-base; lenient mode retains notice-and-skip behavior.
• The temporary base checker is written beside the live script so its dependencies resolve from the current installation, then removed in finally. Dependency drift can therefore affect base measurements.
• The existing realpath-sensitive CLI entry guard remains unchanged; Git fixtures resolve their paths before invoking it.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code for the original implementation, tests, and validation; OpenAI Codex for the strict-mode compatibility and filesystem-error follow-up, regression tests, documentation, rebase, and validation. Materially AI-authored commits carry a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes: CI fails when base-tree derivation or the required base-checker cross-check cannot run, and rejects debt exposed by the base measurement rules. Lenient local runs report unavailable cross-checks and continue.
• No

[chihumyum]

The first CI run failed only in Storybook smoke, on the provider-settings.stories play; every other step passed. This PR changes only the architecture checker script, its tests, the CI step that invokes it, .gitignore, and the renderer README, so it cannot reach that story. Rebased onto current main (9225f80b0) to re-run CI; exact head 54bc30f0fde28541c4d7db2ebb123d39901a7e0f. The checker fixture suite (78/78) and check:renderer-architecture --base upstream/main (with the cross-check exercised, since the script differs from main) pass locally on this head.

Automated update from Claude Code on behalf of the PR author.

[chihumyum]

Rebased onto main 68cda0b54 to clear the conflict. Exact head: 87d1e1dd36ec00cb8cb845ebf9b4020be1d4b0db. The checker fixture suite (78/78), check:renderer-architecture --base upstream/main with the cross-check exercised, and the workflow policy tests (69/69) pass on it; #4643's Vite plugin change on main merged cleanly with this branch.

Posted by Claude Code on behalf of the PR author.

[chihumyum]

Rebased onto main 2c948e4e3 (after #4315). One conflict, in the renderer README: #4581 retired the same-count-replacement sentence, so the merged text keeps main's paragraph and appends this PR's --strict-base / cross-check paragraph after it. The combined checker fixture suite (#4315's controller-owner cases plus this PR's git fixtures) is 105/105; check:renderer-architecture --base upstream/main passes with the cross-check exercised; workflow policy tests 69/69. Exact head: 1c9023760fd3acca3f4f342286b426c9dc05ab81.

[chihumyum]

Rebased onto main e226ac532. The checker, its test file, and ci.yml had all moved on main since this branch's base (#4823 stopped pricing sanctioned imports, #4817 added the locale-hygiene ratchet, #4877 moved coverage out of the E2E tier); the rebase merged cleanly with no hand edits. Exact head: 9c3e285a9.

After the rebase, on Node 24:

• node --test scripts/check-renderer-architecture.test.mjs: 108 tests, 108 pass (the three --strict-base / cross-check git-fixture cases included).
• node scripts/check-renderer-architecture.mjs --base upstream/main --strict-base passes and prints the cross-check notice, since this branch changes the checker itself.
• The CI step still runs --base "$BASE_SHA" --strict-base after the merge with chore(ci): ratchet locale hygiene #4817's workflow changes.

Posted by Claude Code on behalf of the PR author.

[chihumyum]

@Astro-Han with #4491 and #4498 in, this is the last open item under #4582 from my side and it has never been put in front of you: three follow-ups to #4249's base-tree ratchet (--strict-base in CI so a base that cannot be materialized fails instead of falling back to the committed ledger, a cross-check under the base commit's checker when the script itself changes, and the git-fixture tests #4249 shipped without). Head 9c3e285a9 sits on main e226ac532 with no overlap with anything merged since; CI green, 108/108 fixture tests. Whenever you have time.

Posted by Claude Code on behalf of the PR author.

[chihumyum]

Rebased onto main 86404941d. One conflict, in ci.yml: #4913 moved the architecture step and replaced the inline BASE_SHA expression (and this branch's ^0+$ guard) with the shared comparison step, so the branch now keeps main's step verbatim and only appends --strict-base to the --base "$BASE_SHA" invocation — that flag is the whole ci.yml diff against main. #4917's hoisted-binding cache in the checker merged cleanly. Exact head: bba9df665; 108/108 fixture tests, and --base upstream/main --strict-base passes with the cross-check notice.

Posted by Claude Code on behalf of the PR author.

[chihumyum]

The CI failure at bba9df665 was scripts/ci-workflow-policy.test.mjs (shared comparison drives every diff gate…), which now pins the exact argv of every diff gate in ci.yml and still expected the architecture step without --strict-base. That expectation moves with the flag in the same commit, so the policy test now pins --base <sha> --strict-base as CI's invocation. Exact head: 81067b050; the four CI-planner suites pass 127/127 locally, lint clean, everything else unchanged from the previous head.

Posted by Claude Code on behalf of the PR author.

[Astro-Han]

Thanks for the strict base handling and real-git fixtures. The base materialization and CI wiring work, but the cross-check still trusts one part of the modified checker. A focused exact-head fixture reproduces false success; details inline.

中文

strict base 加载与 CI 接线成立，但交叉检查仍信任当前改动中的 comparator，定向 git fixture 已复现假绿。

AI-assisted review; the coordinating Codex agent independently reran the comparator-only counterexample.

[Astro-Han]

Thanks. Following the scope correction in the previous thread, this audit found no P0–P2 within the stated measurement/strict-base contract. The seven new Git fixtures plus an independent comparator probe passed. The Git fixtures exercise the real CLI/history boundary and are not redundant solely because they exceed the implementation in lines. The remaining inline item is documentation-only; no larger checker framework is requested.

中文

更正范围后，已声明契约内无 P0–P2。7 个新增 Git 用例加独立反例通过，真实 CLI/历史边界测试不能因行数多而删成 helper 自证。剩余行内项只改文档，不要求更大的检查器框架。

AI-assisted review using OpenAI Codex/Astra; evidence checked by the coordinating agent. This is not an independent human review.

[me2seeks]

Automated review (Command Code) — not an approval

This is a real hardening: --strict-base in CI, plus a base-checker cross-check so a PR cannot weaken the classifier and lower both sides of the ratchet at once. I reproduced that the new integration tests fail against origin/main's checker, and confirmed renderer-architecture.json is untouched, so no baseline was loosened. Two paths, though, silently bypass the guarantee the flag advertises.

P2 (Should-Fix) — the cross-check is fail-open under --strict-base when the head changes the ledger shape, so a new unclassified file can merge green.

The two "skip" returns do not fail closed, unlike the neighbouring paths:

// apps/desktop/scripts/check-renderer-architecture.mjs:3499-3501
if (typeof baseChecker.generateArchitectureConfig !== 'function') {
  return skip(`the checker at ${base} does not export generateArchitectureConfig`);
}

// :3511-3516
if (
  !validateArchitectureConfig(baseUnderBaseRules, 'base-checker base', shapeViolations) ||
  !validateArchitectureConfig(currentUnderBaseRules, 'base-checker current', shapeViolations)
) {
  return skip(`the checker at ${base} does not produce the current ledger shape (...)`);
}

and skip is unconditional:

// :3483-3486
const skip = (reason) => {
  console.log(`Renderer architecture check: ${reason}; skipping the base-checker cross-check.`);
  return [];
};

Compare unavailable just above it (:3470-3482), which does throw under --strict-base ("--strict-base forbids skipping the cross-check"). So "cannot be imported" and "cannot measure" fail closed, while "produces a shape I do not accept" silently disables the cross-check — precisely on the schema-tightening PRs where the cross-check matters most. Because both validations use the head's validateArchitectureConfig, any head-side version bump or new required field makes the base generator's output fail validation and the guard stand down.

I reproduced it: with a fixture where the head bumps the ledger version and weakens classification while adding a new legacy file, --base <base> --strict-base exits 0 with only "…does not produce the current ledger shape…; skipping the base-checker cross-check." The added file is never reported.

Smallest sound fix: route both skip calls through unavailable(...) when strictBase is set, or validate the base output with the base module's own schema rather than the head's.

P3 (Nice-to-have) — lenient mode now hard-fails on a non-writable script directory.

writeFileSync(tempPath, baseSource) (:3492) sits outside the unavailable/skip handling, so EACCES/EROFS propagates and the check exits 1 without --strict-base — which contradicts "local runs without it are unchanged" for read-only checkouts and sandboxes. Wrap the write and the import together and report unavailable.

P3 (Nice-to-have) — documentation overstates coverage. README.md describes a base that cannot be materialized or analyzed as a hard error and says the base checker "is also imported", but does not mention the two silent skip paths above, nor that the comparator and schema are still the head's (a comparator-only weakening is a documented residual, not closed by this PR).

Review-relevant risks. This is the desktop architecture gate; it changes what CI enforces. No product/runtime behavior change, no security or licensing effect identified in the diff. Gate-policy changes warrant maintainer review under CONTRIBUTING.md.

Required conclusion.

1. Optimal for the actual problem? Nearly. It genuinely hardens the measurement, but only the generator is taken from the base — the schema and comparator remain the head's, which is what leaves the two fail-open skips.
2. Production code that can be deleted? deriveBaseTreeConfig (:3419-3421) is now a single-call pass-through; inline it.
3. Low-quality tests to delete or replace? none identified — the new git-fixture tests are strong. The gap is the two untested fail-open paths above; add a fail-closed case for the shape/no-export skip.
4. Deeper refactor required? Yes, small and already scoped as a follow-up: run the base module's own validateArchitectureConfig and validateMonotonicDebt for the cross-check, which closes the comparator residual and makes the shape skip moot.
5. Ready to merge? Mergeable as a net hardening (no baseline loosening, tests fail without it), but the P2 skip should be fixed or explicitly documented first — right now it silently disables the new guard on exactly the PRs it exists to catch.
6. Residual risks / verification gaps: I did not run the repo suite; I ran the new tests from a /tmp mirror (7/7 pass) and the fail-open reproduction from a throwaway fixture, leaving the repo untouched. The base generator is executed against the head's node_modules, so dependency drift could change "base rules".

Approval boundary. This is automated review; it is not an approval. Per CONTRIBUTING.md, the merge decision requires an independent human review. No approve was submitted.

[chihumyum]

Addressed the strict-mode compatibility and filesystem-error findings in fa9b95c7a9d893a6f400f16e10c57adf2232ccd5, after rebasing onto main a49ba7544c3bdd1ef648ec90c69b8f39f3e881c0.

• Both compatibility skips now fail under --strict-base: a missing generator export and an output shape rejected by the current schema. Lenient mode retains its notice-and-skip behavior.
• Writing the temporary base checker now shares the import error handling, so a non-writable script directory fails in strict mode and reports/skips in lenient mode.
• The existing no-export Git fixture now checks both modes. The classification fixture also exercises a version bump plus hidden new legacy debt, and a real POSIX permission fixture covers the non-writable directory. These three scenarios reproduced the reported failures before the fix and pass afterward.

The README and PR description now spell out strict-mode failures, the no-checker exception, and the current-schema/current-comparator boundary. Following the earlier scope correction, comparator-only protection remains a follow-up under #4582; this change does not add a second checker framework or change comparison policy.

Local validation: architecture fixtures 112/112 (9 real-Git cases), the four CI planner suites 132/132, and the real --base upstream/main --strict-base invocation with the cross-check exercised. Lint, format check, full build, typecheck, Desktop/UI Knip, ASF headers, and diff checks pass. The permission fixture ran locally without a skip; it is skipped on Windows/root. renderer-architecture.json is unchanged.

Automated update by OpenAI Codex on behalf of the PR author.

[chihumyum]

Follow-up head: 26a6484654b1114c3afe23516cf1343d9d93c168. The first CI run failed at Check Windows test inventory because I omitted the generated inventory entry for the new POSIX permissions fixture. The entry and its platform-contract classification are now committed; npm run windows:inventory and the real permissions fixture both pass locally. The checker implementation is unchanged from fa9b95c7a.

Automated update by OpenAI Codex on behalf of the PR author.

[me2seeks]

Follow-up (Command Code) — verified resolved

I re-checked the head after your push.

The P2 fail-open is fixed. skip now routes to unavailable(...) when --strict-base is set, so a base checker that cannot produce the accepted ledger shape fails closed instead of disabling the cross-check with a console.log — which was the case I reproduced (a schema-tightening head plus a new unclassified file exiting 0).

Your follow-up commit also registers the POSIX checker permissions in the Windows inventory, which was not something I raised.

Thanks — the flag now means what it says. (Automated review; not an approval.)

[Astro-Han]

Thanks for the strict-mode follow-up and the scope correction. I rechecked 26a648465: base-tree derivation failures and unavailable/incompatible changed base checkers now fail under --strict-base; local lenient runs retain explicit fallback/skip diagnostics. The README correctly limits the cross-check to measurement, with comparison rules still owned by the current checker and reviewed separately.

The solution fits the demonstrated ratchet problem without adding a second comparator or ledger authority. Temporary tree/checker cleanup remains bounded to the invocation. No P0–P3 findings remain.

Verification: the exact-head architecture fixture and CI workflow-policy suites passed 155/155, including all nine real-Git fixtures and the non-writable POSIX directory case. Current CI is green.

AI-assisted rereview: Reviewer Luna independently checked strict-mode behavior and ownership; the coordinating Codex agent inspected the implementation and ran the scoped tests.