Skip to content

fix(bridges): preserve account ownership and await durable test cleanup - #11

Merged
Hmbown merged 3 commits into
mainfrom
fix/bridge-approvals-turn-drain-20261007
Oct 7, 2026
Merged

Hmbown merged 3 commits into
mainfrom
fix/bridge-approvals-turn-drain-20261007

Conversation

@Hmbown

@Hmbown Hmbown commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Bridge account changes must not transfer pending work or reply context to a different account. This mirrors canonical Engine a12bce903b9eac5bc99210c3bead58059fb9a4c4, including admissions that have not yet received a thread ID, explicit replacement with preserved private receipts, receipt bounds, owned-thread filtering and the tested production cursor path. Windows fixture cleanup waits for durable turn completion.

All 44 integration files match committed Engine source byte for byte. The reviewed integrations wiki page is resealed against its complete 16-source list. Plugin bundle contents are unchanged.

Validation: marketplace contract and wiki freshness (3 fresh, 0 stale); npm test 1032 passed, 0 failed, 27 skipped; browser checks 13 passed, 0 failed, 9 skipped; git diff --check passed. Three new production-process regressions cover unbound admission, unbound delivery and missing account ownership. Exact-head Linux, macOS, Windows and CodeQL checks must pass before merge. No deployment or publication.

Vendor the exact canonical Engine 4a3001df706c053a2be5f48b907eefeea1fbfee8
bridge repairs: bind Weixin context tokens to their owning account, filter
owned thread ids before pagination, use the production cursor helper, and
stop before discarding over-limit retired account receipts. Wait for durable
bridge turn completion before test cleanup to avoid the Windows ENOTEMPTY
race. All 44 upstream manifest blobs match the committed Engine source.

Validation: npm test 1029 passed, 0 failed, 27 skipped. npm run check:web
13 passed, 0 failed, 9 skipped. git diff --check passed. Native Windows,
macOS and Linux hosted gates remain required for this exact branch head.

Signed-off-by: Hunter B <hmbown@gmail.com>
Copilot AI balanced review requested due to automatic review settings October 7, 2026 04:24

@devin-ai-integration devin-ai-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.

Devin Review found 2 potential issues.

Devin Review

Comment on lines +871 to +875
if (!state?.bindingAccountId || state.bindingAccountId === accountIdentity()) {
await threadStore.patchChat(fromUser, {
contextToken: msg.context_token,
contextTokenAccountId: accountIdentity(),
});

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.

🟡 Unbound pending conversation loses its context token

When accounts change before thread creation, handleInbound replaces the pending conversation's old context token with the new account's token. pendingAdmission already belongs to the old account, but bindingAccountId is still absent. The old token is lost from the receipt retained by /new.

Learn more

The first prompt saves its account-owned pendingAdmission before reconcileAdmission creates a thread and assigns bindingAccountId. If the bridge restarts under another bot account during that interval, the old chat can have an account-owned pending request but no binding. An incoming message from the new bot replaces its stored context token because the guard checks only bindingAccountId. An explicit /new then archives the already-modified state, not the original token.

Example: Bot A receives a prompt with context token A-token and persists pendingAdmission, then stops before creating a thread. Bot B receives /new with B-token; the retained Bot A receipt now contains B-token instead of A-token.

Recommended fix: Treat pendingAdmission.accountId and turnDelivery.accountId as existing ownership when deciding whether to update the current chat's context token. Keep the new inbound token available for /new to bind after retiring the untouched old state.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +689 to +691
const query = new URLSearchParams({ limit: "8", include_archived: "true", thread_ids: ownedIds.join(",") });
const threads = (await runtimeJson(`/v1/threads/summary?${query}`))
.filter((thread) => ownsThread(chatId, state, thread.id));

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.

🔍 Confirm summary filtering precedes pagination

The fixture records thread_ids but does not filter or paginate its response. Confirm that Engine applies this parameter before limit=8; otherwise /threads can still omit owned conversations.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Hosted checks correctly rejected the integration page's stale source seal.
Review the changed canonical sources, describe context-token account binding,
retired receipt bounds and filter-before-limit behavior, then reseal the page
against its existing 16 declared sources. No plugin bundle bytes change.

Validation: npm run check passed (19 plugins, 47 skills); check:wiki reports
3 fresh, 0 stale/touched/orphaned/unsealed pages. check:web 13 passed, 0 failed,
9 skipped. Prior unchanged-source Node gate: 1029 passed, 0 failed, 27 skipped.
git diff --check passed. Exact-head three-OS CI remains required.

Signed-off-by: Hunter B <hmbown@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Account-isolation and recovery changes need human validation, and stale wiki seals block the required platform checks.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Synchronizes marketplace bridge integrations with canonical Engine repairs while leaving plugin bundles unchanged.

Changes:

  • Binds Weixin reply tokens to accounts and bounds retained receipts.
  • Updates owned-thread queries and shares cursor-processing logic.
  • Awaits durable test cleanup and refreshes upstream metadata.
File Description
integrations/​weixin-bridge/​test/​runtime.test.mjs Adds ownership, receipt-limit, and query assertions.
integrations/​weixin-bridge/​src/​index.mjs Updates account isolation, retention limits, and polling.
integrations/​weixin-bridge/​README.md Documents receipt limits and manual reconciliation.
integrations/​upstream.json Refreshes the canonical commit and file hashes.
integrations/​bridge-core/​test/​approvals.test.mjs Waits for durable turn cleanup before teardown.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread integrations/upstream.json Outdated
{
"repository": "https://github.com/codewhale-hq/Codewhale",
"commit": "e09cf47bb84a0d84e7fcd7a3c71d885edd440025",
"commit": "4a3001df706c053a2be5f48b907eefeea1fbfee8",
Mirror canonical Engine a12bce903b9eac5bc99210c3bead58059fb9a4c4. New inbound context cannot overwrite a foreign or unattributed pending receipt; explicit /new retains the original token before binding the new account. All 44 canonical integration files match Engine. Reviewed and resealed the wiki page with all 16 source paths.

Validation: marketplace contract and wiki freshness passed (3 fresh, 0 stale); npm test 1032 passed, 0 failed, 27 skipped; npm run check:web 13 passed, 0 failed, 9 skipped; git diff --check passed. Three new production-process regressions. Exact-head hosted checks still required.
@Hmbown
Hmbown merged commit 8cecc3b into main Oct 7, 2026
8 checks passed
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