Skip to content

Fix Slack continuation replies and Content provenance#2223

Open
3mdistal wants to merge 1 commit into
mainfrom
codex/slack-qa-followups
Open

Fix Slack continuation replies and Content provenance#2223
3mdistal wants to merge 1 commit into
mainfrom
codex/slack-qa-followups

Conversation

@3mdistal

Copy link
Copy Markdown
Contributor

Summary

  • suppress the framework unverified-artifact warning when a durable queued A2A continuation owns the final Slack response, without weakening artifact URL verification
  • carry validated Slack platform and thread provenance through authenticated A2A metadata and receiver-generated hidden context
  • teach Content intake to submit both the exact source thread and Submitted via = Slack only when the live form exposes unique enabled matches
  • preserve the successful-mutation and delivery-only-retry contract from Fix Slack reply recovery after successful mutations #2212

Verification

  • 233 focused tests across artifact guarding, A2A client and receiver validation, webhook suppression, continuation terminal delivery, and call-agent forwarding
  • pnpm --filter @agent-native/core run typecheck
  • pnpm changeset status
  • oxfmt on modified TypeScript
  • git diff --check
  • independent final code review: no blockers

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:

  1. Create one disposable Design Ask from Slack.
  2. Confirm there is no preliminary unverified-artifact warning, exactly one verified terminal reply, one Content row, and Submitted via = Slack when the live form option is enabled.
  3. Rename the row in Content.
  4. Correct an unrelated field in the same Slack thread.
  5. Confirm the same Content ID and live title remain, no duplicate appears, and Slack posts exactly one truthful confirmation.

No production mutation or live Slack reproduction was run in this PR.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Visual recap — screenshot failed

A 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

@builder-io-integration builder-io-integration 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.

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.

Comment on lines +1097 to +1099
suppressPlatformReply ||=
queuedA2AContinuation &&
guardedResponse.rejectedUnverifiedArtifactReferences;

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.

🟡 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.

Fix in Builder

if (incoming?.platform !== "slack" || !incoming.sourceUrl) return undefined;

try {
const sourceUrl = new URL(incoming.sourceUrl);

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.

🟡 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.

Fix in Builder

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants