Skip to content

feat(workhub): return delegated task results automatically - #5595

Merged
hqhq1025 merged 8 commits into
mainfrom
feat/workhub-result-return
Sep 23, 2026
Merged

hqhq1025 merged 8 commits into
mainfrom
feat/workhub-result-return

Conversation

@xxhZs

@xxhZs xxhZs commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

WorkHub can delegate a task, but its result currently does not return to the coordinating conversation. This change automatically resumes WorkHub when a newly delegated task completes, fails, is cancelled, or needs a user answer, so the user can follow the task through to completion in WorkHub.

  • Deliver results through durable, Host-owned turns and a native WorkHubResult tool that reads the full result or relays the target task's question. Delegation acknowledges and yields while the task runs.
  • Forward only the user's actual answer through the existing interaction authority. Preserve the original authorization and treat returned task content as data.
  • Reconcile after restart or provider reconnection, queue while WorkHub is busy, deduplicate delivery, and suppress obsolete assignments. Show automatic returns as Task result update in the conversation.

New assignments opt into result return; historical assignments are not replayed. Runtime Host compatibility epoch increases from 177 to 178, requiring matching Client and Host versions.

Verification

  • npm run build, npm run typecheck, npm run lint, and npm run format:check passed.
  • Built core, storage, runtime-host, and UI suites: 4,952 passed, 20 skipped, 0 failed (NODE_NO_WARNINGS=1 node --test --test-concurrency=3 on the four packages' compiled tests; warnings suppressed for Node 25's experimental SQLite notice).
  • ASF license-header audit and git diff --check passed. Merge simulation against the current main completed without conflicts.
  • Real-model end-to-end acceptance: built Electron + Runtime Host using the configured DeepSeek V4.1 Flash connection (deepseek-v4-flash), with isolated sessions and real model responses. Two user requests produced three automatic returns and 19 completed model calls, with no tool errors:
    • A delegated CSV task wrote {"paid_count":3,"paid_total":40.0}; WorkHub automatically read and reported the result.
    • A delegated task asked a question; WorkHub displayed the identical question/options, forwarded the user's answer, and reported the resulting {"format":"详细"} file.
    • Restarting both Desktop and Host produced no duplicate turns or additional model calls.
  • The first real-model run exposed a non-serializable undefined result field. The fix has JSON round-trip regression coverage, and both scenarios passed when rerun in a clean profile.

Real-model acceptance covers completion and question relay; cancellation, permission interruption, and cross-Host stress scenarios were not exercised with a real model.

Review follow-up (2026-09-23)

The three review findings are addressed:

  • Reconciliation first reads only event identity and terminal status. It loads the target transcript only when preparing a new delivery, and the new result read is limited to its target Turn. Idle reconciliation backs off to 60 seconds, while target changes wake it immediately.
  • A delegated message durably cancelled before it starts now produces a stable cancelled result instead of remaining pending.
  • When the original target question settles directly, the corresponding copied WorkHub question is closed through the canonical interaction authority. This releases the waiting WorkHub turn so the final task result can return.

An ordinary-delegation request asked only for a CSV report file, without asking WorkHub to report back or mentioning WorkHubResult. The target wrote the file, and WorkHub followed up with the completed result (paid_count=3, paid_total=40) without a second user message.

Presentation follow-up (2026-09-23)

The automatic WorkHub result wake previously appeared as a right-aligned user message, making it look like a second request. It now appears as a centered system status, Task result update · target name, while the normal assistant reply remains below it. The underlying durable result turn and delivery behavior are unchanged. All 617 UI tests, renderer build, lint, format, and ASF header checks pass; the refreshed real-model Desktop run returned the correct CSV result.

Multiple-result follow-up (2026-09-23)

Two real DeepSeek V4.1 Flash target tasks were delegated through WorkHub, then released together. Both wrote independently verified results ({"sum":6} and {"sum":9}), and WorkHub produced two distinct Task result update turns. The first reported A while B was still running; the second reported B and the model summarized both from conversation context. These target results actually finished about 47 seconds apart, so this run tests two outstanding delegations rather than simultaneous completed results. A coordinator regression test makes both results ready at once while WorkHub can admit only one turn, and confirms distinct, non-duplicated delivery. The Host serializes result updates; it does not batch them into a single turn.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex implemented the result-return flow and regression tests, and ran local and real-model checks. The commit includes Generated-by: OpenAI Codex.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Sep 22, 2026

@me2seeks me2seeks 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.

Review: approve, with two non-blocking findings

I checked out eed9e9a22 in a clean worktree and verified this independently rather than relying on the PR description.

Independent verification

  • npm run build, npm run typecheck, npm run lint, npm run format:check, the ASF header audit, and git diff --check all pass.
  • Compiled core, storage, runtime-host, and UI suites: 4,941 passed / 30 skipped / 1 failed. The single failure is managed-activation ("two real managed activations converge on one Host and exit at true idle"). I ran the same test against the base 0052f1cfd5 and against current main in the same environment: it fails identically on both, so it is a pre-existing environment-specific issue on my machine, not a regression from this PR.
  • All 131 new or touched tests pass, including restart dedup, late-answer prevention, permission-relay refusal, JSON round-trip, and Unicode-safe paging.

What I confirmed in the code

  • Deduplication is sound: the deterministic whf_ Turn identity plus the durable root admission record gives exactly-once delivery, including across Host restarts (the in-memory #delivered set is only an optimization; readRootTurnAdmission is the receipt).
  • The answer-relay security model holds: WorkHubResult has no answer parameter, the forwarded value comes only from a real askUserQuestion collection, delegation and target identity are rechecked under both admission leases after the human responds, and answerDelegatedQuestion only forwards question-kind interactions — permission approvals stay in the target Session, with refusal tests.
  • Non-feedback paths are unchanged: hostedExecutionMessageOrigin still returns undefined for ordinary coordination executions; only the single assignment write site sets returnResults: true, so historical delegations are not replayed.
  • The resolveRootTools substitution for the coordination Session looks aggressive but is safe: the v2 profile projection is the effective gate (and throws on a missing tool), and workHubResults is always assigned before any Session can run.

Findings (neither blocks merge)

1. Medium-low — perpetual 5s reconciliation with per-delegation transcript reads (workhub-result-coordinator.ts #schedule/reconcile). Once any returnResults delegation exists, #targets.size >= 1 holds forever — a normally completed delegation is never retired (only supersede/stop/archive removes it). So the Host re-runs reconcile() every 5 seconds indefinitely, and the #delivered.has(eventId) check happens after inspect(), which for a terminal observation includes a full manager.getMessages(targetSessionId) transcript materialization. Every completed delegation therefore re-reads its full transcript every 5 seconds, growing linearly with delegation count and session length. The event-driven wakeups (transcript/event subscriptions -> notify) are already wired; the 5s backstop only needs to serve pending-delivery/provider-reconnect cases. Consider skipping the expensive inspect for assignments with a confirmed terminal delivery, or backing off when nothing is pending. Minor aside: #delivered grows unboundedly (48 chars per entry, so practically small).

2. Low — confirm intent: result Turns record root: { kind: 'user' } (agent-run.ts invocationRootAuthority). The new workhub_result origin falls through to the default, so the immutable invocation_opened fact attributes this Host wake to the user. I audited the consumers of root: only the agent-graph timeline distinguishes non-user roots, and this profile disables memory extraction, so there is no functional impact today. I assume leaving the closed union untouched was deliberate to avoid a wider protocol change — a one-line comment saying so would help future readers.

Notes

The acceptance record's scoping is honest (no real-model cancellation, permission-interruption, or cross-Host stress coverage), and the ADR section matches the implementation. The epoch bump rationale comment chain is accurate. Nice work — the durable-receipt design and the recheck-under-both-leases pattern are exactly the conservative shape this flow needs.

@Astro-Han Astro-Han 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.

Review on head eed9e9a22f1af1bbdccae128df723bec3bc192d9: GO on code (feature — merge decision belongs to a human)

I reviewed the full diff (27 files, +1647/−19), verified both before/after screenshots against the implementation, and ran the new and adjacent test suites locally. Summary:

Design as implemented. The Host gains an owned result-return path for WorkHub delegations: a reconciliation coordinator derives a deterministic event id whf_<sha256(delegationId, runId, eventKey)>, and delivery goes through durable root admission (startWorkHubResult) with a persisted-admission lookup plus a strict identity-conflict check, which is what makes restart deduplication hold. Result notifications bypass the routing step and their activation content is replaced with the original task request. The new WorkHubResult tool is only resolvable in the coordination session; read is paged (16,000 characters per page) and ask_question only relays an actual pending question via answerDelegatedQuestion, so permission approvals stay in the target task. New regressions cover the lossless-JSON round-trip for completed/failed/cancelled results (packages/runtime-host/src/__tests__/workhub-result-runtime.test.ts, the survives lossless JSON persistence case) — matching the failure the acceptance document describes fixing.

Screenshots. before.png shows the old acknowledgement-only behavior; after.png shows the automatic "Task result update" turn (display name = target session name) reporting paid_count: 3, paid_total: 40.0 and the report path. Both match the code paths described above.

Local evidence. All workspaces build; workhub-result-coordinator/workhub-result-runtime 22/22; adjacent execution-composition/interaction-coordinator/tool-profile/storage admission suites 68/68; chat-turn-answer-identity passes; core/ui typecheck clean. Hosted CI test/audit/label all succeeded at this SHA. Epoch ledger: main at 177, branch claims 178 with a protocol comment — coherent.

Minor notes (non-blocking).

  1. The in-process deduplication set grows without bound over a long-lived host (cross-restart dedup correctly relies on the durable admission check).
  2. In the notification payload only result is truncated; details and the original request text are not size-capped.
  3. While the coordination session is busy, reconciliation retries every 5 s until idle — correct semantics, just continuous light polling (the timer is unref'd).

What I could not verify. The real-model acceptance runs (19 model calls, restart dedup) are documented by the author but not reproducible in my environment; multi-host concurrency and long-run soak are untested here.

This is a feature; the merge decision belongs to a human maintainer.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@ARE404 ARE404 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.

AI-assisted review by OpenAI Codex, revalidated against eed9e9a. Three P2 findings remain: one reconciliation performance issue and two result-return behavior gaps.

Validation: the 22 focused WorkHub result coordinator/runtime tests pass. Two additional stub-based probes reproduce cancelled queued messages reporting pending and a relayed question remaining blocked after the original task completes. A coordinator probe also confirms repeated inspections after delivery. These are code-path checks and isolated probes, not full Desktop end-to-end reproductions; the full build was not validated because the local dependency environment has type mismatches.

Comment on lines +156 to +159
const observed = await this.ports.inspect(assignment);
if (!observed) continue;
const origin = workHubResultOrigin(assignment, observed);
if (this.#delivered.has(origin.eventId)) continue;

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.

[P2] Check delivery identity before loading the complete result

The delivered-event check happens after inspect(). Completed assignments remain active until retirement/archive, so the five-second sweep keeps inspecting every completed delegation even when nothing changes. Production inspection rereads the target assignments and calls manager.getMessages(), reconstructing the entire target transcript under both the WorkHub and target admission leases. With many completed delegations in one growing Session, this repeatedly scans the same history and delays competing admissions. A 100-assignment coordinator probe produced cumulative inspection counts of 100, 200, and 300 across three unchanged sweeps, while deliveries stayed at 100. Please obtain the current event identity cheaply, skip delivered events before loading content, and bound result reads to the relevant Turn. Preserve detection of genuinely new continuation events.

assignment.targetMessageId,
lease,
);
if (disposition.kind !== 'owned_root' && disposition.kind !== 'shared_turn') return undefined;

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.

[P2] Return cancellation for delegated messages cancelled before execution

Delegate into a busy target, then stop the target or retract its queued message before the delegation is consumed. The message authority durably cancels that message (declareStopFence / queue retraction), and readMessageExecutionDispositionAdmitted() returns cancelled; this does not retire the WorkHub assignment. This guard discards the terminal state, so no cancellation notification is delivered and WorkHubResult.read keeps reporting pending for work that can never run. A probe with the real result tool and a cancelled message disposition reproduces pending. Please handle durable message cancellation separately from outcomes that already have an owning/shared Turn, including a stable delivery identity for that case.

Comment on lines +235 to +240
const answer = await ctx.askUserQuestion(
request.request.questions.map((q) => ({
question: q.question,
options: q.options.map((o) => ({ ...o })),
})),
);

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.

[P2] Unblock the relayed question when the original interaction settles

After WorkHub presents this question, the user can still answer it in the original target task. That resumes the target and can let it finish, but this separately created WorkHub interaction remains parked: it has no settlement link to the original interaction, and the target is rechecked only after another answer arrives here. Completion delivery then stays pending because startWorkHubResult() requires idle WorkHub. A stub-based probe that completes the target after presenting the relay confirms the tool remains waiting until the copied question is answered separately. Please close/unblock the relay when the original interaction is answered or invalidated, so results can return without a redundant answer or manual cancellation.

@hqhq1025
hqhq1025 merged commit bc0786e into main Sep 23, 2026
2 checks passed
@hqhq1025
hqhq1025 deleted the feat/workhub-result-return branch September 23, 2026 07:33

@Astro-Han Astro-Han 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.

Re-verification on new head 15818b6b39df8a96b96e9a1f50e14c4bce009997: GO stands; required gates are green again

This supersedes the situation described in my earlier comments: those were bound to older heads and are fully superseded by the newer commits.

What changed since d362c7a2. Two commits:

  1. f8fbf77c removes the PR-only demo media (both .mp4 files and the extra screenshot) and the in-repo acceptance notes. This closes the "Verify repository source archive" failure I reported earlier — the archive step now passes, because the previously unclassified non-text release inputs are gone.
  2. 15818b6b3 deflakes the WorkHub Resume and Stop follow logical lineage across repeated physical handoffs test in packages/runtime-host/src/__tests__/execution-composition.test.ts: on a session_busy admission race it now waits (up to 10 s) for the running whf_* feedback turn to complete before retrying the action. That test was the remaining hosted-CI failure I had flagged; it now passes in the hosted run.

Gate: green. Hosted CI at this SHA: test passed (run 35830218863, 22m37s, including the previously failing lineage test), audit passed, archive verification passed. Head unchanged between check and post; mergeable, merge-tree clean, git diff --check clean.

Local evidence. All workspaces build; workhub-result-coordinator/workhub-result-runtime 27/27 (including the concurrent-delivery test); chat-turn-answer-identity 30/30; the lineage test passes 4/4 when run in isolation. One honest caveat: when the six adjacent suites are run together locally, that test can still occasionally hit the session_busy timing window (it passes in isolation and in the hosted environment), so a batch-isolation cleanup would be a nice follow-up — non-blocking.

Ledger. Epoch: main at 177, branch at 178 — unchanged and coherent. Remaining known minor notes: the in-process deduplication set grows without bound, and notification details payloads are not size-capped; both non-blocking.

This is a feature; the merge decision belongs to a human maintainer.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

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

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants