Skip to content

fix(orchestrator): truncate long completion relays instead of destroying them - #11811

Merged
lalalune merged 1 commit into
developfrom
fix/rh-2
Jul 3, 2026
Merged

lalalune merged 1 commit into
developfrom
fix/rh-2

Conversation

@NubsCarson

Copy link
Copy Markdown
Member

fix(orchestrator): truncate long completion relays instead of destroying them

Defect

elideLongBlocks (plugins/plugin-agent-orchestrator/src/services/transcript-sanitizer.ts) hard-replaced ANY post-strip text over 2000 chars with the literal string [output elided — N chars] — no truncation, total data loss. Regression from 3781312 (#11605); before that commit the coordinator relayed record.response verbatim.

Two concrete failure paths on develop:

  1. Long pure-prose deliverable destroyed. A dashboard/API-spawned task ("write a detailed migration plan in your final message") whose 2.4KB pure-prose answer contains no tool-output envelopes (strip is a no-op) synthesizes as literally [output elided — 2450 chars]. sessionHasRouterOrigin is false for these sessions so synthesis IS the relay path, and buildTaskResultLine (packages/agent/src/api/server-helpers-swarm.ts) posts completionSummary verbatim to the connector — no LLM pass, no bypass for opencode/codex. The user receives the marker instead of their deliverable.
  2. App-verification verdict shadowed then destroyed. runCustomValidatorAndDispatch spreads enrichedData (retaining the raw ACP finalText in response) and adds summary: "App verification passed.". The coordinator read ladder takes response first, so when finalText exceeds 2KB the user sees the elision marker — and the verdict, which exists ONLY on the custom-validator record (the raw task_complete was withheld until validation), never posts at all.

The router's own relay path never had this problem: composeNarration uses only stripToolTranscript, and its 2KB verbatim cap applies to captured tool-output blocks with a summarized fallback. Synthesis-path-only regression.

Fix

  • elideLongBlocks now truncates: head + … [output truncated — N chars total] marker, with the whole result bounded to maxChars so the defensive re-sanitize in buildTaskResultLine is a provable no-op (idempotent).
  • Coordinator read ladder (swarm-coordinator-service.ts): for custom-validator completions the verdict in record.summary now leads the completion summary, with the sanitized deliverable appended under a budget that keeps the combined text inside the relay cap. response no longer shadows the only record of the validation outcome.

Reproduction (failing tests on unmodified develop)

7 new/updated regression tests fail before the fix, reproducing both scenarios exactly:

× TRUNCATES an over-cap remnant, preserving the head (#11605 destroyed it)
  AssertionError: expected '[output elided — 3000 chars]' not to be '[output elided — 3000 chars]'
× does NOT reduce a long pure-prose deliverable to a bare marker (#11605 regression)
× relays the head of a long pure-prose deliverable instead of destroying it (#11605)
× posts the validated verdict plus the deliverable head when finalText exceeds the relay cap (#11605)
  expected summary.startsWith("App verification passed.") — got '[output elided — 3007 chars]'
...
Tests  7 failed | 56 passed (63)

Verification (after fix)

  • bunx vitest run __tests__/unit/transcript-sanitizer.test.ts __tests__/unit/swarm-coordinator-service.test.ts63 passed (63)
  • Full plugin suite bun run test:unit1276 passed (1276), 118 files
  • Consumer suite packages/agent/src/api/server-helpers-swarm.test.ts13 passed (13)
  • bun run --cwd plugins/plugin-agent-orchestrator typecheck → clean
  • bunx biome check on the four touched files → clean

Evidence notes

  • Real-LLM trajectory: N/A — deterministic string-pipeline defect in the relay sanitizer; the failing-then-passing unit tests drive the exact task_complete event payloads the ACP service emits (response = raw finalText), which is the full real path for this bug (no model in the loop between record.response and the connector post).
  • UI screenshots/video: N/A — no UI surface changed; the connector-visible change is the relayed completion text, asserted byte-for-byte in the tests.

Refs 3781312 (#11605), #11578.

…ing them

elideLongBlocks hard-replaced any post-strip text over 2000 chars with
'[output elided — N chars]', so a legit long deliverable (a 2.4KB
pure-prose plan) synthesized as literally the marker — total data loss
on the synthesis relay path (buildTaskResultLine posts completionSummary
verbatim, no LLM pass). Regression from 3781312 (#11605); pre-commit
the coordinator relayed record.response verbatim.

- elideLongBlocks now truncates: head + '… [output truncated — N chars
  total]' marker, bounded to maxChars so the defensive re-sanitize in
  buildTaskResultLine is a no-op.
- custom-validator completions: the verdict in record.summary ('App
  verification passed.') exists only on that record but was shadowed by
  record.response in the coordinator read ladder. It now leads the
  summary, with the sanitized deliverable appended under a budget that
  keeps the combined text inside the relay cap.

@greptile-apps greptile-apps Bot 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ccf265d5-0272-4768-a357-cf5e8670dca4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/rh-2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@lalalune
lalalune merged commit 763d51a into develop Jul 3, 2026
34 of 38 checks passed
@lalalune
lalalune deleted the fix/rh-2 branch July 3, 2026 04:08
@lalalune

lalalune commented Jul 3, 2026

Copy link
Copy Markdown
Member

Post-merge validation pass from a PR worktree on current develop:

  • Generated standard keyword artifacts with node packages/shared/scripts/generate-keywords.mjs --target ts for test imports.
  • bunx vitest run plugins/plugin-agent-orchestrator/__tests__/unit/transcript-sanitizer.test.ts plugins/plugin-agent-orchestrator/__tests__/unit/swarm-coordinator-service.test.ts --reporter=dot -> 63 tests passed.
  • bun run --cwd plugins/plugin-agent-orchestrator typecheck -> passed.
  • bunx biome check on the four touched files -> passed.
  • bunx vitest run packages/agent/src/api/server-helpers-swarm.test.ts --reporter=dot -> 13 tests passed.
  • bun run --cwd plugins/plugin-agent-orchestrator test:unit -> 118 files / 1279 tests passed.
  • git diff --check origin/develop..HEAD -> passed.

Reviewed the relay cap behavior and custom-validator path. The sanitizer now preserves the head of long pure-prose completions and appends a bounded truncation marker, while validated completions lead with the app-verification verdict and append the sanitized deliverable head.

@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants