fix(schema): require check_id on budget-committing report_plan_outcome outcomes (3.2 hardening) - #5985
Conversation
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
Argus is not auto-reviewing this PR because it modifies protected paths that require human review (static/schemas/source/governance/report-plan-outcome-request.json). A human reviewer should review and merge this PR; Argus will resume on subsequent PRs once these changes land on main.
bokelley
left a comment
bokelley
left a comment
There was a problem hiding this comment.
This is not eligible for 3.1.x and does not close #5827. check_id is a correlator, not cryptographic binding between the verified authorization and committed budget; the accepted direction is the bound-token work in 3.2. Please also rebase away the unrelated #5984 commit. If the WG still wants this as a separate validation improvement, reframe it as partial hardening for 3.2 with the correct release scope rather than a patch that claims to resolve the P0.
…e outcomes report-plan-outcome-request.json documents check_id as "Required for 'completed' and 'failed' outcomes" (schema description + task reference) but never listed it in any required[], so a validator accepted a budget-committing self-report with no reference to the authorizing check_governance decision. Adds an if/then conditional requiring check_id when outcome is completed or failed; delivery outcomes are unaffected. Aligns the schema with its own documented contract — no prose change. Scope: partial validation hardening targeted at 3.2. This does NOT close adcontextprotocol#5827 — check_id is a correlator, not a cryptographic binding between the verified authorization and committed budget; the bound-token work that closes the P0 is deferred to the 3.2 governance design. Refs adcontextprotocol#5827.
35237e1 to
0028c89
Compare
garvitkaushik-123
changed the title
There was a problem hiding this comment.
Argus is not auto-reviewing this PR because it modifies protected paths that require human review (static/schemas/source/governance/report-plan-outcome-request.json). A human reviewer should review and merge this PR; Argus will resume on subsequent PRs once these changes land on main.
2 participants
Summary
Reframed per @bokelley's review. This is a narrow validation-drift fix targeted at 3.2 — it does not close #5827, and does not claim to.
report-plan-outcome-request.jsondocumentscheck_idas "Required forcompletedandfailedoutcomes" (both in the field description and the task reference), but never listed it in anyrequired[]. A conformant validator therefore accepted a budget-committing self-report (outcome: "completed"/"failed") carrying no reference to the authorizingcheck_governancedecision.Adds an
if/thenconditional: whenoutcomeiscompletedorfailed,check_idis required.deliveryoutcomes (not budget-committing) are unaffected. No prose change — the schema is brought in line with its own documented contract.Scope / release line
minor, waspatch).check_idis a correlator, not a cryptographic binding between the verified authorization and the committed budget. The bound-token work that actually closes the P0 is deferred to the 3.2 governance design. This PR only closes the enforcement drift where the schema failed to require a field its own prose marks required.Refs #5827.main— the stray docs(media-buy): add Budget & Pacing Controls reference to create_media_buy #5984 doc commit is gone; the branch now carries only thereport_plan_outcomechange.Test plan
if/thenvalidates: accepts completed/failed withcheck_id, rejects without, acceptsdeliverywithout — incomposed-schema-validationminorIf the WG would rather fold this into the 3.2 bound-token work wholesale, happy to close this in favor of that.