Follow-up: Fail closed on six reconciled workflow-pack contracts

[justin808]

Context

React on Rails PR #4749 reconciles its vendored agent-workflow seam to immutable shakacode/agent-workflows commit 1958648b70a450aa67c15b833428485d17021045. Review and replay found six fail-open or ambiguous contracts that belong upstream. The consumer sync did not introduce them and should keep pack-identical helpers byte-identical instead of adding consumer-only overlays.

Problems and expected behavior

1. Unknown pr-ci-readiness buckets are accepted (script)

skills/pr-batch/bin/pr-ci-readiness treats an unrecognized check-row bucket as a successful row. A row such as {"workflow":"CI","name":"mystery","bucket":"future-state"} currently produces READY when it is the only required row. Missing or non-object row fields are likewise not validated before readiness classification.

Expected: fail closed when a fetched check row is structurally invalid or has an unknown bucket. Only explicitly recognized successful/skipped buckets may contribute to READY, and the result must explain why readiness is UNKNOWN or NOT_READY.

Reproduction:

PrCiReadiness.assess(
  pr_number: 1,
  required_used: true,
  rows: [{ "workflow" => "CI", "name" => "mystery", "bucket" => "future-state" }]
)
# Current verdict: READY

This behavior was reproduced against old consumer pin 4bb0c1e4944276e271799fd179e0c166b54ef4dc and the reconciled pack line.

2. Trust-role values accept malformed types (schema)

agent-workflow-seam-doctor normalizes trusted_bots and trusted_metadata_bots with Array(value) and to_s. Hashes, numbers, mixed arrays, and empty entries therefore become apparent logins instead of being rejected. A numeric scalar such as 42 can become the login "42".

Expected: accept a legacy nonempty string or an array containing only nonempty strings, then apply the existing normalization and cross-role overlap check. Reject every other shape before normalization. Cover the same schema at all trust-config consumption seams.

3. status: not_applicable can satisfy required priority evidence (schema)

closeout-evidence-replay --require-priority-dispositions rejects an empty status: not_applicable marker, but the same marker with a valid P1 finding returns SATISFIED.

Observed at the reconciled head:

overall_verdict: SATISFIED
priority verdict: SATISFIED
status: not_applicable
finding count: 1
missing: []
errors: []

Expected: the status and finding shape must be internally consistent. Required-mode priority evidence containing status: not_applicable must never produce SATISFIED, including when a syntactically valid finding is appended.

4. Fresh dispatch approval persists stale refresh provenance (script)

When a persisted dispatch-decision-refresh is followed by a fresh dispatch-decision, selection uses the new authority but history_resolution retains the old refresh resolution.

Observed:

selected status: selected
selected authority: {dispatch: false, route: true}
persisted action: dispatch-decision-refresh
persisted decision id: refresh-before-dispatch
replay without transient operator_decision: blocked-user-input

Expected: the fresh dispatch decision supersedes the prior refresh resolution, persists its decision id/action/updated authority, and replays to the same selected assignment without transient operator input.

5. Explicit GH_HOST does not verify the resolved repository URL host (script)

pr-security-preflight requests both nameWithOwner and url, but its explicit-GH_HOST branch validates only the slug and then assigns the expected host without parsing the returned URL. A same-slug response from a different host can therefore be accepted and misclassify repository-local trust state.

Expected: canonicalize and compare both the returned slug and URL host with the requested repository and explicit GH_HOST; fail closed on either mismatch. Add a same-slug/different-host regression.

6. Completed-audit refs and OUTSTANDING comma fallback are ambiguous (schema)

The completed-batch-audit v1 grammar reserves ; and | inside record refs but permits commas, while findings: OUTSTANDING ... uses comma splitting as a fallback. A multi-ref findings value containing a valid comma-bearing record ref cannot be parsed into a unique blocker identity.

Expected: make the grammar unambiguous everywhere it is documented, parsed, mirrored, and tested. One acceptable mechanism is reserving commas in refs; another is removing comma fallback in favor of an unambiguous delimiter. Preserve canonical ref normalization and blocker-union completeness.

Acceptance coverage

• Unknown, missing, null, and non-object readiness rows fail closed; existing pass, skip, pending, fail, and cancel behavior remains covered.
• Trust config accepts only legacy nonempty scalar strings or arrays of nonempty strings and rejects malformed/mixed shapes before normalization.
• Required priority evidence cannot be satisfied by status: not_applicable, with or without findings.
• Refresh-then-dispatch persists the fresh approval and restart replay succeeds without transient operator state.
• Explicit-host security preflight rejects same-slug payloads whose repository URL resolves to another host.
• Completed-audit parsing has one documented and tested, non-ambiguous ref grammar across pack copies.
• Each regression is demonstrated red before the fix and green after it against the immutable pack source.

Mechanism disposition

• Targets: script for readiness, dispatcher persistence, and host verification; schema for trust roles, priority evidence consistency, and completed-audit refs.
• Motivating miss: machine-checkable readiness, trust, replay, and blocker identities currently accept unknown, contradictory, stale, or ambiguous state.
• Replay evidence: React on Rails PR #4749 review threads and the concrete reproductions above at head 04c1b8149e4bf6f7ccb8edd41f8e478d78bdfcc6.
• Non-goals: consumer-only overlays; changing pull-request trust boundaries; pull_request_target; optional final-candidate edge nits concerning absolute-path npm invocation, pathological post-SIGKILL reaping, or mutable documentation links.

[justin808]

Partial coverage from the AW E 08-05 batch — this issue stays open

PR #329 implements contract 1 only. The other five contracts are untouched and remain in scope here. The PR deliberately says Refs #202, not Fixes #202, so this issue is not auto-closed.

Landed (contract 1 — pr-ci-readiness unknown/malformed buckets)

RECOGNIZED_BUCKETS now pins the documented gh pr checks bucket set, and a structurally invalid row (non-object, missing bucket, or unrecognized bucket) fails closed to NOT_READY and is named in a new "invalid" result field so the verdict explains itself. Invalid rows take priority over the empty-rows UNKNOWN case.

The issue's exact reproduction now behaves as specified:

verdict=NOT_READY  invalid=["mystery (bucket: \"future-state\")"]     # previously READY

Review also caught that the first cut only satisfied the contract at the PrCiReadiness.assess library level while the production CLI crashed before reaching it — Runner#assess indexed unvalidated parse_rows output and raised an uncaught NoMethodError (Error < StandardError is a distinct class, so Runner#run's rescue Error did not catch it). That is fixed: every raw-row accessor is guarded, and malformed rows are kept rather than filtered so they still reach invalid_names — dropping them would have converted fail-closed back into fail-open. CLI-level regression tests were added, with red-before/green-after evidence.

Still open here

#	Contract	Owning file
2	Trust-role values (trusted_bots / trusted_metadata_bots) accept malformed types	bin/agent-workflow-seam-doctor
3	status: not_applicable can satisfy required priority evidence	closeout-evidence-replay
4	Fresh dispatch approval persists stale refresh provenance	skills/pr-batch/bin/dispatcher-capability-preflight
5	Explicit GH_HOST does not verify the resolved repository URL host	skills/pr-batch/bin/pr-security-preflight
6	Completed-audit ref / OUTSTANDING comma-fallback grammar ambiguity	skills/post-merge-audit + docs

Those five were out of the AW E ci-readiness lane's owned paths (skills/pr-batch/bin/pr-ci-readiness, skills/pr-batch/bin/pr-ci-readiness-test.rb) and belong to concurrent batches or a future lane. The issue's acceptance criterion that each regression be demonstrated red before its fix still applies to all five.

[justin808]

Contract 2 is covered in PR #717:
#717

This change adds strict pre-normalization validation for trusted_bots and trusted_metadata_bots at the shared trust-config seams, while preserving scalar-string compatibility, normalization, and overlap checks.

Contracts 3-6 remain open.