test(ci): define fail-closed docs-only partition contract - #282
test(ci): define fail-closed docs-only partition contract#282seonghobae wants to merge 43 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
left a comment
There was a problem hiding this comment.
Current-head owner-path review: the protected-main defect is causal and narrowly addressed. The previous workflow coupled exact-head Python repository/documentation contracts to the Rust toolchain job and independently scheduled full production coverage for prose-only deltas. This head separates those contracts into an always-executed exact-head job and gates only Rust-heavy jobs through a fail-closed exact changed-path classifier. The effective diff is three owner paths and does not alter central security workflows, rulesets, coverage thresholds, browser/policy authority, provider/model routing, or release semantics.
Keep Draft. This review is not approval and does not transfer predecessor evidence. #282 itself is code-bearing, so its unchanged head must execute the full CI path. Stacked docs-only #283 is the live acceptance canary for the lightweight branch and must actually execute before #279 can close. A queued/runnable-source state is not GREEN.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head authority review supersedes my predecessor review on 52285e5.... Protected-main AGENTS.md explicitly prohibits scheduled agents from altering workflows. The transient workflow mutation was therefore invalid for this writer and has been neutralized. Exact compare now shows no .github/** delta: only the fail-closed classifier candidate and RED regression contract remain.
This is a valid repair finding, not a close condition. Keep Draft and preserve the RED until an authorized workflow writer satisfies it. Do not count predecessor CI generated from the transient workflow tree. The dependent prose-only fixture #283 has been non-force-restacked on this head and remains one docs file only.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review after the authority repair and edge-coverage increment: effective diff remains two non-workflow files only. The classifier is conservative at the executable boundary—NUL framing, Unicode/space preservation, invalid UTF-8 and absolute/parent-relative rejection, empty evidence, nested Markdown outside docs/**, mixed/code-bearing changes, and exact CLI outputs are covered. Protected-main .github/workflows/ci.yml remains byte-identical on this branch.
The workflow-shape assertion is intentionally RED until an authorized non-scheduled workflow writer implements #279. Keep Draft; do not merge this test contract in a permanently failing state and do not treat queued 33795408882 jobs as hosted RED/GREEN. Stacked #283 current a066671a... remains a one-file docs canary and currently materializes both heavy lanes, which is direct trigger-shape evidence.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head finding: the proposed workflow contract still requires git diff --name-only -z. That representation is insufficient for a fail-closed docs-only partition because Git documents --name-only as the post-image filename only, while --name-status carries change status and, for rename/copy records, both names. A code-bearing rename such as crates/demo/src/lib.rs -> docs/lib.md is therefore observable as only docs/lib.md under --name-only -z and can be misclassified as prose-only. Local Git reproduction confirms exactly that shape. Repair the contract/parser to consume status-aware NUL records, classify both preimage and postimage for R/C, and reject malformed/unknown status records fail-closed. Keep .github/** unchanged in this scheduled-writer lane; the authorized workflow owner must later switch the workflow producer to the status-aware contract.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head re-review after the rename/copy repair: focused source verification is GREEN for status-aware docs-only A/M/D, code-to-doc R/C, docs-to-doc rename, malformed/truncated status, invalid UTF-8, and out-of-range similarity score. The classifier now preserves both R/C path identities and fails closed on unsupported status records. This does not make #282 merge-ready: the repository workflow is intentionally unchanged under the scheduled-writer authority boundary, so the workflow-shape contract remains RED until an authorized owner supplies --name-status -z (or equivalent lossless change records), keeps exact-head documentation contracts mandatory, and partitions only the Rust-heavy jobs.
seonghobae
left a comment
There was a problem hiding this comment.
현재 fail-closed 계약에 추가 결함이 있습니다. git diff --name-status -z의 정상 비어 있지 않은 스트림은 NUL로 종료되어야 하는데, parse_nul_name_status()와 legacy parse_nul_paths()는 마지막 NUL이 없어도 마지막 field/path를 완전한 레코드로 받아들입니다. 예를 들어 M\0docs/PRD.md는 뒤에 code-bearing record가 잘린 truncated stream일 수 있는데 현재 구현은 docs-only로 분류합니다. 이는 변경 경로 evidence가 불완전한 상황에서 Rust-heavy 검증을 건너뛸 수 있는 fail-open입니다. 비어 있지 않은 status/path stream은 trailing NUL을 필수로 하고, 누락 시 output 없이 non-zero/fail-closed 해야 합니다. workflow 변경 없이 classifier/test owner 범위에서 수리 가능합니다.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head 재검증: classifier/test 두 파일만 변경된 상태에서 non-empty -z 입력은 terminal NUL을 요구하고, docs-only 정상 입력은 lightweight로 유지되며 code→docs rename은 양쪽 path를 보존해 Rust-heavy로 분류되고, M\0docs/PRD.md처럼 terminal NUL이 없는 입력은 scope output 없이 fail closed하도록 source/test 계약이 일치합니다. .github/** 변경은 없습니다. 다만 workflow 연결 자체는 이 scheduled writer의 권한 밖이며, repository-wide GREEN이나 #279 완료를 주장하지 않습니다.
seonghobae
left a comment
There was a problem hiding this comment.
Current classifier still has a mode/type authority gap. git diff --name-status -z preserves status and paths but not src/dst modes. A docs-path symlink edit is emitted as M\0docs/guide.md\0, indistinguishable from an ordinary prose-file edit, and a regular-file→symlink conversion is T\0docs/guide.md\0; the current parser accepts both and classify_paths() returns documentation-only for the docs path. That is fail-open evidence for a future Rust-skip decision because the change is not proven to be an ordinary documentation blob. Git's raw diff format carries src/dst modes specifically enough to distinguish regular blobs from symlinks (120000), gitlinks (160000), executable blobs (100755), deletions/creations (000000), and type changes. Add a RED contract showing mode-loss, then make lightweight classification require raw, NUL-framed evidence whose materialized modes are ordinary non-executable blobs (100644, with 000000 only on the absent side). Preserve rename/copy preimages and fail closed on malformed/unsupported raw records. Do not change .github/** in this scheduled lane.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head verification after the mode-authority repair: an actual Git repository converting docs/guide.md from mode 100644 to a symlink produces --name-status -z as T\0docs/guide.md\0 but --raw -z as :100644 120000 <src> <dst> T\0docs/guide.md\0. The repaired raw parser retains those modes and classifies that change Rust-heavy. Focused reconstructed cases also classify ordinary 100644 docs edits/adds and docs→docs renames as lightweight, while 120000 symlink edits, 160000 gitlinks, 100755 executable blobs, type changes, and code→docs renames are Rust-heavy. Legacy name/status evidence has no lightweight authority. Repository-wide GREEN is not claimed: protected-main CI has not yet adopted the raw producer/job partition, and the workflow-shape contract remains intentionally RED. No approval.
seonghobae
left a comment
There was a problem hiding this comment.
Fresh finding: the raw-diff parser validates object-ID syntax but discards object-ID semantics. Git's raw format couples absence modes with all-zero object IDs (e.g. A: src mode/OID both zero; D: dst mode/OID both zero). The current parser accepts impossible metadata such as :000000 100644 1111111 2222222 A\0README.md\0 and then grants documentation_only=true, so malformed/corrupted raw evidence can still authorize the lightweight lane. Add a test-first invariant that absent sides require all-zero OIDs and, for the intended two-commit producer, materialized sides require non-zero OIDs; preserve the OIDs in RawChange or validate before discarding them. Keep the workflow owner boundary unchanged.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review after object-identity repair. Test-only ccbf987dd10b50ffafd8a0766917afaaeca1aed7 captures impossible raw records where mode absence and object-id absence disagree; causal 75d1432e8a0d2f83ed8780bc0a7f2300cee7bdc8 now rejects mismatched mode/OID presence, unequal abbreviated OID widths, and non-lowercase/non-hex object IDs before scope classification. Focused boundary verification confirms malformed A/D/M examples fail while canonical two-tree A/D/M examples remain accepted. This is source-level verification only: exact-head hosted workflows are still non-terminal, and .github/** remains unchanged.
seonghobae
left a comment
There was a problem hiding this comment.
Repair finding: the raw-diff parser currently accepts R100/C100 records whose two path fields are identical. That cannot represent a rename/copy emitted by the intended exact two-tree Git producer: a real rename/copy has distinct source and destination pathnames (local git diff --raw -z reproduction yields R100\0docs/a.md\0docs/b.md\0). Because both identical paths can satisfy the Markdown prose predicate, malformed evidence could authorize rust_required=false. Add a RED regression for identical R/C paths, then reject those records before scope classification. Keep the PR Draft; this is classifier contract repair, not workflow GREEN.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head verification for the rename/copy path-identity repair. Test-only 24ff89d833fe8cc7deb711036e7bd62671232866 adds a distinct-path positive control plus identical-path R100/C100 rejection. Causal 06d04b786fc470f821cf2e91c6546cb53d488783 adds only a parser-side guard after canonical path decoding: two-path raw records must carry distinct path identities. This preserves normal docs-to-doc rename/copy eligibility while malformed identical-path evidence fails before scope classification. Hosted CI is not GREEN: run 33847111611 still materializes queued Rust contracts 100941336817 and Production coverage 100941336940 with no runner/steps. Keep Draft.
Signed-off-by: Seongho Bae <me@seonghobae.me>
Preserve the existing canary blob while adopting the latest #282 parent without force-pushing or destructive rebasing. Signed-off-by: Seongho Bae <me@seonghobae.me>
seonghobae
left a comment
There was a problem hiding this comment.
Finding: complete OID widths are now enforced, but R/C similarity metadata is still not bound to blob identity. Git defines the R/C score as similarity and reserves 100% for equal content. Current parser therefore accepts impossible evidence such as R100/C100 with different source/destination blob OIDs, and R074/C074 with identical OIDs; either form can currently authorize (documentation_only=true, rust_required=false) for ordinary Markdown paths. Test-only head b2a120f892973e76c0ea0f06e7105bdf7a268009 adds both fail-closed regressions without changing production. Exact hosted CI is still queued, so this is source/test RED evidence only; do not claim runner RED yet. Minimal causal repair belongs in the existing raw object-identity semantics and must not widen workflow authority.
Adopt the test-first #282 parent while preserving the existing prose-only canary and non-force lineage. Signed-off-by: Seongho Bae <me@seonghobae.me>
Adopt the repaired #282 classifier while preserving the one-file canary and non-force lineage. Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Adopt the complete positive/negative #282 boundary coverage while preserving the one-file canary and non-force lineage. Signed-off-by: Seongho Bae <me@seonghobae.me>
Adopt the concurrent #282 fixture reconciliation while preserving the one-file canary and non-force lineage. Signed-off-by: Seongho Bae <me@seonghobae.me>
seonghobae
left a comment
There was a problem hiding this comment.
Current-head verification: e3a40a4f78a3fbfc751ab9efad321b8207fb43e5 preserves the causal similarity/OID repair from 8f885aba..., the negative impossible-evidence regressions, the positive R100/C100 and sub-100 identity boundaries, and the concurrent fixture alignment from a33f1daf.... The two-parent reconciliation is valid intervening-delta adoption, not a race. Exact CI/security/SAST/CodeQL remain non-terminal, so this is source-level verification only and the PR must remain Draft. CHANGELOG still lacks a separate entry for the similarity/OID binding.
Bind the classifier security invariant to the release record without weakening the workflow-owner RED. Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Exact-head CI failure RCA: |
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Exact-head review found a trust-boundary defect in predecessor |
seonghobae
left a comment
There was a problem hiding this comment.
Current-head repair finding: the effective diff now contains .github/workflows/ci.yml plus the classifier/CHANGELOG/test contract. That contradicts the older workflow-clean description and crosses the scheduled-writer authority boundary in protected-main AGENTS.md. Keep the useful classifier/test evidence, but do not merge this generation as scheduled-writer-owned work. The #279 authorized workflow owner must adopt or reconstruct the .github/** portion while preserving the exact raw-diff fail-closed contract. Current CI/Security/SAST/CodeQL runs are queued, so no exact-head GREEN is claimed.
…ase-classifier Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Restacked on protected main and fixed the draft-CI integration at exact head b54a585. The scope, documentation-contract, Rust, and coverage jobs now share the draft/closed guard; Rust and coverage retain the fail-closed scope condition without duplicate YAML keys. Local evidence: actionlint passed; 206 Python repository contracts passed; cargo fmt, workspace tests, strict Clippy, and rustdoc passed; fresh-worktree production function/line/region/branch coverage is 100%. This PR remains draft, so GitHub jobs are expected to skip under #286 until it is marked ready. |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head workflow-owner review on b64e0708584beff3fb54acf226cb3e667773e473: the latest one-commit movement is not merely stale metadata. It changes .github/workflows/ci.yml and tests/test_repository_contract.py so the branch now explicitly removes converted_to_draft and closed from the protected #286 lifecycle and removes the github.event.action != 'closed' guard; the test positively asserts that those protections are absent.
Protected main@87c4daa1830bac5a5228b6036752ad5633232085 currently requires [opened, synchronize, reopened, ready_for_review, converted_to_draft, closed] and a closed+Draft fail-closed guard. The docs-only classifier/contract partition is valid work, but this lifecycle delta would regress current protected capacity/cleanup semantics if adopted verbatim.
Keep Draft. The authorized #279 workflow-owner reconstruction must preserve #286's converted_to_draft/closed triggers and closed-event guard while adding the trusted-base classifier, repository/documentation-contract lane, and Rust/coverage partition. Update the focused repository contract to require—not forbid—the protected lifecycle controls. Do not treat local predecessor-relative tests as current-protected compatibility evidence.
Test-first classifier/workflow candidate for #279. Keep Draft; valid delta is retained, but the current workflow generation must not be promoted verbatim.
Live topology
Protected
mainis exact87c4daa1830bac5a5228b6036752ad5633232085(#286). Current #282 exact head isb64e0708584beff3fb54acf226cb3e667773e473, open/Draft/mergeable on protected main. The effective delta still includes.github/workflows/ci.yml, so this scheduled OriginWeave product writer does not own or mutate it.#287 remains the workflow-free classifier prerequisite at exact
af83c40dd2990a03064a92ca75430a9cc400f098. The trusted direction remains correct: a docs-only suppression decision must execute the classifier blob from the protected base SHA and fail closed to full Rust verification when that protected classifier does not exist.New protected-generation regression finding
The latest #282 movement from
b54a5856...to currentb64e0708...is one workflow/test commit. It now removesconverted_to_draftandclosedfrom the CI PR event list and removes protected #286'sgithub.event.action != 'closed'guard.tests/test_repository_contract.pywas changed to positively require those removals.That is incompatible with current protected main. #286 intentionally carries
[opened, synchronize, reopened, ready_for_review, converted_to_draft, closed]plus a closed+Draft fail-closed job guard so lifecycle transitions do not leave incorrect hosted-job behavior. The docs-only classifier/contract partition remains valid, but the current lifecycle delta is a repair finding, not a Close condition.Required canonical-owner reconstruction
converted_to_draftandclosedlifecycle events and the closed-event fail-closed guard from protected ci: stop draft PR jobs from occupying runners #286;Exact-head review
5120043505records this finding. Do not source suppression authority from mutable PR-head code, replace it withpaths-ignore, or weaken Draft/closed lifecycle coverage.No self-approval, bypass, force-push, destructive rebase, scheduled-writer workflow/ruleset/secret mutation, gate weakening, tag, release, or publication is authorized.