Skip to content

fix(orchestration): emit the real task status on non-status updates - #12550

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
pacocartones:fix/orchestration-emit-real-status
Sep 11, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
pacocartones:fix/orchestration-emit-real-status

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

Summary

  • updateCloudAgentTask (src/lib/cloudAgent/db.ts:124) published state: "updated" on agent.task.updated whenever the update carried no status field. The activities sync in src/app/api/v1/agents/tasks/[id]/route.ts:175 and the result/error writes hit that path, so the agents WS channel carried a fabricated state that no OrchState maps to. This is the first bullet of the PR-B1 minors in feat: orchestration canvas polish (fase 2 PR-A final-review minors) #12392 ("read the row's real status or document "updated" as a sentinel").
  • The writer now reads the row's current status after the UPDATE and publishes that; a status-carrying update still publishes the value it wrote, without the extra read. When no row matched the id nothing was written, so nothing is published either (the A2A updateTask already behaves this way for an unknown id).
  • Deliberately out of scope: the duplicated emitAgentTaskUpdated helper (third bullet, it needs a2a/taskManager.ts and lands better after PR-B2), the no-op status write in the GET sync (fourth bullet), and the hubProxy module-level state (second bullet).

Related Issues

Validation

  • Change type: DB (cloud-agent task writer, event payload)
  • Focused tests and category gates from the golden path: tests/unit/agents-channel-publish.test.ts 10/10 (node --test), node scripts/check/check-complexity-ratchets.mjs --base-ref origin/release/v3.8.51 OK (0 violations in touched files), npm run check:changelog-integrity OK, npm run typecheck:core 0 errors
  • npm run lint
  • Reconciled with the current active release base release/v3.8.51; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR
  • SonarQube is temporarily opt-in while the private project has no quota; it is not a PR gate.

eslint over the two touched files exits 0. Mutation: restoring the ?? "updated" fallback fails the "emits the row's current status" case; dropping the if (state) guard fails the "unknown id does not emit" case.

Tests Added Or Updated

  • tests/unit/agents-channel-publish.test.ts: the "without a status field emits state 'updated'" case is replaced by "without a status field emits the row's current status" (queued → running, then a result-only update publishes running), and a new "on an unknown id does not emit" case. Red on the base (2 fail / 8 pass), green with the change (10/10). The other eight cases are unchanged.

Coverage Notes

  • src/lib/cloudAgent/db.ts: the new read-back branch and the no-row guard in updateCloudAgentTask are both exercised by the two cases above; the status-carrying branch stays covered by the existing "emits agent.task.updated with the new status" case.
  • No touched file lost coverage.

Reviewer Notes

  • One extra SELECT ... WHERE id = ? per status-less update (the activities sync on drawer open, result/error writes). It runs on the primary key of a table that is already read on the same request path.
  • Behaviour change for consumers of the agents channel: after a result-only write they now receive the task's real status (for example running) instead of "updated"; nothing in src/ matched on the old sentinel.

`updateCloudAgentTask` published `state: "updated"` on `agent.task.updated`
whenever the update carried no `status` field (result/activities/error
writes), a state no consumer of the channel recognises. Read the row's
current `status` after the write and publish that instead; when no row
matched the id nothing was written, so nothing is published either.

Refs diegosouzapw#12392
@diegosouzapw
diegosouzapw merged commit 3198c54 into diegosouzapw:release/v3.8.51 Sep 11, 2026
16 checks passed
Githab-capibara added a commit to Githab-capibara/OmniRoute that referenced this pull request Sep 17, 2026
…iegosouzapw#12550)

Correct: `state: "updated"` mapped to no `OrchState`, so the channel carried a value no consumer could interpret. Reading the row back only on the no-status path, and publishing nothing when no row matched, both match what the A2A side already does.

---

Validated in one consolidated worktree cut from `release/v3.8.51`, boarded with the rest of this batch — zero conflicts between the 19 PRs.

- `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, all within the frozen baseline); `check:changelog-integrity` OK
- complexity 2802 / baseline 3218 and cognitive-complexity 1267 / baseline 1437 — both under baseline
- 226 of 228 focused assertions green across the batch's 23 test files. The 2 remaining belong to diegosouzapw#12551, which is held separately.

Two batch-owned defects were found and fixed in flight, both pure base drift: `173_xp_action_counts.sql` collided with `173_call_logs_video_content_removed.sql` (renumbered to 176 on diegosouzapw#12651 — it aborted every DB open, which is what 53 of the first run's failures were), and the feature-flag catalog was missing the `SERVER_OWNED_TOOL_LOOP_ENABLED` row the base gained after diegosouzapw#12552 was written.

⚠️ base-red inherited: diegosouzapw#12732 — `Docs Gates`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` reproduce on the pure tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098, which this batch does not touch).

Thanks @pacocartones — the `file:line` citations and the explicit out-of-scope notes on every one of these made a 19-PR batch reviewable in one pass.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…iegosouzapw#12550)

Correct: `state: "updated"` mapped to no `OrchState`, so the channel carried a value no consumer could interpret. Reading the row back only on the no-status path, and publishing nothing when no row matched, both match what the A2A side already does.

---

Validated in one consolidated worktree cut from `release/v3.8.51`, boarded with the rest of this batch — zero conflicts between the 19 PRs.

- `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, all within the frozen baseline); `check:changelog-integrity` OK
- complexity 2802 / baseline 3218 and cognitive-complexity 1267 / baseline 1437 — both under baseline
- 226 of 228 focused assertions green across the batch's 23 test files. The 2 remaining belong to diegosouzapw#12551, which is held separately.

Two batch-owned defects were found and fixed in flight, both pure base drift: `173_xp_action_counts.sql` collided with `173_call_logs_video_content_removed.sql` (renumbered to 176 on diegosouzapw#12651 — it aborted every DB open, which is what 53 of the first run's failures were), and the feature-flag catalog was missing the `SERVER_OWNED_TOOL_LOOP_ENABLED` row the base gained after diegosouzapw#12552 was written.

⚠️ base-red inherited: diegosouzapw#12732 — `Docs Gates`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` reproduce on the pure tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098, which this batch does not touch).

Thanks @pacocartones — the `file:line` citations and the explicit out-of-scope notes on every one of these made a 19-PR batch reviewable in one pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants