fix(rm-03): make CI queue guard fail on asserted non-readiness (#1032)
Co-authored-by: coder-mos1 <[email protected]>
This commit was merged in pull request #1032.
This commit is contained in:
@@ -0,0 +1,120 @@
|
||||
# RM-03 — CI Queue Guard Repair
|
||||
|
||||
- **Task:** RM-03
|
||||
- **Issue:** #1019
|
||||
- **Branch:** `fix/rm-03-queue-guard`
|
||||
- **Owner:** coder-mos1
|
||||
- **Reviewer:** rev-974 (independent; author != reviewer)
|
||||
- **Started:** 2026-08-01
|
||||
|
||||
## Objective
|
||||
|
||||
Repair the mandatory CI queue guard so it reads provider payloads, blocks asserted non-green CI, distinguishes provider unavailability from a real non-green result, and inspects the branch actually being pushed or merged.
|
||||
|
||||
## Constraints
|
||||
|
||||
- Worktree only: `/home/hermes/agent-work/rm-03`; never mutate `/src/mosaic-stack`.
|
||||
- JSON payload travels through stdin; never argv. Large payload must remain below no ARG_MAX dependency.
|
||||
- TDD is mandatory. Every behavior case must be observed red before implementation.
|
||||
- No bypass flags or hook suppression.
|
||||
- Do not cite the existing guard's green as evidence; D-23 establishes it is zero-information.
|
||||
- Gate-ready is a frozen exact head. Any push after a merge-gate verdict voids that verdict.
|
||||
- No merge: coordinator holds the merge hand pending Jason.
|
||||
|
||||
## Design
|
||||
|
||||
1. Feed JSON to `python3 -c` on stdin, including pending-context rendering.
|
||||
2. Classify valid green as `READY`; pending/failure/no-status/malformed/mixed as `ASSERTED_NOT_READY`; provider/credential/transport inability as `CANNOT_ASSERT`.
|
||||
3. `ASSERTED_NOT_READY` exits nonzero. `CANNOT_ASSERT` emits a loud diagnostic and appends a local JSONL audit record. Push degrades to exit 0; merge holds with distinct retryable exit 75 until provider recovery, then self-clears without manual reset. Inability to write the audit exits nonzero.
|
||||
4. Derive the current branch when `-B` is omitted. The merge wrapper passes the exact PR head branch, repository, and full commit SHA—not its `main` base—so fork PRs cannot resolve against an adjacent base-repository branch.
|
||||
|
||||
## Test matrix
|
||||
|
||||
| Case | Required outcome |
|
||||
| --- | --- |
|
||||
| success | exit 0; terminal-success |
|
||||
| pending | nonzero after bounded timeout |
|
||||
| failure | nonzero |
|
||||
| no-status | nonzero |
|
||||
| malformed | nonzero |
|
||||
| >=150 KiB payload | unchanged classification; never rc126 |
|
||||
| provider unreachable on push | loud audited CANNOT_ASSERT; degraded exit 0 |
|
||||
| provider unreachable on merge | loud audited CANNOT_ASSERT; retryable exit 75/HOLD |
|
||||
| audit unavailable | nonzero |
|
||||
| implicit push branch | provider URL uses checked-out feature branch |
|
||||
| merge wrapper | queue guard receives exact PR head branch/repository/full SHA |
|
||||
|
||||
## RED-first evidence
|
||||
|
||||
Observed against the unmodified `origin/main` implementation before source edits:
|
||||
|
||||
- `bash packages/mosaic/framework/tools/git/test-ci-queue-wait-tristate.sh` → rc 1 with 15 failed assertions.
|
||||
- Success payload was reported `state=unknown`.
|
||||
- Pending, failure, no-status, and malformed payloads each exited 0 and omitted `ASSERTED_NOT_READY`.
|
||||
- The 160 KiB payload produced rc 141 because Python never consumed the pipe; it did not classify success.
|
||||
- Provider-unreachable exited 7 with no `CANNOT_ASSERT` audit record.
|
||||
- Implicit push queried `/branches/main`, not `/branches/fix/rm-03-fixture`.
|
||||
- Audit-unavailable emitted no audit diagnostic.
|
||||
- A credential-resolution hard-block mutant was then run before trusting that added case: `credential-unresolvable` returned rc 1 and omitted `CANNOT_ASSERT`; the matrix returned rc 1 with two named assertion failures.
|
||||
- Review-blocker controls were observed red: structurally invalid `statuses` string and null-entry payloads each exited 0 as `terminal-success`; unsupported-platform discovery exited 1 without diagnostic or audit (seven named assertion failures total).
|
||||
- After the push/merge asymmetry ruling, merge-side provider unavailability was observed red at rc 0; its registered case required distinct retryable rc 75.
|
||||
- Aggregate `state=success` with zero contexts was observed red: it exited 0 as `terminal-success`; the registered case requires `no-status`/nonzero.
|
||||
- Fork/exact-head controls were observed red: `pr-merge.sh` omitted the fork repository and full SHA, and an ignored-arguments mutant re-resolved through `/branches/` instead of the exact fork commit (two named failures).
|
||||
- GitHub check-run-only success/pending/failure were each misclassified as `no-status`; the RED run had five named failures and proved the Checks API was never queried.
|
||||
- The first merge-pin control was unrunnable because one `local` declaration referenced a variable before assignment under `set -u`; this was disclosed and corrected rather than counted. The runnable RED then showed Gitea payload `{"Do":"squash"}` lacked `head_commit_id`; a separate GitHub run showed `gh pr merge 123 --squash` lacked `--match-head-commit`.
|
||||
- A stale-verdict mutant removed the `--expect-head` comparison and was observed red because a moved head reached the provider merge call.
|
||||
- `bash packages/mosaic/framework/tools/git/test-pr-merge-queue-branch.sh` initially returned rc 1; captured call was `--purpose merge -B main -t 900 -i 15`.
|
||||
|
||||
Logs remain untracked under the worktree as `.mosaic-test-work-red-*.log` and will not be committed.
|
||||
|
||||
## Progress
|
||||
|
||||
- [x] Mission, remediation charter, task evidence, board, issue #1019, and superseded PR #1023 read.
|
||||
- [x] Isolated worktree created and identity configured coherently.
|
||||
- [x] Mutant tests authored and observed red.
|
||||
- [x] Implementation green.
|
||||
- [x] Baseline and focused situational gates green; full package suite has an unrelated framework-shell environment abort recorded below.
|
||||
- [ ] Independent review clean (rev-974 requested changes at `44ffa99a`; bypass remediation committed and awaiting re-review).
|
||||
- [ ] PR CI terminal-green at exact head by full step scan.
|
||||
- [ ] Merge-gate verdict issued against frozen head.
|
||||
|
||||
## Scope disposition
|
||||
|
||||
- The five framework guides are consequential documentation: they define the purpose-aware tri-state contract, including audited push degradation and merge HOLD.
|
||||
- The agent templates are consequential because they ship the same queue-guard instructions into newly seeded agent contracts; leaving them binary/stale would contradict the repaired tool.
|
||||
- `pr-merge.sh` is consequential: it must inspect the PR's exact head branch/repository/SHA and enforce the exact-head merge pin.
|
||||
- `pr-metadata.sh` is consequential only as the normalized source of that head branch/repository/SHA. Its diff is limited to exposing those fields on GitHub and Gitea.
|
||||
- `test-pr-merge-gitea-empty-uid.sh` changes because exact-head Gitea merges now always use the API path (the only path that can send `head_commit_id`), superseding the prior tea-empty-identity fallback behavior.
|
||||
|
||||
## Review remediation
|
||||
|
||||
- rev-974 independently proved that the documented `--skip-queue-guard` merge option bypassed an exit-99 guard stub, reached the provider merge payload, printed success, and exited 0 at head `44ffa99a`.
|
||||
- RED-first reproduction was added to `test-pr-merge-head-pin.sh` before the production fix: `FAIL merge-bypass: --skip-queue-guard reached the provider merge path`, suite rc 1. The test-only commit is `241113e6`.
|
||||
- Production remediation `37aae650` removes the option from parsing, usage, help, and examples. Every merge-capable path now invokes the queue guard; `--dry-run` alone omits it and has a regression proving that it exits before provider dispatch and creates no merge payload.
|
||||
- Existing Gitea merge tests now exercise a successful guard response rather than bypassing the guard.
|
||||
|
||||
## Risks / boundaries
|
||||
|
||||
- The local JSONL audit is durable operational evidence but not tamper-resistant against the same UID. RM-03 does not claim otherwise.
|
||||
- Push-side audited exit 0 is an explicit owner ruling (Option B), accepted to avoid bricking recovery work; merge-side CANNOT_ASSERT remains retryable exit 75/HOLD. The automated security reviewer continues to flag the deliberate push availability tradeoff.
|
||||
- Source/deployed-copy equality is owned by RM-02/D-22; this branch changes repository source and its tests only.
|
||||
|
||||
## Test evidence
|
||||
|
||||
Fresh after rescue checkpoint `b7175012`:
|
||||
|
||||
- Focused situational matrix: tri-state, GitHub checks pagination, branch-absent, merge head branch/repository/SHA, exact-head pin, and Gitea exact-head API regressions all passed.
|
||||
- `bash -n` on the three production shell scripts passed.
|
||||
- `shellcheck -x -P packages/mosaic/framework/tools/git ...` on all changed shell scripts passed.
|
||||
- `pnpm typecheck` passed (45/45 Turbo tasks).
|
||||
- `pnpm lint` passed (25/25 Turbo tasks).
|
||||
- `pnpm format:check` passed.
|
||||
- `pnpm --filter @mosaicstack/mosaic test`: Vitest passed 1508/1508 on the confirmation run; framework-shell then aborted at the pre-existing wake coordinate assertion with exit 97: `BASH_LINENO ... probe reported [3 5], expected [3 4] ... (#973)`. This is outside the RM-03 diff and is disclosed rather than substituted or called green.
|
||||
- The prior package-suite attempt had one transient, out-of-diff `install-ordering-guard.spec.ts` failure (1/1508); its isolated rerun passed 19/19 and the confirmation full Vitest run passed 1508/1508.
|
||||
- After bypass remediation: all six focused RM-03 queue/merge regressions passed, including bypass refusal and dry-run non-dispatch; shell syntax and source-aware ShellCheck passed; `pnpm typecheck`, `pnpm lint`, and `pnpm format:check` passed.
|
||||
- Fresh `test:framework-shell` reached and passed every RM-03 test, then again aborted at the unrelated wake coordinate assertion with exit 97; it remains explicitly non-green rather than substituted.
|
||||
- An ad hoc raw Prettier invocation over `.template` and `.sh` files was unrunnable because no parser is registered for those extensions; it was not used as a substitute for canonical `pnpm format:check`.
|
||||
|
||||
## Final evidence
|
||||
|
||||
Pending.
|
||||
Reference in New Issue
Block a user