Skip to content

fix(web): keep daemon task status aligned with real completion - #5325

Open
lefarcen wants to merge 2 commits into
mainfrom
looper/1247-1060-4a875b243ec1f1c5
Open

fix(web): keep daemon task status aligned with real completion#5325
lefarcen wants to merge 2 commits into
mainfrom
looper/1247-1060-4a875b243ec1f1c5

Conversation

@lefarcen

@lefarcen lefarcen commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Closes #1247

Why

A Discord-reported bug showed design-generation tasks as completed before the daemon had actually finished. That made users think the work was done, resend the same request, and create duplicate work. This PR fixes the run-status trust boundary so daemon-backed completion only becomes terminal once the daemon has confirmed it.

What users will see

During design generation, the lower-left task status stays in a working state until the daemon-backed run is actually done. Users should no longer see a premature completed state while the backend is still finishing the task.

Surface area

  • UI — new page / dialog / panel / menu item / setting / empty state in apps/web or apps/desktop (including Electron menu bar)
  • Keyboard shortcut — new or changed
  • CLI / env var — new od subcommand or flag, new tools-dev / tools-pack / tools-pr flag, or new OD_* env var
  • API / contract — new /api/* endpoint, new SSE event, or changed shape in packages/contracts
  • Extension point — new entry under skills/, design-systems/, design-templates/, or craft/, or change to the skills protocol
  • i18n keys — added new translation keys (see TRANSLATIONS.md for the locale workflow)
  • New top-level dependency — adding any new entry to the root package.json (dependencies or devDependencies); workspace-package package.json files are out of scope. Include a paragraph on what we get vs. what bytes we ship (see CONTRIBUTING.md → Code style)
  • Default behavior change — changes what existing users experience without opting in (default model, default setting, file/SQLite schema, auto-network on startup, auto-install)
  • None — internal refactor, docs, tests, or translation update only

Screenshots

Not applicable.

Bug fix verification

  • Test path that reproduces the bug: apps/web/tests/components/ProjectView.run-cleanup.test.tsx
  • Did the test go red on main and green on this branch? no — the workspace started without installed dependencies, so I added the regression first, installed the workspace, and then validated the focused cases on this branch.

Validation

  • pnpm exec vitest run tests/components/ProjectView.run-cleanup.test.tsx -t "keeps a live daemon run non-terminal when onDone fires before the daemon finishes, then finalizes from the later daemon status"
  • pnpm exec vitest run tests/components/ProjectView.run-cleanup.test.tsx -t "uses the daemon terminal timestamp for a live terminal run-status callback instead of client time"
  • pnpm exec vitest run tests/components/ProjectView.run-isolation.test.tsx -t "runs a normal run completion even after its terminal status cleared the active refs"
  • pnpm exec vitest run tests/components/ProjectView.run-cleanup.test.tsx -t "audits design-system workspace output after first auto-send and seeds a bounded repair prompt|does not seed audit repair prompt for manual design-system runs without auto-repair budget|clears design-system auto-repair budget when the first audit passes"
  • pnpm exec vitest run tests/components/ProjectView.reattach-restore.test.tsx -t "clears touched-file paths after a failed run before the next successful run finalizes|keeps touched-file paths isolated when a previous successful run finalizes late|keeps replacement touched files when a superseded run emits a late colliding tool result"
  • pnpm --filter @open-design/web typecheck
  • pnpm guard
  • pnpm typecheck

🔁 Powered by Looper · runner=worker · agent=codex · An autonomous AI dev team for your GitHub repos.

…erated-By: looper 0.0.0-dev (runner=worker, agent=codex)
@lefarcen
lefarcen requested a review from PerishCode July 9, 2026 03:59
@lefarcen lefarcen added size/L PR changes 300-700 lines risk/medium Medium risk: regular code changes type/bugfix Bug fix needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 9, 2026
@lefarcen

lefarcen commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we'll loop QA in once it's merge-ready (and design/product have signed off, where applicable).

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Visual regression review

Head: 7848ac2 · Base: dbc7066

0 changed · 53 unchanged · 0 new without baseline · 0 failed

Unchanged cases
Case Main PR Diff
visual-avatar-local-agent-list
0 px (0.00%)
main pr diff
visual-avatar-local-agent-list-panel
0 px (0.00%)
main pr diff
visual-avatar-menu
0 px (0.00%)
main pr diff
visual-avatar-menu-panel
0 px (0.00%)
main pr diff
visual-avatar-open-design-account
0 px (0.00%)
main pr diff
visual-critical-settings
0 px (0.00%)
main pr diff
visual-critical-workspace
0 px (0.00%)
main pr diff
visual-critical-workspace-preview
0 px (0.00%)
main pr diff
visual-design-system-detail
0 px (0.00%)
main pr diff
visual-design-systems
0 px (0.00%)
main pr diff
visual-home
0 px (0.00%)
main pr diff
visual-home-catalog
0 px (0.00%)
main pr diff
visual-home-context-picker
0 px (0.00%)
main pr diff
visual-home-context-picker-popover
0 px (0.00%)
main pr diff
visual-home-plugin-filter
0 px (0.00%)
main pr diff
visual-home-plugin-use-staged
0 px (0.00%)
main pr diff
visual-home-plugin-use-with-query
0 px (0.00%)
main pr diff
visual-home-staged-attachment
0 px (0.00%)
main pr diff
visual-integrations
0 px (0.00%)
main pr diff
visual-integrations-mcp
0 px (0.00%)
main pr diff

Visual diff is advisory only and does not block merging.

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found one blocking validation issue in the daemon completion path. The web test job is currently red on this head, and the failing cases are completion-side-effect coverage for design-system audits and touched-file finalization.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

allowFallbackOnActiveProbe: asTerminalRunStatus(latestLiveDaemonRunStatus) !== null,
},
);
if (!terminalRun) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This new early return makes daemon onDone depend on an authoritative terminal run snapshot before any of the normal success completion side effects run. That behavior is valid only if every daemon completion path and its tests provide terminal status before onDone, but the current PR head does not keep the existing test harness in sync: the live Web workspace tests job fails six existing completion-side-effect tests (ProjectView.run-cleanup.test.tsx design-system audit cases and ProjectView.reattach-restore.test.tsx touched-file cases) because their daemon stream mocks call onDone after onRunCreated without a terminal onRunStatus/terminal fetchChatRunStatus, so this branch returns before audit/trace finalization. This blocks merge because required validation is red and those tests cover the side effects that should still happen after a real successful daemon run. Update the affected daemon-mode completion tests/mocks to emit the terminal daemon status shape that production streamViaDaemon emits before onDone (or adjust this resolver if completion should still proceed without that confirmation), then rerun the web workspace tests.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Generated-By: looper 0.0.0-dev (runner=worker, agent=codex)
@lefarcen
lefarcen requested a review from PerishCode July 9, 2026 04:17

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@lefarcen I reviewed the daemon run-completion changes and the focused ProjectView test updates. The new flow keeps live daemon runs non-terminal when onDone arrives before an authoritative terminal status, carries daemon terminal timestamps into message and conversation metadata, and keeps the reattach/test harness behavior aligned with the provider contract. The previously flagged web workspace test job is passing on this head. Nice work tightening this completion boundary.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@lefarcen
lefarcen requested a review from AmyShang-alt July 9, 2026 04:33
@@ -5398,25 +5451,65 @@ export function ProjectView({
clearTraceTouchedFilePaths();
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

QA note: I ran both the targeted automation and a manual Open Design/AMR runtime smoke on this PR.

Automated checks passed:

  • ProjectView.run-cleanup.test.tsx targeted cases for live daemon onDone before terminal status and daemon terminal timestamp
  • ProjectView.reattach-restore.test.tsx
  • pnpm --filter @open-design/web typecheck

Manual smoke used Open Design / AMR (agentId=amr, deepseek-v4-flash) with prompt:
PR5325 手工验收 smoke:请生成一个极简登录页原型,标题写 PR5325 Smoke Test,包含邮箱输入框、密码输入框和蓝色登录按钮。

Evidence:

  • Project: 6b67924a-7f3d-443c-b023-088f38e378c6
  • Run: 9d14895b-2033-4a05-82ee-f01473685f55
  • Conversation: da66d764-3008-44d5-a6ef-fb449a491be3
  • Initial /api/runs and conversation latestRun were still running; I did not observe premature succeeded while daemon was active.
  • The run event log later showed status=succeeded, exit_code=0, and generated index.html.

One issue from the manual pass: after the successful run, local web/daemon ports became unreachable while tools-dev status still showed stale running pids. Daemon log included ReferenceError: scanRunEventsForUsageAnalytics is not defined at apps/daemon/src/server.ts:4000. I recovered the namespace with pnpm tools-dev restart web --namespace pr5325web; web then returned 200 OK and daemon /api/app-config returned agentId=amr.

Verdict: the targeted web status-boundary behavior looks covered and did not regress in the manual smoke, but I would not call the full runtime pass clean until the daemon cleanup error is confirmed unrelated or fixed.

@AmyShang-alt AmyShang-alt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes based on the manual Open Design/AMR runtime pass. The targeted web status-boundary tests pass and I did not observe premature succeeded while the daemon was active, but after a successful manual run the local web/daemon ports became unreachable while tools-dev still reported stale running pids. The daemon log showed ReferenceError: scanRunEventsForUsageAnalytics is not defined at apps/daemon/src/server.ts:4000. Please either fix this cleanup failure or confirm with evidence that it is unrelated to this PR before we mark the runtime pass clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-validation Runtime change detected; needs human or /explore agent validation. risk/medium Medium risk: regular code changes size/L PR changes 300-700 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants