Fix Slack continuation replies and Content provenance#2223
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Visual recap — screenshot failedA recap was published, but the PR-comment screenshot could not be captured or uploaded. Open the interactive recap directly: Open the full interactive recap Diagnostic: light: page.waitForSelector: Timeout 30000ms exceeded. Call log: - waiting for locator('[data-plan-document]') to be visible; dark: page.waitForSelector: Timeout 30000ms exceeded. Call log: - waiting for locator('[data-plan-document]') to be visible |
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
Code Review Summary
PR #2223 updates the core A2A and integration pipeline so authenticated Slack provenance can survive queued cross-app continuations, Content intake can use that provenance when the live form has unambiguous matching fields, and provisional artifact URL warnings can be withheld while a continuation owns the final reply. The overall design is sound: provenance is receiver-validated only after A2A authentication, sensitive metadata is stripped from responses, the legacy artifact-link helper remains compatible, and the continuation path uses atomic delivery claims. The focused tests provide good coverage across transport, validation, queue processing, and delivery behavior.
Risk level: High, because the change modifies authenticated metadata handling and cross-app data provenance.
Key findings
- 🟡 MEDIUM — A queued continuation can suppress an entire substantive parent response after any unverified artifact reference is detected, potentially hiding a completed local mutation or verified result.
- 🟡 MEDIUM — The sender-side Slack URL validation does not reject leading/trailing whitespace, unlike the receiver validator, so provenance can be forwarded and then silently discarded downstream.
🧪 Browser testing: Skipped — PR only modifies backend/core integration behavior, tests, and skills/changelog; no browser UI impact.
| suppressPlatformReply ||= | ||
| queuedA2AContinuation && | ||
| guardedResponse.rejectedUnverifiedArtifactReferences; |
There was a problem hiding this comment.
🟡 Keep substantive parent results when guarding provisional URLs
When a queued continuation exists, this condition suppresses the entire parent reply whenever guardA2AArtifactResponse rejects any unverified artifact-shaped URL. That can discard a substantive local mutation result or other verified parent output in a mixed workflow; limit this suppression to the deferred/non-substantive response case, or preserve the substantive parent content while withholding only the provisional warning.
Additional Info
Reported by 1 of 3 parallel code-review agents.
| if (incoming?.platform !== "slack" || !incoming.sourceUrl) return undefined; | ||
|
|
||
| try { | ||
| const sourceUrl = new URL(incoming.sourceUrl); |
There was a problem hiding this comment.
🟡 Reject whitespace-padded Slack provenance URLs before forwarding
The sender-side validator parses and forwards the original incoming.sourceUrl without checking that it is already trimmed, while trustedSourceContext on the receiver rejects sourceUrl !== sourceUrl.trim(). A whitespace-padded Slack permalink can therefore be sent as structured provenance but be silently dropped by the receiver; apply the same trim validation before constructing the source context.
Additional Info
Reported by 1 of 3 parallel code-review agents.
Summary
Verification
Human QA
The real successful-user story requires the production Slack bot; there is no preview Slack environment. After merge and clean deployment, run the frozen story:
No production mutation or live Slack reproduction was run in this PR.