fix(core): forward pre_output gate on_fail into compiled output#82
Conversation
The Gate type carries on_fail?: OnFailAction and quality_gates.pre_output is Gate[], but the pre_output gate call sites compiled via compileGateValidator(check, message) and never forwarded on_fail, so a runtime executing the compiled spec could not recover what action a failing pre_output gate should trigger. Same class as #64, different authoring site. Mirror #64: carry the recovery action as a first-class typed field on the compiled gate representation. For pre_output gates that representation is the gate validator's return shape, so add onFail?: OnFailAction to QualityGateResult, add an onFail parameter to compileGateValidator, and forward gate.on_fail at both pre_output call sites (per-step qualityGates and globalQualityGates). The action is surfaced on the failing result and omitted when the author did not specify on_fail (mirrors the optional source field, no invented default). The step.verification call site is unchanged; its recovery lives on CompiledStep.verification per #64. Adds a parametrised canary across all five OnFailAction values plus the absent case, asserting the compiled pre_output gate carries the authored on_fail through compilation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 20 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks 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 |
Summary
Gatecarrieson_fail?: OnFailActionandquality_gates.pre_outputisGate[], but the pre_output gate call sites compiled viacompileGateValidator(check, message)and never forwardedon_fail. The recovery action was dropped from compiled output, so a runtime executing the compiled spec could not recover what action a failing pre_output gate should trigger. Same class as #64, different authoring site (surfaced by CodeRabbit and cubic during review of #71).Typed shape
Mirrors #64, which carried recovery semantics as a first-class typed field on the compiled representation. For step verification that representation is the singular
CompiledStep.verification(CompiledVerification). pre_output gates have no per-gate data structure; their compiled representation is the gate validator's return shape, so the recovery action is carried there:compileGateValidatorgains anonFailparameter;gate.on_failis forwarded at both pre_output call sites (the per-stepqualityGateslist incompileStepand theglobalQualityGateslist incompileWorkflow). The action is surfaced on the failing result (passed: false) and omitted when the author did not specifyon_fail, mirroring the optional source field with no invented default. Thestep.verificationgate call site is unchanged; its recovery lives onCompiledStep.verificationper #64.Test (TDD, RED first)
Added a parametrised canary in
compiler.test.tsmatching the existingOnFailAction/ExecutionModestyle:it.eachover all fiveOnFailActionvalues (retry,escalate,skip,abort,revise) compiling a spec whosepre_outputgate carries thaton_fail, then asserting the compiled gate's failing result carries the authoredonFail. A sixth case assertsonFailis absent when the author omittedon_fail.Observed RED before the fix (all five forwarding cases failed because
onFailwas dropped):GREEN after (canary 6/6). Existing pre_output gate tests stay green because gates without an authored
on_failstill return{ passed, message? }unchanged.Scope
packages/coreonly:types.ts,compiler.ts,compiler.test.ts. The executor and adapter are untouched; consuming the field is a separate issue.Gates (all green, node v22.22.2)
npm run build:coreand fullnpm run build: successnpm test: 482 passed, 0 failed (476 + 6 new)npm run typecheck: cleannpm run lint: all three changed files clean (repo baseline of 5 errors / 22 warnings is in untouched files)node spec/fixtures/run-fixtures.mjs: 29 passed, 0 failedCloses #72
Summary by cubic
Fixes dropped
on_failon pre_output gates by forwarding it into the compiled output. Runtimes can now see the authored recovery action on failing gate results.gate.on_failto validators viacompileGateValidator(check, message, onFail)at both per-step and global pre_output call sites.QualityGateResultwith optionalonFail; set only on failed results and only when authored.verificationlogic unchanged; no defaults added.Written for commit 4a4f89e. Summary will update on new commits. Review in cubic