# PR #17031 audit (reviewer B)

Reviewed SHA: 34fcb08ce3146c0b6424d676b1d05ec8b49014c4 (`git diff origin/main...HEAD`, 16 files, +342/-36)
Worktree: /home/ubuntu/wt/pr-17031 (read-only)

## Verdict: MERGE

## What I checked
- `htmlRenderFromToolItem(item, threadId)` (packages/shared/src/toolOutput.ts:203): `execute` references are accepted only when `attachmentIdBelongsToThread` matches. All call sites pass the item's owning thread: web (`row.sourceThreadId`), mobile (`row.sourceThreadId`), the GuestShareService render path (`thread.id`), and attachmentStore cleanup (`threadId`). The ACP adapter call sites only use direct html_render names, so the missing threadId is harmless there.
- `attachmentIdBelongsToThread`: anchors the full uuid and optional extension after the segment, so a `<segment>-foo-<uuid>` from a sibling thread with a dash-extended ID does not match.
- The bare `-html` fallback in `compactDynamicToolOutput` is used only when `data.htmlRender` is absent. Rendering still goes through the toolName gate.
- `readResult`: `block.text ?? block.content` handles ACP `{type:"content",content:{type:"text",text}}`. `isTextContentBlock` unwraps only pure MCP text blocks, so an object with extra keys keeps its outer metadata. The earlier bot findings on this are resolved.
- ACP: `acpMcpToolCallOutput` Grok `output.OkayOutput` unwrap only fires when there is neither `result` nor `error`. Child (subagent) MCP output is now normalized the same way as parent output (adapter.ts:2675). The snake_case `server_name`/`tool_name` and `cognition.ai/inferenceToolName` candidates are additive. `rawInput.toolName` is used only when it is qualified `mcp__x__y`, so a CodeBuddy bare `ToolSearch` is no longer misread. The Grok " — source file:" annotation is stripped before matching.
- ProjectionStore `LIKE '%-html%'` is an added OR on an already-filtered dynamic_tool scan. It only widens which rows are parsed, and the segment check still filters them. Cost is negligible.
- `toSafeThreadAttachmentSegment` moved into shared unchanged, and the server re-exports it, so existing imports still work.

## Non-blocking notes
- saphid's 80-char segment collision (`thread:mcp:<36>:<36>:0` vs `:1`) is real but predates this PR: every attachment ID already uses the truncated segment. Under the trusted-execute decision, exploiting it needs a script in the sibling thread that returns the sibling's exact attachment ID. This is not a regression from this diff and should be tracked separately if wanted.

## Diagnostics run
- `vp test run` on shared toolOutput/htmlRender, provider-acp runtimeModel, web session-logic and mobile threadActivity: 5 files, 167 tests passed.
- `vp test run apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts`: 66 tests passed.

## Verification limits
- Did not run typecheck, lint or `check:changed`, and did not check CI for this head. The forge CI at ed7d8da is stale.
- No live ACP providers (Devin/CodeBuddy/Grok/OpenCode) and no browser or mobile client runs. Behaviour is checked through unit and adapter tests and by reading the source.
- Did not re-run the "test fails before fix" check myself.