[PR] ci: repair three Windows lane contracts [maka]

["Astro-Han (via GitHub)" <gi...@apache.org>]

Astro-Han opened a new pull request, #3317:
URL: https://github.com/apache/maka/pull/3317

   ## Summary
   
   Three CI contracts that were stated somewhere but enforced nowhere, all on the Windows lanes.
   
   **The W0 sandbox lane billed a Windows runner to changes it cannot observe.** Its `paths` filter had grown to `packages/runtime/**` plus `packages/core/**`, which put a windows-2025 job on the pull request for 21 of the last 60 first-parent commits; exactly 1 of those touched anything the lane observes. It had no concurrency group either, so every push started another one.
   
   The filter grew that way because it was asked to be a correctness claim, and it cannot be one: the lane's real input is the import closure of `packages/runtime/src/sandbox` and `packages/runtime/src/filesystem-worker`, which reaches 9 files at the top of `packages/runtime/src` and 5 `@maka/core` modules, inside source roots holding more than 130 top-level files each. No hand-written list is that closure, and one that tried would drift silently. So the filter now names the directories that own the sandbox — still blocking before merge on a change there — and a nightly run covers the transitive edits it cannot match, after they land. Three packaging inputs came out of the list at the same time; no step in the lane reads them, and `release-windows-check` already owns the packaged sandbox.
   
   The lane stays separate from `windows-baseline` on purpose: baseline sets `continue-on-error` at the job level and on each diagnostic step because it carries a known Phase 1 backlog, so folding these assertions in would trade a live regression signal for one fewer file.
   
   **The Windows inventory check could not fail.** `npm run windows:inventory` is a regenerate-and-diff gate that carried `continue-on-error: true`, so a drifted inventory reported a green run and merged. It now runs unconditionally beside `Test CI planner` and can fail the job — it needs only the checkout, running on Node's built-ins in about a third of a second without `npm ci`. Gating it on the `code` surface like the Astryx checks was the obvious-looking alternative, but those are gated because they are expensive; copying the shape without the reason would have required a planner special case for a generated file under `docs/`, and made an inventory-only pull request pay for install, lint, build, typecheck and knip to run a sub-second diff.
   
   **Two Windows lanes persisted the job credential.** Ten workflows check out the repository and eight disable `persist-credentials`. `windows-baseline` and `windows-recovery` did not, and both then execute `npm ci` with lifecycle scripts, the desktop smoke, and the full storage and runtime suites. Neither pushes, calls `gh`, or reads a token.
   
   Each contract is now enforced across the whole workflow directory rather than on the files being fixed. The trigger allowlist reads each workflow's `on:` block instead of naming lanes that must stay off pull requests — the previous version named three and omitted W0, which is how that lane kept an unbounded trigger through a CI reduction that touched every workflow around it.
   
   ## Verification
   
   - `node --test --test-concurrency=1 scripts/ci-test-plan.test.mjs` — 21 passing
   - `npm run windows:inventory` — exit 0, inventory current at 62 declarations, so making it blocking fails nothing that passes today
   - `actionlint .github/workflows/*.yml` — clean
   - `npx biome check scripts/` — clean
   - Not run: repository-wide suites and typecheck. No TypeScript or product code changes here; CI covers the rest.
   
   Every added contract was verified in both directions — it passes on this branch, and it fails when the property it claims is removed:
   
   | Contract | Injected regression | Result |
   |---|---|---|
   | Trigger allowlist | rewrote a lane as `on: [pull_request]` | fails — the flow-style form the previous regex would have missed |
   | Trigger allowlist | restored W0's unbounded `pull_request` | fails |
   | Path filter paired with nightly | deleted the `schedule` | fails |
   | Path filter paired with nightly | deleted the `paths` | fails |
   | Contract steps can fail | restored `continue-on-error` on the inventory step | fails |
   | Credential hardening | removed one `persist-credentials: false` | fails |
   
   The credential assertion slices each checkout step rather than counting strings file-wide, so a bare checkout cannot be balanced out by a sibling step that opts out, or by the string appearing in a comment. Both failure modes were reproduced against the counting version before the change.
   
   ## Review focus
   
   The judgement call worth a second opinion is the W0 filter. It is deliberately **not** the lane's dependency closure, so a change to a transitive dependency — `packages/runtime/src/path-containment.ts` or `packages/core/src/windows-path.ts`, for example — is caught by the nightly run after it merges, not before. That is the trade being made: pre-merge coverage for the sources that own the sandbox, post-merge coverage for everything the filter cannot express. If pre-merge coverage of the full closure is required, this is the wrong shape and the planner would have to own the decision instead.
   
   ## AI use
   
   - [ ] No generative tool made a substantive contribution
   - [x] Generative tooling made a substantive contribution
   
   Tool(s) and scope: Claude Code authored the workflow and test changes and the commit messages; commits carry `Generated-by: Claude Code`. The design was revised twice under adversarial review by Claude (Fable) and Codex, which independently verified the statistics and the ownership claims above; the inventory fix in particular was reduced to its current form because that review showed the first attempt fixed it at the wrong layer. The human contributor reviews the final diff and owns the merge decision.
   
   ## Checklist
   
   - [x] Tests cover the change and fail without it
   - [x] Lint, format, typecheck and the affected suites pass locally
   
   Does this PR entail a change in behavior?
   
   - [x] Yes — described under Summary above
   - [ ] No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org

Re: [PR] ci: repair three Windows lane contracts [maka]

["Astro-Han (via GitHub)" <gi...@apache.org>]

Astro-Han merged PR #3317:
URL: https://github.com/apache/maka/pull/3317


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org