Skip to content

fix: write orchestrator failure reasons to task_logs.json (#1978) #2005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
octo-patch wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: write orchestrator failure reasons to task_logs.json (#1978) #2005

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions apps/desktop/src/main/ai/agent/worker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,18 @@ async function runBuildOrchestrator(
// state that doesn't map to a log phase. In that case, close whichever log phase
// is still marked 'active' so the UI shows "Complete" instead of "Running".
if (logWriter) {
// Write the failure reason to the log before closing the phase so the
// Logs tab shows a visible error entry. outcome.error originates from
// build-level validation failures that never flow through onEvent.
if (!outcome.success && outcome.error) {
const data = logWriter.getData();
const activePhase = (['validation', 'coding', 'planning'] as const).find(
(p) => data.phases[p]?.status === 'active'
);
const errorPhase: 'qa' | 'coding' | 'planning' =
activePhase === 'validation' ? 'qa' : (activePhase ?? 'planning');
logWriter.logText(outcome.error, errorPhase, 'error');
}
Comment on lines +684 to +692
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for identifying the active phase and mapping it to a Phase type is correct and ensures that orchestrator-level errors are attributed to the correct log section. However, note that this logic for finding and mapping the active phase is partially duplicated in the terminal state cleanup block (lines 699-705). While not a bug, consolidating this into a helper or a shared variable within the if (logWriter) block would improve maintainability.

const finalLogPhase = mapExecutionPhaseToPhase(outcome.finalPhase);
if (finalLogPhase) {
logWriter.endPhase(finalLogPhase, outcome.success);
Expand Down Expand Up @@ -946,6 +958,14 @@ async function runSpecOrchestrator(

// Ensure any still-active log phase is closed and flushed
if (logWriter) {
// Write the failure reason to the log before closing the phase so the
// Logs tab shows a visible error entry instead of just a red badge with
// no message. outcome.error comes from spec phase validation failures
// (e.g. "expected files not created: context.json") which never flow
// through the onEvent stream callback.
if (!outcome.success && outcome.error) {
logWriter.logText(outcome.error, 'spec', 'error');
}
const data = logWriter.getData();
// toLogPhase('spec') maps to 'planning' in the log writer
if (data.phases.planning?.status === 'active') {
Expand Down
Loading