refactor(core): executor reads verification from CompiledStep, not the spec#84
Conversation
…e spec #64 added CompiledStep.verification (CompiledVerification | null) so recovery semantics survive compilation, but the dry-run executor still reached into spec.steps[name].verification when building the per-step quality-gate definitions. That left the runtime depending on the un-compiled spec for a value the compiled artifact already carries. Switch the read to compiledStep.verification, which is already in scope at the call site (the executor compiles the spec and holds the CompiledStep). The compiler maps on_fail_message -> message, so behaviour is identical: same check expression, same failure message in the gate result. No signature or plumbing change was needed. Tests: parametrised characterization across all five OnFailAction values plus the absent case, pinning that an authored verification surfaces its message as a quality-gate result and is preserved through the source switch. A loop-closing test wraps compileWorkflow to attach a verification to the compiled step while the source spec carries none, proving the executor reads the compiled artifact and no longer the spec. Scope is the verification read only; executionPlan consumption is #75. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe executor now reads step verification metadata from the compiled workflow artifact ( ChangesCompiled Verification Integration
Possibly Related PRs
Poem
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
Closes #73.
Summary
The dry-run executor (
packages/core/executor.ts) read each step's verification from the un-compiled spec (spec.steps[name].verification) when building per-step quality-gate definitions, even though #64 addedCompiledStep.verification(CompiledVerification | null) to the compiled artifact for exactly this purpose. This is the consumer-side counterpart to #64: the runtime now reads the compiled artifact and no longer reaches back into the source spec.compiledStep.verification, which is already in scope at the call site (dryRuncompiles the spec and holds theCompiledStep). The compiler mapson_fail_message -> message, so behaviour is identical: same check expression, same failure message.Data-flow finding
The executor already had the
CompiledStepin scope at the read site (executor.ts:226), so this is a local read switch, not a plumbing change.Scope
packages/coreonly (executor.ts + tests). The LangGraph adapter and allexecutionPlanconsumption are untouched (that is #75).Tests
OnFailActionvalues plus the absent case, pinning that an authored verification surfaces its message as a quality-gate result and is preserved through the source switch.executor.verification-source.test.ts) wrapscompileWorkflowto attach a verification to the compiled step while the source spec carries none, proving the executor reads the compiled artifact and no longer the spec. Verified RED before the fix.Test Plan
npm run build:core&&npm run buildnpm test(489 passed)npm run typechecknpm run lint(no new errors; 5 pre-existing out-of-scope errors unchanged)node spec/fixtures/run-fixtures.mjs(29 passed)🤖 Generated with Claude Code
Summary by cubic
Switches the dry-run executor to read step verification from the compiled artifact (
CompiledStep.verification) instead of the source spec. This keeps quality-gate behavior the same while removing runtime dependence on the authoring spec.compiledStep.verification(notspec.steps[name].verification); preserves check and message (on_fail_message->message).packages/core/executor.ts.Written for commit 05c1028. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Tests
Bug Fixes