test(e2e): convert chat-harness-subagent to llmKeywordRules (#4517)

[CodeGhost21]

Summary

• Convert chat-harness-subagent.spec.ts from the shared llmForcedResponses FIFO to llmKeywordRules — content-addressed, never depleted.
• Rename per-turn tokens (llama-research / coded marker phrase / researcher trace signal) so each of the 3 turns routes to exactly one rule via pickProbeText's substring match.
• No product/runtime change; E2E-mock fixture only.

Problem

Follow-up to #4519 for #4517. #4519 stopped the fire-and-forget thread-title-gen call (chat_with_system, tools: None) from draining the FIFO queue, but the orchestrator→researcher fan-out spec was left as its own known follow-up (see #4519 body: "the genuinely multi-agent spec chat-harness-subagent (orchestrator→researcher) additionally needs its FORCED_RESPONSES to cover the sub-agent's own model calls (or convert to llmKeywordRules, which are content-addressed and never shifted)").

The residual failure mechanism: the delegated researcher runs its own agent-harness loop, so the mock's global FIFO can still be shifted by an off-turn call. The scripted final canary then lands on the wrong turn (or never renders). Keyword rules are matched by a case-insensitive substring of the latest user/tool message per call and are never consumed, so any extra ancillary call that doesn't match a rule falls through to the mock's dynamic default and leaves the scripted turns intact.

Solution

app/test/e2e/specs/chat-harness-subagent.spec.ts:

• Swap llmForcedResponses → llmKeywordRules in the before/after hooks.
• Three content-addressed rules keyed on tokens that are disjoint across the three probe surfaces:

Turn	Probe (latest user/tool msg)	Keyword
Orchestrator turn 1	user PROMPT = "Please delegate a llama-research task and return the marker."	llama-research
Sub-agent (researcher)	"Task:\n" + DELEGATE_PROMPT (rendered by render_structured_handoff in src/openhuman/agent_orchestration/tools/archetype_delegation.rs)	coded marker phrase
Orchestrator turn 2 (final)	role: tool msg content = RESEARCHER_REPLY (via ToolResult::success(outcome.output) in src/openhuman/agent_orchestration/tools/dispatch.rs)	researcher trace signal

The title-gen call from app/src/store/threadSlice.ts shares the turn-1 probe; it matches the llama-research rule and gets both content and tool_calls back. chat_with_system (no tools) consumes content ("Delegating to researcher.") as the title and ignores unexpected tool_calls — benign for both callers.

Validated against the mock in isolation (in-process, non-stream and stream): each of the three turn shapes routes to its intended rule and returns the expected content/tool_calls; the final canary lands as the turn-2 response.

Submission Checklist

• Tests added or updated — this PR is the E2E fixture correction; the happy path (single-turn tool-round → final canary) is exercised by the existing 3 it blocks (canary in DOM, IN_FLIGHT drains after chat_done, persisted thread JSONL contains the canary); failure-path signal is preserved via expect(content).toContain(CANARY_FINAL) and the ≥2 mock-hit count check.
• Diff coverage ≥ 80% — N/A: change is E2E test-fixture TS in app/test/e2e/specs/, no product src lines under coverage.
• Coverage matrix updated — N/A: test-infra only, no feature IDs change.
• All affected feature IDs from the matrix are listed in the PR description under ## Related — N/A: no feature IDs affected.
• No new external network dependencies introduced — mock backend only.
• Manual smoke checklist updated if this touches release-cut surfaces — N/A: does not touch release-cut UX.
• Linked issue closed via Closes #NNN in the ## Related section — N/A: this PR only fixes one of the two specs Desktop chat tool-round E2E specs fail: TinyAgents harness model-call sequence desyncs the mock forced-response queue #4517 tracks; leaving the issue open for the sibling chat-harness-subagent-continue.spec.ts follow-up (details below).

Impact

• E2E mock only. Deterministically fixes the orchestrator→researcher fan-out spec (chat-harness-subagent.spec.ts) that test(e2e): stop ancillary LLM calls draining the mock forced-response queue (#4517) #4519 flagged as still-failing after its own mock-side gating landed. No runtime, product, or platform behavior change.

Related

• Refs Desktop chat tool-round E2E specs fail: TinyAgents harness model-call sequence desyncs the mock forced-response queue #4517 (does not close — see follow-up below).
• Follows test(e2e): stop ancillary LLM calls draining the mock forced-response queue (#4517) #4519 (the FIFO gating for ancillary no-tools calls that fixed the single-turn specs).
• Follow-up PR(s)/TODOs: chat-harness-subagent-continue.spec.ts shares the same FIFO pattern and has an unresolved {{DYNAMIC_TASK_ID}} literal placeholder in the scripted continue_subagent tool-call args (spec line 86). Converting it to keyword rules alone won't make it pass — continue_subagent needs a real task_id: sub-<uuid> from the last [SUBAGENT_AWAITING_USER] envelope. Options for a follow-up: extend the mock to substitute template placeholders from a message-history parse, or rework the spec to drive the resume via a different mechanism.

---

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

• Key: N/A
• URL: N/A

Commit & Branch

• Branch: fix/4517-harness-subagent-keyword-rules
• Commit SHA: 0308a6cd1

Validation Run

• pnpm --filter openhuman-app format:check — prettier ✓ (spec re-formatted; no other files touched)
• pnpm typecheck — tsc --noEmit on app/tsconfig.json ✓ 0 errors
• Focused tests: in-process mock probe (non-stream + streaming) for all three turn probes and the title-gen collision — see the Solution section above
• Rust fmt/check (if changed): N/A — no Rust changes
• Tauri fmt/check (if changed): N/A — no Tauri changes

Validation Blocked

• command: pnpm debug e2e app/test/e2e/specs/chat-harness-subagent.spec.ts
• error: Full WDIO/CEF chain needs a built Tauri bundle + Appium XCUITest (macOS) or tauri-driver (Linux); not runnable in this session's environment.
• impact: Runtime side of the fix (real agent harness making the /chat/completions calls) will be exercised by Release CI on this PR; in-process mock probes above prove the routing side.

Behavior Changes

• Intended behavior change: none in shipped product; corrects an E2E fixture that was still red after test(e2e): stop ancillary LLM calls draining the mock forced-response queue (#4517) #4519.
• User-visible effect: none.

Parity Contract

• Legacy behavior preserved: yes — the three it assertions (canary in DOM, IN_FLIGHT drains, thread JSONL persists canary) are unchanged; only the scripting mechanism swaps FIFO → content-addressed rules.
• Guard/fallback/dispatch parity checks: the mock's defaultStreamScript handles {content, toolCalls} identically whether it came from the FIFO or from a keyword rule (scripts/mock-api/routes/llm.mjs:249-253 and :264-268); the non-stream path likewise (:622-639 vs :642-668).

Duplicate / Superseded PR Handling

• Duplicate PR(s): none
• Canonical PR: this
• Resolution: N/A

Summary by CodeRabbit

• Tests
  • Updated automated chat flow coverage to better simulate multi-step assistant handoffs.
  • Improved test reliability by matching scripted responses to specific conversation content instead of relying on response order.
  • Kept existing checks for subagent activity, final output, and workspace persistence.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR updates an end-to-end test spec to mock the orchestrator/subagent chat flow using content-addressed llmKeywordRules instead of the previous FIFO llmForcedResponses queue, changing the mock data structure and suite lifecycle configuration accordingly.

Changes

Keyword Rule Mocking Migration

Layer / File(s)	Summary
Keyword rules setup and mocking app/test/e2e/specs/chat-harness-subagent.spec.ts	Header comment rewritten to describe keyword-substring-based turn routing; FORCED_RESPONSES replaced with tokenized constants (PROMPT, DELEGATE_PROMPT, RESEARCHER_REPLY, CANARY_FINAL) and a KEYWORD_RULES array mapping keywords to tool calls or reply text; suite before/after hooks now set and clear llmKeywordRules instead of llmForcedResponses.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: working

Poem

A rabbit hopped through mocked chat lines,
Swapping queues for keyword signs,
No more FIFO to deplete,
Just matching words, precise and neat,
Canary found, the test compiles! 🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main change: converting the chat-harness-subagent E2E test to llmKeywordRules.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

app/test/e2e/specs/chat-harness-subagent.spec.ts (1)

78-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rule order doesn't match the documented A→B→C turn sequence.

Header comment (lines 26-30) lists turns as A) orchestrator, B) researcher, C) orchestrator, but KEYWORD_RULES is ordered B, C, A. Matching is substring-based and order-independent here since the keywords don't collide, so this is purely a readability nit — reordering to A, B, C would make the mapping easier to follow at a glance.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/test/e2e/specs/chat-harness-subagent.spec.ts` around lines 78 - 98, The
KEYWORD_RULES array is ordered B, C, A even though the surrounding comment
documents the turn sequence as A, B, C, so reorder the entries in KEYWORD_RULES
to match that documented flow. Keep the same rule contents in place, just move
the orchestrator rule first, then the researcher rule, then the post-delegation
rule so the mapping is easier to read alongside the comment.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/test/e2e/specs/chat-harness-subagent.spec.ts`:
- Around line 78-98: The KEYWORD_RULES array is ordered B, C, A even though the
surrounding comment documents the turn sequence as A, B, C, so reorder the
entries in KEYWORD_RULES to match that documented flow. Keep the same rule
contents in place, just move the orchestrator rule first, then the researcher
rule, then the post-delegation rule so the mapping is easier to read alongside
the comment.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0e351ec4-409f-4460-a068-a99224832cc8

📥 Commits

Reviewing files that changed from the base of the PR and between 8018eeb and 0308a6c.

📒 Files selected for processing (1)

• app/test/e2e/specs/chat-harness-subagent.spec.ts