feat(product-workflow): add WebUI service facade - #3691
Conversation
Introduces `WebUiService` (with `DefaultWebUiService` + `FakeWebUiService`) as the typed surface that Slice 2 WebChat v2 route handlers will depend on. Per #3611 acceptance criterion 3, handlers must consume only this facade and never reach the dispatcher / HostRuntime / run-state / DB / runtime-lane adapters directly.
There was a problem hiding this comment.
Code Review
This pull request introduces the WebUiService facade and its default implementation, DefaultWebUiService, to handle native WebChat v2 operations. It integrates with the thread service, turn coordinator, and event projection service to provide a unified interface for thread creation, message submission, run cancellation, gate resolution, and timeline retrieval. Additionally, a FakeWebUiService and comprehensive contract tests have been added. Feedback highlights an idempotency issue in thread creation where the client_action_id should be used to derive thread IDs deterministically. There is also a suggestion to log internal SessionThreadError details before redacting them to improve observability.
There was a problem hiding this comment.
Pull request overview
Adds a native WebUI-facing service facade in ironclaw_product_workflow for WebChat v2 route handlers, composing thread, turn, and projection services behind a typed API.
Changes:
- Introduces
WebUiService,DefaultWebUiService, command/result DTOs, timeline cursor/read support, and redacted service errors. - Adds
FakeWebUiServicebehindtest-supportfor downstream handler tests. - Adds contract tests covering thread creation, message submission, run cancellation, gate resolution, and timeline reads.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Cargo.lock |
Records new workflow crate dependencies. |
crates/ironclaw_product_workflow/Cargo.toml |
Adds event projection/event crates required for timeline reads. |
crates/ironclaw_product_workflow/CLAUDE.md |
Documents the new WebUI facade and dependencies. |
crates/ironclaw_product_workflow/src/fakes.rs |
Adds FakeWebUiService and call recording/programmed outcomes. |
crates/ironclaw_product_workflow/src/lib.rs |
Exports the new WebUI service types and fake. |
crates/ironclaw_product_workflow/src/webui_service.rs |
Implements the WebUI facade, default service, errors, helpers, and timeline projection routing. |
crates/ironclaw_product_workflow/tests/webui_service_contract.rs |
Adds contract tests for service behavior and fake sanity checks. |
Comments suppressed due to low confidence (2)
crates/ironclaw_product_workflow/src/webui_service.rs:627
- The denied/cancelled gate path also reaches the coordinator without checking thread ownership. Since coordinator authorization is limited to
TurnScopeand ignoresactor, this can let a user cancel another user's gated run in the same tenant/agent/project when they know the thread/run identifiers; perform the same owner-scoped thread validation before this call.
let response = self
.turn_coordinator
.cancel_run(CancelRunRequest {
crates/ironclaw_product_workflow/src/webui_service.rs:600
- Approved gate resolution reaches the coordinator without checking that the authenticated user owns the thread.
TurnScopedoes not includeowner_user_id, and the coordinator only compares scope/run/gate, so a caller who knows another user'sthread_id/run_id/gate_refin the same tenant/agent/project could resume that run. Validate owner-scoped thread access before this call.
let response = self
.turn_coordinator
.resume_turn(ResumeTurnRequest {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3cd4c98 to
c13f990
Compare
The reborn-integration merge brought in `RebornServicesApi`, an independently-built parallel facade with overlapping responsibility to the `WebUiService` we landed earlier. Two facades for the same browser-handler surface defeats the single-entry goal: handlers would have to choose between them and the boundary tests cannot keep both honest. Consolidate onto `RebornServicesApi` (the canonical surface the integration team has been tracking in AGENTS.md and the docs/reborn briefs) and port across the security fixes that the old `WebUiService` carried but the upstream version was missing.
serrrfirat
left a comment
There was a problem hiding this comment.
No blocking findings from my review.
Checked the final diff against the RebornServices/thread/turn contracts, including the new thread-ownership probe, denied/cancelled gate-ref validation, and deterministic create-thread id behavior. The added caller-level contract tests cover the mutation side effects rather than just helpers.
Verification run in an isolated worktree:
cargo test -p ironclaw_product_workflow --test reborn_services_contractcargo test -p ironclaw_product_workflowcargo clippy -p ironclaw_product_workflow --all-targets -- -D warnings
…3611 feat(product-workflow): add WebUI service facade
Summary
Consolidates the WebUI-facing facade in
ironclaw_product_workflowonto
RebornServicesApi(the canonical facade reborn-integrationintroduced in parallel) and ports forward the security checks the
earlier
WebUiServicecarried that the canonical version was missing:thread-ownership gating on
cancel_run/resolve_gate, gate-parkingverification on denied/cancelled resolutions, explicit rejection of
persistent (
always: true) approvals, and aThreadScopeMismatch→NotFoundremap that prevents existence leaks. Five regression testscover those paths. The earlier-added
WebUiService,FakeWebUiService,and
tests/webui_service_contract.rsare removed so the crate exposesexactly one facade for browser handlers — satisfying #3611 acceptance
criterion #3.
Dependency
The follow-up handler PR that closes #3611 is blocked on
#3683 (host-owned
ingress contracts, closes #3578). Handlers need #3683's
RouteDescriptor/IngressPolicyvocabulary to declare auth / CORS /body / rate / streaming policies, and #3683 also introduces the
architecture guardrail that forbids product crates from binding
listeners directly. This PR has no such dependency and is reviewable
on its own.