Skip to content

Harden the work-record guard: exit 2, self-test, mixed-case and .pi rules - #150

Merged
woksin merged 8 commits into
mainfrom
ci/work-record-guard-script
Oct 6, 2026
Merged

woksin merged 8 commits into
mainfrom
ci/work-record-guard-script

Conversation

@woksin

@woksin woksin commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Changed

@woksin woksin self-assigned this Oct 6, 2026
@woksin

woksin commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Reviewer notes

  • Deviation from the issue text: the checker is not a separate .github/scripts/verify-no-work-records.sh. A reusable workflow checks out the caller's repository, so a script file from this repository is not on the runner (the same constraint is documented in cleanup-pr-artifacts.yml). The program stays inline, supports --self-test, and .github/scripts/tests/verify-no-work-records.test.py extracts and runs the exact inline program, with a new verify-work-record-guard.yml gate.
  • Mixed-case detection covers ALL CAPS and Titlecase prefixes (PLAN-/Plan-, DESIGN-/Design-, REPORT-/Report-, STATUS-/Status-, anchored session names). Fully lowercase names are not flagged: replaying the rules over every non-archived Cratis repository tree found no new violation, while full case-insensitivity would turn Strategy's main red (governance/plan-coverage-audit.md). Widening it later is one regex.
  • Local checks: new python test (9 cases, includes --self-test and the VERIFY_SELF_TEST_BREAK=1 red control), node --test for the controller (22 pass), actionlint on all workflows, every .github/scripts/tests/*.test.py.
  • The controller's managed .gitignore block is applied for update runs only; a rollback leaves it alone.

@woksin

woksin commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

The checker is now extracted to .github/scripts/verify-no-work-records.mjs; the reusable workflow fetches the exact committed script by immutable SHA before inspecting the caller's checkout. The workflow checks the downloaded script's SHA-256 before execution. Offline tests compare that checksum with the tested source, inspect caller files and require download or checksum failures to exit 2. This avoids depending on the pinned commit's presence in a shallow or squash-merged checkout.

Local verification passed: all Python script suites (including the updater suite), both release-note/intent Node suites, subscription-controller fixtures, guard syntax, actionlint for the changed workflows and whitespace checks. Guard coverage includes arbitrary mixed-case prefixes, each rule's self-test and red control, empty/failed/missing-git listings, filename spaces/newlines and wrapper download failure. The initial broad updater run exceeded a 120-second tool limit; its bounded pi-phase rerun passed in 130 seconds.

The accidental generated Python bytecode is no longer in the PR. Its local file is preserved untracked; no prior-session work was discarded. Script changes need a new immutable pin and checksum, as documented in README. The live pinned download and checksum were also checked.

Awaiting the parent session's independent review before merge.

@woksin

woksin commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Review corrections are prepared locally, but are not pushed and not ready: the full Python gate could not acquire a pi-phase runner slot within 60 seconds, and the corrected Node gate could not acquire one within 120 seconds (both exit 75, commands never started). No CI run on the correction exists; green checks on the previous head do not validate it.

Finding-to-fix mapping:

  • Large Git listings: the checker explicitly allows up to 1 GiB, with a PATH-stub regression generating a >2 MiB NUL-delimited listing.
  • Ordinary prompt/session/handover documents: mixed-case session matching requires a clear separated session shape; product prompt.md, session.md and handover.md are clean fixtures. Legacy uppercase session names remain guarded.
  • Existing root exemptions: extra-allowed exempts both root-document and work-record-shape checks; compatibility fixtures cover PLAN, DESIGN, REPORT and STATUS, with and without .md. Session/runtime rules remain mandatory.
  • Strategy rollout: evidence/ and governance/ are default documentation directories, so existing callers need no new input at merge time. README documents the optional input's availability.
  • Pi trigger coverage: the bootstrap's encoded caller and its documented template now include .pi/** for PR and push paths. README tells existing callers to add that filter while preserving their settings; the reusable implementation alone cannot change their triggers. No downstream checkout was changed or fleet bootstrap executed.
  • Newly created .gitignore: staging selection includes modified and untracked paths, retains the allowed-path guard, and checks Git's listing exit status. The workflow-level regression applies the real controller to a subscriber without .gitignore, then executes the actual staging shell and confirms all three files are staged.
  • Violation annotations: Actions emits an escaped ::error title=rule::path for each violation, retaining log diagnostics.
  • Managed block idempotence: unmatched, nested, duplicate and reversed markers fail before writes; two-pass fixtures prove unrelated node_modules/ rules and subscription content remain unchanged.

The caller sweep also discovered a legitimate Direct product template (Source/Direct/Global/Work/Workers/Prompts/Templates/sections/report-progress.md). Product templates directories now exempt the shape rule, not session/runtime rules, and this path is a CLEAN regression.

Read-only caller sweep after those corrections: 54 local caller checkouts, 50 distinct repository origins, exercising both current Git indexes and local origin default-branch snapshots with each caller's inputs. All pass; no genuine work-record hits and no could-not-run results. Chronicle, Direct, Strategy and Eventmodelers-Build-Kits pass. Other repositories were not fetched or modified during the sweep.

The local script pin is 366135082754c74199b284163fb00b8e596cc424, SHA-256 4b0628fa06fb573f8b16f4398ded1c394f2b39294d6000bb1a65ce2711387c34; it will become remotely available in the one authorized push after the local gates pass.

Completed local checks: actionlint over all workflows, bootstrap Bash syntax, both controller/checker Node syntax checks, and git diff whitespace checks. The initial full Node run passed the new controller and malformed-marker regressions but found an existing template assertion needing the new .pi/** filter; that assertion is corrected locally, awaiting a successful full rerun. The full Python suite (including checker compatibility, large-listing and annotation regressions) has not run on this state. No live subscriber update can be exercised locally without the protected GitHub App credentials; workflow staging is covered with a disposable Git fixture instead.

Next: run full Python and Node gates when runner capacity is available, commit the remaining corrections, push once, and inspect immutable-head CI before READY. Do not merge.

…files, restore annotations and make managed-block planning idempotent (#89, #90, #91)
@woksin

woksin commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Review follow-up. All confirmed findings from the full review are addressed on this branch:

  • Large listings (critical): fixed in 7e1d15f, with a regression test over 1 MiB.
  • Session false positives: bare prompt.md, session.md and handover.md are ordinary product files again, and only explicit session prefixes match.
  • Caller compatibility: existing extra-allowed values keep their root exemptions, and evidence/ and governance/ are documentation by default, so Strategy needs no new input.
  • Profile subscription: a newly created .gitignore is staged.
  • Annotations: ::error:: annotations are restored for each violation.
  • Idempotence: pi-runtime managed-block planning is idempotent.

A read-only sweep of 54 caller checkouts (50 distinct repositories) found no new failures and no genuine work-record hits. Locally, all Python and Node test suites pass and actionlint is clean.

Bootstrap caller change removed from this PR. The review asked for .pi/** in the bootstrapped caller's path filters. .github/scripts/bootstrap-common-workflows.sh is a watched organization-wide write trigger, and bootstrap-package-update-safety.test.py deliberately fails when a rollout is bundled with other changes. That rollout is now its own issue, #151, and needs explicit approval. The README states what callers must add.

@woksin

woksin commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Full re-review (Opus 5.5 and GPT-6.1 Sol) confirmed all eight earlier findings are fixed. The pinned script at 3661350 is byte-identical to the head script, and nothing in this PR triggers organization-wide writes. The one remaining minor finding (the pin test only checked the commit format) is fixed: when the pinned commit is in the local history, the test now hashes that commit's script and compares it with script_sha256, so a changed script cannot ship with a stale pin.

@woksin
woksin merged commit 64c0c1d into main Oct 6, 2026
@cratis-direct
cratis-direct Bot deleted the ci/work-record-guard-script branch October 6, 2026 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant