Fix mission workflow phase orchestration#313
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Owner
Author
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughThis PR extends CLI worker mission tools ( ChangesWorker Mission Tools, Phase Orchestration, and Orchestrator Closeout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/main/services/orchestrator/coordinatorTools.ts (1)
3292-3300:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidator specialists can still be spawned outside validation-capable phases.
This path passes
validationHeuristics: "prompt_only"and drops the explicit role, so a specialist role likevalidator/revieweris never considered bylooksLikeValidationWorkerRequest(). If the objective text avoids the prompt keywords, the coordinator can still launch a validator specialist during development. The same gap exists in the subagent delegation call sites below.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/orchestrator/coordinatorTools.ts` around lines 3292 - 3300, The validator-specialist spawn check is bypassed because validateDelegationPromptForCurrentPhase is called with roleName: null and validationHeuristics: "prompt_only", so looksLikeValidationWorkerRequest never considers validator/reviewer roles; update the call sites (including the one using validateDelegationPromptForCurrentPhase and the subagent delegation calls below) to pass an explicit roleName of "validator" or "reviewer" (whichever matches your role enum) when evaluating a validation specialist, or change validationHeuristics to include role-based checks (e.g., "prompt_and_role") so looksLikeValidationWorkerRequest can detect validator requests; ensure the same fix is applied consistently to all matching calls in coordinatorTools.ts.
🧹 Nitpick comments (2)
apps/ade-cli/src/adeRpcServer.test.ts (2)
3497-3566: ⚡ Quick winConsider strengthening the
rawfield assertion to verify all input strings are preserved.The current assertion on line 3563 uses
expect.stringContainingto verify thatrawcontains only one of the three input strings. If the normalization logic is intended to preserve all original legacy strings (not just the one with explicit counts), the test should verify that all three entries appear in therawfield to ensure complete legacy data retention and serve as clearer documentation of expected behavior.♻️ Suggested enhancement to verify complete raw field preservation
expect(response?.isError).toBeUndefined(); expect(response.structuredContent.report.testsRun).toMatchObject({ passed: 3, failed: 0, skipped: 0, - raw: expect.stringContaining("npm test (passed: 2 files, 3 tests)") }); + expect(response.structuredContent.report.testsRun.raw).toContain("npm run typecheck (passed)"); + expect(response.structuredContent.report.testsRun.raw).toContain("npm test (passed: 2 files, 3 tests)"); + expect(response.structuredContent.report.testsRun.raw).toContain("ADE_PROJECT_ROOT=/tmp/app npm run build (passed)");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/adeRpcServer.test.ts` around lines 3497 - 3566, Update the test "normalizes legacy string test summaries for report_result" to assert that the normalized testsRun.raw preserves all original input strings: after calling callTool(handler, "report_result", ...) check response.structuredContent.report.testsRun.raw includes each of the three provided strings ("npm run typecheck (passed)", "npm test (passed: 2 files, 3 tests)", "ADE_PROJECT_ROOT=/tmp/app npm run build (passed)"). Modify the expectation that currently uses expect.stringContaining for a single string to verify all three substrings are present (e.g., three stringContaining checks or a combined check) so the test ensures complete legacy-data retention for testsRun.raw.
3551-3556: ⚖️ Poor tradeoffConsider adding edge case tests for legacy test normalization.
This test covers the happy path where one entry contains explicit numeric counts. To ensure robust parsing behavior, consider adding test cases for:
- All entries without explicit counts (to verify default behavior)
- Multiple entries with explicit counts (to verify summing logic)
- Entries with failed/skipped indicators (to verify failed and skipped count parsing)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/adeRpcServer.test.ts` around lines 3551 - 3556, Add edge-case unit tests for the legacy test normalization logic that exercises the testsRun parsing: create tests that (1) supply multiple entries with no explicit numeric counts to assert defaults are applied, (2) supply multiple entries with explicit numeric counts to assert counts are summed correctly across entries, and (3) include entries containing failed/skipped indicators to assert failed and skipped counts are parsed and included in the resulting summary; update the existing assertions that inspect the testsRun output (the testsRun array used in the current test) to validate total, passed, failed, and skipped counts for each case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/src/cli.ts`:
- Around line 2270-2282: The status handler currently builds the fallback
message by filtering the raw args array before flag consumption, which lets
flags like --status or structured flags leak into the message; fix this in the
report_status branch by first reading known flags via readValue (e.g.
"--text","--message","--summary","--status") and then compute the fallback
message only from positional tokens remaining after removing all recognized
flags and their values (include "--arg" and "--arg-json" in the recognized set
and strip their following value(s) too), ignoring standalone "--", then pass
that cleaned message as summary/nextAction/details when calling
collectGenericObjectArgs so flag values are not included in the generated
message.
- Around line 10250-10255: The current shouldReplaceMachineRuntimeVersion guards
on runtimeVersion being truthy, so a missing/empty daemon version is treated as
compatible; change the logic in shouldReplaceMachineRuntimeVersion to consider a
null/undefined runtimeVersion as incompatible when the CLI has a real VERSION
(i.e., remove the truthy check and instead return VERSION !==
PLACEHOLDER_VERSION && runtimeVersion !== VERSION) so that the function returns
true when the CLI has a non-placeholder VERSION and the daemon version is absent
or different.
In `@apps/desktop/src/main/services/adeActions/registry.ts`:
- Around line 1995-2001: The current finalizeRun implementation passes a
possibly unresolved Promise (result) into waitForMissionCloseoutAfterFinalize,
so if runtime.aiOrchestratorService.finalizeRun or service.finalizeRun is async
the helper receives a Promise instead of the resolved finalized status; fix by
awaiting the finalize call first (await the result of either
runtime.aiOrchestratorService.finalizeRun(args) or service.finalizeRun(args)),
store the resolved value, then call await
waitForMissionCloseoutAfterFinalize(runtime, args.runId, resolvedResult) and
return the resolved value; reference functions: finalizeRun,
runtime.aiOrchestratorService.finalizeRun, service.finalizeRun, and
waitForMissionCloseoutAfterFinalize.
- Around line 1968-1972: The poll loop inside finalizeRun() currently awaits
runtime.missionService.get(missionId) directly so a transient rejection can
cause finalizeRun() to fail; wrap the get call in a try/catch and treat any
error as a best-effort stop/skip of the wait (e.g., log/debug and break out of
the loop or return) instead of rethrowing, keeping the existing timeout and
terminalMissionStatuses checks; update the block that calls
runtime.missionService.get(missionId) to catch errors and exit the polling loop
on failure.
In `@apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts`:
- Line 6698: The waitFor call awaiting mission completion uses the default 2s
timeout which causes CI flakes; update the await waitFor invocation (the call
that checks fixture.missionService.get(mission.id)?.status === "completed") to
pass an explicit longer timeout option (e.g., 10_000 ms or another appropriate
value) as the second argument so the test allows more time for async
finalization.
In `@apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts`:
- Around line 2994-3031: The code marks read-only worker runs as "planningLike"
which suppresses plain summary evidence; update the planningLike computation to
stop treating meta.readOnlyExecution as equivalent to a planning phase.
Specifically, modify the planningLike variable (currently checking
meta.readOnlyExecution === true, meta.phaseKey and meta.stepType) to only
consider explicit planning markers (meta.phaseKey and meta.stepType) and remove
the meta.readOnlyExecution check so summaries from read-only validation/review
phases are preserved when collecting details (affects uses in the blocks that
read report and validationReport).
- Around line 5453-5461: The success sync is being triggered before the
retrospective write completes, risking stale mission state; instead, create a
promise (e.g., retrospectiveStateSync) that awaits the
updateMissionStateDocument call (including latestRetrospective and reflections
via orchestratorService.listReflections) and catch/log errors using the existing
logger, then chain the syncMissionFromRun(runId, "finalize_run_succeeded") in a
.then() (or await) after retrospectiveStateSync so the retrospective persistence
completes before triggering syncMissionFromRun; reference
updateMissionStateDocument, latestRetrospective, retrospective,
orchestratorService.listReflections, retrospectiveStateSync, and
syncMissionFromRun in your fix.
In `@apps/desktop/src/main/services/orchestrator/coordinatorTools.ts`:
- Around line 2465-2473: The mustPrecede check uses
phaseHasSucceeded(predecessorPhase) which allows a predecessor to be considered
OK even if it skipped required validation gates; make this gating consistent
with set_current_phase by using
hasValidatedSuccessfulCompletion(predecessorPhase) (or otherwise ensure
validateRequiredValidationGates is expanded to include mustPrecede-linked
phases) when evaluating predecessors returned by
resolveMustPrecedePredecessors(sorted, currentPhase); update the for-loop that
iterates mustPrecedePredecessors to call hasValidatedSuccessfulCompletion and
return the same invalid/reason structure if that check fails so spawn gating and
phase-transition gating behave identically.
- Around line 1675-1680: phaseAllowsValidationWorker currently only accepts
exact step types "validation", "review", or "test_review", which excludes custom
review phases; update the check in phaseAllowsValidationWorker to treat any
resolved step type containing "review" or "validation" (e.g.,
stepType.includes("review") || stepType.includes("validation")) as
validation-capable, while preserving the existing fallback that inspects
phase.validationGate (gate.required and gate.tier === "dedicated"); reference
resolvePhaseStepType(phase.phaseKey), phase.phaseKey, phase.validationGate,
gate.required, and gate.tier when making the change.
In `@apps/desktop/src/main/services/orchestrator/executionPolicy.ts`:
- Around line 930-948: validationSurrogateSearchText currently only concatenates
free-text fields, so structured phase/report fields stored in step.metadata or
in meta.lastResultReport/meta.lastValidationReport are not indexed; update
validationSurrogateSearchText to also extract and include structured keys used
by the validation finalizer (e.g., targetPhase, phase, reviewEvidence or similar
fields present on meta, meta.lastResultReport and meta.lastValidationReport) by
converting them to strings (e.g., grab meta.targetPhase ||
lastResultReport.phase || lastValidationReport.phase and stringify any objects
or arrays) and append them to the values array before calling
normalizeSearchableText so canUseValidationStepAsPhaseCompletionSurrogate can
match structured phase/report data as well.
- Around line 1293-1297: The fallback loop that iterates phaseMap.keys() can
re-add the "planning" phase into phaseOrder; update that loop in
buildExecutionPlanPreviewFromPhases so it also skips the planning phase (e.g.,
if phaseName === "planning") just like the first loop does, i.e., continue when
seenPhases.has(phaseName) OR phaseName === "planning", so planning cards are
never pushed into phaseOrder as a fallback.
In `@apps/desktop/src/main/services/orchestrator/orchestratorService.ts`:
- Around line 1440-1443: The planSummary assignment currently treats
rawPlan?.summary as real text without excluding plan-template placeholders;
change the predicate to also call the plan-template placeholder detector (use
the same check used for plan.markdown, e.g.
!looksLikePlanTemplatePlaceholder(rawPlan.summary) or combine with
!looksLikeReportTemplatePlaceholder if both are relevant) so that planSummary
still requires a non-empty trimmed string and not a template placeholder; update
the same pattern wherever plan.summary is normalized (e.g., the other summary
handling around the planSummary/plan.markdown logic) to ensure placeholders are
never promoted into summary.
- Around line 331-334: getWorkerStepOutputPath currently scopes artifacts only
by stepKey, allowing retries to pick up stale outputs; change the function
signature to accept an attempt identifier (e.g., attemptId: string or number)
and include a sanitized attemptId in the filename (e.g.,
`step-output-${sanitizedStepKey}-attempt-${sanitizedAttempt}.md`), update the
sanitation logic to handle the attempt value, and update all callers of
getWorkerStepOutputPath (and the other similar occurrences around the codebase
that generate/read step-output-*.md) to pass the current attempt identifier so
recovered files are scoped per-attempt not per-step.
---
Outside diff comments:
In `@apps/desktop/src/main/services/orchestrator/coordinatorTools.ts`:
- Around line 3292-3300: The validator-specialist spawn check is bypassed
because validateDelegationPromptForCurrentPhase is called with roleName: null
and validationHeuristics: "prompt_only", so looksLikeValidationWorkerRequest
never considers validator/reviewer roles; update the call sites (including the
one using validateDelegationPromptForCurrentPhase and the subagent delegation
calls below) to pass an explicit roleName of "validator" or "reviewer"
(whichever matches your role enum) when evaluating a validation specialist, or
change validationHeuristics to include role-based checks (e.g.,
"prompt_and_role") so looksLikeValidationWorkerRequest can detect validator
requests; ensure the same fix is applied consistently to all matching calls in
coordinatorTools.ts.
---
Nitpick comments:
In `@apps/ade-cli/src/adeRpcServer.test.ts`:
- Around line 3497-3566: Update the test "normalizes legacy string test
summaries for report_result" to assert that the normalized testsRun.raw
preserves all original input strings: after calling callTool(handler,
"report_result", ...) check response.structuredContent.report.testsRun.raw
includes each of the three provided strings ("npm run typecheck (passed)", "npm
test (passed: 2 files, 3 tests)", "ADE_PROJECT_ROOT=/tmp/app npm run build
(passed)"). Modify the expectation that currently uses expect.stringContaining
for a single string to verify all three substrings are present (e.g., three
stringContaining checks or a combined check) so the test ensures complete
legacy-data retention for testsRun.raw.
- Around line 3551-3556: Add edge-case unit tests for the legacy test
normalization logic that exercises the testsRun parsing: create tests that (1)
supply multiple entries with no explicit numeric counts to assert defaults are
applied, (2) supply multiple entries with explicit numeric counts to assert
counts are summed correctly across entries, and (3) include entries containing
failed/skipped indicators to assert failed and skipped counts are parsed and
included in the resulting summary; update the existing assertions that inspect
the testsRun output (the testsRun array used in the current test) to validate
total, passed, failed, and skipped counts for each case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45925274-8cd9-4e50-93f0-8d2a24271372
⛔ Files ignored due to path filters (1)
docs/features/missions/README.mdis excluded by!docs/**
📒 Files selected for processing (23)
apps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/stdioRpcDaemon.test.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/missions/missionPreflightService.tsapps/desktop/src/main/services/missions/missionService.tsapps/desktop/src/main/services/missions/phaseEngine.test.tsapps/desktop/src/main/services/missions/phaseEngine.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.tsapps/desktop/src/main/services/orchestrator/coordinatorAgent.tsapps/desktop/src/main/services/orchestrator/coordinatorEventFormatter.tsapps/desktop/src/main/services/orchestrator/coordinatorTools.test.tsapps/desktop/src/main/services/orchestrator/coordinatorTools.tsapps/desktop/src/main/services/orchestrator/executionPolicy.test.tsapps/desktop/src/main/services/orchestrator/executionPolicy.tsapps/desktop/src/main/services/orchestrator/orchestratorAdapters.test.tsapps/desktop/src/main/services/orchestrator/orchestratorService.test.tsapps/desktop/src/main/services/orchestrator/orchestratorService.tsapps/desktop/src/main/services/orchestrator/providerOrchestratorAdapter.ts
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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
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
mustPrecedeis enforced at runtime, custom validation/review phases can run validation workers, and execution previews keep custom phases.Audit notes
The audit compared ADE missions against the expected long-running mission shape: collaborative planning, worker execution, validation/review gates, adaptable prompts, and configurable paths. The concrete gaps fixed here were phase normalization, custom phase routing, ordering constraints, custom validation gates, read-only enforcement, and completion/finalization consistency.
Validation
npm --prefix apps/desktop run typecheck -- --pretty falsenpm --prefix apps/desktop run lintnpm --prefix apps/desktop run test -- --shard=1/8npm --prefix apps/desktop run test -- --shard=2/8npm --prefix apps/desktop run test -- --shard=3/8npm --prefix apps/desktop run test -- --shard=4/8npm --prefix apps/desktop run test -- --shard=5/8npm --prefix apps/desktop run test -- --shard=6/8npm --prefix apps/desktop run test -- --shard=7/8npm --prefix apps/desktop run test -- --shard=8/8npm --prefix apps/desktop run buildnpm --prefix apps/ade-cli run typechecknpm --prefix apps/ade-cli run testnpm --prefix apps/ade-cli run buildnpm --prefix apps/web run typechecknpm --prefix apps/web run buildnode scripts/validate-docs.mjsgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Tests
Greptile Summary
This PR hardens ADE mission workflow orchestration across three areas: phase normalization (planning phase is now always injected before any execution phase), custom-phase routing (first post-planning transition follows configured order,
mustPrecedeconstraints are enforced bidirectionally, validation workers are permitted in any dedicated-gate phase), and reliability fixes (read-only Codex workers stay in a read-only sandbox, result reports can be recovered from step-output files with a legacy-path fallback,syncMissionFromRunreplaces its stalesyncLocksguard with a proper in-flight Promise, andfinalizeRunwaits for mission closeout before returning).ensurePlanningPhaseis applied at every mission-launch and phase-override site;resolveDevelopmentPhaseKeyis replaced byresolveFirstPostPlanningPhaseKeyeverywhere a coordinator needs the first post-planning destination.findPhaseByRef/resolveMustPrecedePredecessorsare lifted intophaseEngine.tsand used from bothcoordinatorTools.tsandcoordinatorAgent.ts;buildExecutionPlanPreviewFromPhasesnow iterates phase cards in position order rather than a hardcoded six-element list.syncMissionFromRundeduplicates concurrent callers via asyncInFlightPromise map; step-output recovery probes both the newstep-output-{key}-attempt-{id}.mdand the legacystep-output-{key}.mdpath; newreport_status/report_result/report_validationCLI commands are wired up; version-mismatch detection skips the placeholder0.0.0build.Confidence Score: 4/5
The core logic changes are well-tested and directionally correct, but the coordinator's
read_step_outputtool still does not probe the legacy step-output filename when a latest attempt is known, leaving active missions mid-deployment unable to retrieve their output files through that tool.The phase-normalization, mustPrecede enforcement, and syncMissionFromRun deduplication changes are solid and backed by new tests. The read-only Codex sandbox fix closes a real security gap. The remaining concern is that
read_step_outputincoordinatorTools.tsonly tries the new attempt-scoped filename when alatestAttemptis available and never falls back to the legacy format, whereas the recovery path inorchestratorService.tscorrectly probes both. Any coordinator agent running against a worker whose output was written in the old format before this deployment would get a silent read failure from that tool, even though the file exists on disk.apps/desktop/src/main/services/orchestrator/coordinatorTools.ts— theread_step_outputhandler'scandidateRelativePathsshould include the legacy path as a second candidate whenlatestAttemptis non-null, mirroring the fallback already present inextractReportResultPayloadFromWorkerStepOutput.Important Files Changed
ensurePlanningPhase,resolveFirstPostPlanningPhaseKey,findPhaseByRef,resolveMustPrecedePredecessors, andnormalizePhaseRef— all with correct edge-case handling (planning-absent, -1 index, mustBeFirst clearing).extractReportResultPayloadFromWorkerStepOutput) at both open-session and finalize-session call sites; new-format step-output path includes attempt ID. Recovery logic is correct, but the coordinator'sread_step_outputtool still omits the legacy-path fallback (noted in a prior review).syncLocks-based deduplication with asyncInFlightPromise map so concurrent callers serialise on the same run; addsretrospectiveStateSyncbefore the post-finalize mission sync; adds runtimereview_summaryevidence collection; collects snake_caserisk_notesalongside camelCase.syncLocksis still populated inside the IIFE but is never read for gating — it is now dead state.phaseAllowsValidationWorkerso review/dedicated phases can spawn validation workers;roleNameis now matched against the validation pattern unconditionally (previously requiredincludeNameHeuristics);read_step_outputprobes the new attempt-scoped filename but still lacks a legacy-format fallback whenlatestAttemptis known (pre-existing gap).canUseValidationStepAsPhaseCompletionSurrogateto credit a passing validation step as evidence for required phases likecodeReview/testReview; rewritesbuildExecutionPlanPreviewFromPhasesto iterate phase cards in position order rather than a fixed six-phase list, preserving custom workflow phases in previews.allowReadOnlyControlPlaneFullAutoloophole that let read-only Codex workers usedanger-full-accesssandboxing; read-only workers now always getsandboxMode: "read-only". Addsmodel_reasoning_effortconfig when a reasoning level is resolved.waitForMissionCloseoutAfterFinalizeto bothbuildOrchestratorCoreDomainServiceandbuildAiOrchestratorDomainServicewrappers so callers block until the mission reaches a terminal status after finalization. These are independent wrappers over the same underlying service method, so there is no double-poll risk for a single call.summarizeLegacyTestsRunto parse numeric counts ("12 passed, 1 failed") viaparseLegacyTestCountbefore falling back to per-entry status; result type widens toRecord<string, unknown>and includes raw text and commands.report_status,report_result,report_validationto worker mission tool names with appropriate argument parsing; introducesshouldReplaceMachineRuntimeVersionso the0.0.0placeholder version never triggers a daemon restart loop.createMissionPreflightServicefrom a type-only import to a value import and actually instantiates it in the ADE CLI runtime; addsELECTRON_RUN_AS_NODE=1to the generated shim scripts so the Electron binary runs as a plain Node.js process.Sequence Diagram
sequenceDiagram participant C1 as Caller A participant C2 as Caller B participant S as syncMissionFromRun participant IF as syncInFlight Map participant DB as OrchestratorService / DB C1->>S: syncMissionFromRun(runId, "reason_a") S->>IF: get(runId) → undefined S->>IF: set(runId, executeSync Promise) S-->>S: launch IIFE (void async) S->>DB: getRunGraph / updateMission C2->>S: "syncMissionFromRun(runId, "reason_b", {nextMissionStatus})" S->>IF: get(runId) → executeSync (A's promise) C2-->>C2: await activeSync (blocked) DB-->>S: sync complete S->>IF: delete(runId) S-->>S: resolveSync() → executeSync resolves C1-->>C1: await executeSync returns C2-->>C2: activeSync resolved, wakes up alt C2 has nextMissionStatus C2->>S: "syncMissionFromRun(runId, "reason_b", {nextMissionStatus})" S->>IF: get(runId) → undefined (clear) S->>IF: set(runId, executeSync2 Promise) S->>DB: getRunGraph / updateMission (applies nextMissionStatus) DB-->>S: sync complete S->>IF: delete(runId) C2-->>C2: returns else no nextMissionStatus C2-->>C2: returns immediately endComments Outside Diff (1)
apps/desktop/src/main/services/orchestrator/coordinatorTools.ts, line 6817-6855 (link)read_step_outputWhen
latestAttemptis non-null (essentially always for any live step),candidateRelativePathsonly contains the newstep-output-{key}-attempt-{id}.mdpath. Workers that were already running before this PR deployed were instructed to write to the old path.ade/step-output-{key}.md. After deployment, the coordinator cannot read those files — it returnsok: false, error: "…"as if no output exists.The legacy fallback (
.ade/step-output-${sanitized}.md) is only tried whenlatestAttemptisnull, which won't occur for any in-progress mission. Adding it as a second candidate whenlatestAttemptis known would preserve backward compatibility for active missions mid-deployment.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "Preserve legacy worker step output recov..." | Re-trigger Greptile