fix(provenance): let a run that never started report why it failed - #7281
Merged
Conversation
A copilot-run workflow that fails before reaching the engine crossed back with no provenance, which latched the tool's registry and reduced the result to "result unavailable". The caller was told its run failed but not that the workflow was undeployed, or the input invalid, or the slot unavailable — the reasons this layer produces before any block runs, naming no secret because none had been resolved yet. The executor attaches its execution result to every throw, so the absence of one is proof that no block ran: output, logs and error are all undefined and the only content is a message this layer wrote. That is an absence, not an inability to vouch, so the crossing now carries an exact-empty envelope. The message still passes the tool boundary's egress projection against the same registry, so anything that registry knows is still redacted. A run that did execute and could not vouch hands back its incomplete envelope exactly as before, and that still latches. Make the attach total rather than conditional to keep that inference sound. A block failure is already normalized on the way in, so the old `instanceof Error` guard held in practice; what it did not give was a guarantee covering a non-Error raised by the engine's own synchronous work. toError is identity-preserving, so ordinary failures keep their type. The empty envelope moves to the registry module, which owns the vocabulary, replacing a private copy in the logging session so one definition states what "vouched for, naming nothing" is.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Greptile SummaryThis PR lets Copilot surface the original reason when a workflow fails before entering the execution engine while preserving untrusted provenance for failures after execution.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/workflows/application/run-workflow-from-copilot.ts | Distinguishes genuinely never-started failures from engine, post-execution, and post-crossing failures using attached or retained execution results. |
| apps/sim/lib/workflows/executor/execute-workflow.ts | Retains the core execution result and attaches it to normalized failures raised by subsequent processing. |
| apps/sim/executor/execution/engine.ts | Normalizes arbitrary thrown values so every propagated engine failure can carry its execution result. |
| apps/sim/executor/utils/resolved-secret-trace-registry.ts | Exposes a shared factory for fresh complete-empty provenance envelopes. |
| apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts | Covers never-started, post-crossing, post-execution, and incomplete-provenance failure paths. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Copilot starts workflow] --> B[executeWorkflow]
B -->|Fails before engine result| C[No execution result]
B -->|Engine or post-execution failure| D[Attach execution result]
B -->|Returns successfully| E[Retain run result]
C --> F[Import complete-empty provenance]
D --> G[Import attached provenance]
E --> H[Import returned provenance]
H -->|Crossing import fails| I[Retry failed crossing with retained result provenance]
Reviews (4): Last reviewed commit: "fix(provenance): normalize a post-execut..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…tarted claim Review round 1, both findings accepted. The post-run crossing runs inside the same try as the executor call, so when that import is what throws, the catch sees an error carrying no execution result — the same evidence a run that never started leaves. The previous condition read that as "nothing crossed" and vouched for it, when in fact an execution exists and its provenance was never imported, which is exactly the content that cannot be vouched for. Record whether the executor returned and require both facts before claiming the absence: not past the executor, and no result attached. Everything else hands back whatever envelope it has, and an incomplete one still latches. The executor test also could not fail against the old gated attach: a block failure is normalized on the way in, so its rejection already arrived as an Error. Drive it through the cancellation subscribe run() awaits before the queue instead, which is its own synchronous work and reaches the catch untouched — the case the total attach exists for.
Collaborator
Author
Collaborator
Author
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Round 2, cubic's finding accepted — and it was a distinct window, not a restatement of round 1. The executor's post-execution work runs after the run has produced a result but before `executeWorkflow` returns, so a failure there reached callers with no result attached: the run threw nothing itself, and the flag added last round could not be set yet. Every consumer that reads a missing result as "no block ran" was wrong in that window, this crossing included. Fix it where the result lives rather than at each reader. The executor attaches its own on the throws it raises; `executeWorkflow` now does the same for failures raised after it holds one, skipping the case the executor already recorded. Logging and trace spans get the same benefit for free — they read the identical signal. That makes an absent result total again, so the boolean flag goes and the crossing reads one thing: the result from the error, or the one already returned when the failure came later still, from the crossing itself. Only a failure with neither can claim nothing ran. The post-return case now describes content with the run's real envelope rather than latching blind, which is strictly more accurate than either prior behaviour.
Collaborator
Author
Collaborator
Author
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…he result Round 3, cubic's finding accepted. The guard added last round required the caught value to already be an `Error`, so a non-Error raised by post-execution work skipped the attach and was rethrown bare — the same hole this branch closed in the executor, left open one layer up by my own change. A Copilot run would have reported an executed workflow as never started and vouched for content it cannot describe. Normalize once at the top of the catch and use that value throughout, including the rethrow, matching what the executor does. `toError` returns an `Error` unchanged, so a custom error class keeps its identity and every ordinary failure is untouched — the existing identity assertion on the rejection path still holds. Two tests: the result reaches an ordinary post-execution failure, and a non-Error one is normalized so it can carry the result too. The second fails against the previous guard.
Collaborator
Author
Collaborator
Author
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
value-provenance-absentoncopilotWorkflowMutation.failedRunCrossing, unchanged since 08-21 across five deploys.inspectToolResultForCopilotreduced the result to{success: false, error: "result unavailable"}. The caller was told its run failed but not why, for a message this layer wrote that named no secret because none had been resolved yet.output,logsanderrorare all undefined and the only content isthrownMessage. That's an absence, not an inability to vouch, so the crossing now carries an exact-empty envelope. The message still passes the tool boundary's egress projection against the same registry, so anything that registry knows is still redacted. A run that did execute and could not vouch hands back its incomplete envelope exactly as before, and that still latches — pinned by its own test.error instanceof Error, so that inference is a guarantee rather than an accident. A block failure is already normalized on the way in viatoError, so the old guard held in practice; it just didn't cover a non-Errorraised by the engine's own synchronous work.toErroris identity-preserving.Verified against production
Read the externalized execution payloads from S3 for this workflow's runs (DB gives the storage pointer; the envelope isn't visible to SQL). Sampling before and after the v0.8.13 deploy that carried #7173: pre-fix 2/10 had
finalOutput complete=false; post-fix 0/10, across completed, failed and cancelled. So theTraceStore — displayProjectionstream is confirmed decaying stock of pre-08-27 executions, not a live producer — its emission logic is fixed, and this PR is about the separate copilot path.Also worth recording:
inherited-incomplete-source(2026-08-09) andmounted-file-provenance-unavailable(2026-08-10) are long-standing reasons that were newly observed, not newly added — the former is just a per-tool fork inheriting a latched parent, which is the isolation working.Type of Change
Testing
Two new tests on the crossing: a failure that never reached the engine now imports a complete envelope; one that did execute still passes through its incomplete envelope. Verified the first fails without the fix. Plus an executor test pinning that every throw out of a failed run carries its execution result. 6,828 tests green across
executor/,lib/logs/,lib/workflows/,lib/copilot/;bun run type-checkclean; all 39 audits pass. Rebased onto latest staging, conflicts resolved keeping both sides' tests.Checklist