Skip to content

fix: report unlinked pinned accounts instead of organization denial - #86

Merged
shuv1337 merged 4 commits into
v2from
fix/pinned-account-unlinked-error
Nov 10, 2026
Merged

shuv1337 merged 4 commits into
v2from
fix/pinned-account-unlinked-error

Conversation

@shuv1337

@shuv1337 shuv1337 commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

Change

  • Report ConnectionAccountUnlinked (HTTP 409), not OrganizationForbidden, when a connection targets accounts but none remain among the caller's usable app links. Covers plain and many() requirements, HTTP tool discovery/calls, MCP search/describe/calls and their shared account-resolution consumers.
  • Keep membership/app authorization checks and non-connection grants unchanged. Messages name only IDs already in the caller's connection policy—no personal account labels or credential lookups.
  • Show Needs account — save to relink in the existing connection editor, or Needs account — saving replaces when relinking replaces a single-account requirement's existing link. Saving an otherwise unchanged connection can restore its usable pinned account's app link; unavailable accounts remain blocked by existing selection validation.

Error shape

HTTP 409:

{
  "_tag": "ConnectionAccountUnlinked",
  "app": "app_...",
  "accounts": ["acc_..."],
  "message": "This connection runs as account acc_..., which is no longer linked to this app for you. Relink it, or edit the connection to run as another of your accounts.",
  "recovery": { "action": "Relink the account, or edit the connection to run as another account.", "instructions": "..." }
}

MCP preserves the stable code and status in its error projection/diagnostics, with recovery and retryable: false. Several pinned IDs use the plural message.

Review fixes

  • d7c3f106f: replaced the nonexistent operator/admin reset recovery with owner-only relinking or editing the connection to run as another owned account. Instructions explicitly say organization admins cannot edit another member's connection, explain both recovery entry points, warn that relinking a single-account app replaces the current link, and retain the prohibition on changing roles or bypassing account access. Updated HTTP and MCP expectations, including the full HTTP recovery instructions.
  • 3266d8292: show Needs account — saving replaces before replacing a single-account link. Extended both pinned-account browser scenarios to inspect the warning and save: a plain requirement replaces the other link, while many() retains it. Error responses still do not disclose account labels.
  • Discard behavior is deliberately unchanged to keep this fix small: it restores the saved draft, but if the saved connection still needs relinking, relinking keeps the save bar visible. Discard does not repair or dismiss the missing-link condition; leaving the editor makes no changes.
  • Follow-up: requireConnectionAccounts in apps/hosted/server/src/implementation/app-accounts.ts uses .some across requirements. For apps needing several providers, one provider's still-linked pinned account suppresses the new error even when another provider's pinned account is unlinked; those cases fall back to the old errors. Per-provider missing-link diagnosis is not fixed here.
  • Follow-up: audit the possibly-dead hosted GrantForbidden target explanation path in apps/hosted/server/src/implementation/mcp.ts (explained, formerly lines 119–145). This PR does not remove or establish reachability of that path.
  • 3dc3d7c6d is an unrelated preparatory skill-digest refresh required for bun run check, not part of the pinned-account fix.

Review-fix verification

  • bun run format, bun run check, and bun run e2e:prepare: passed (check reports existing Effect warnings).
  • bun run e2e:self-host --test-name 'Pinned (many|plain)-account connections|pinned account is replaced during an execution': 3 passed, covering both touched specs (connection-account-unlinked.spec.ts and mcp-connections.spec.ts).
  • Regression proof: before the UI fix, the new plain-account assertion failed waiting for Needs account — saving replaces Other private label. After the fix, both cardinalities and the mid-execution scenario passed.
  • Inspected both captured replacement/relink states; UI detector returned no findings.
  • Final evidence: .local/e2e/2026-11-10T01-31-55.884Z-self-host-bc4144; regression evidence: .local/e2e/2026-11-10T01-30-57.657Z-self-host-255997.

Original implementation verification

  • bun run format and bun run check: pass.
  • bun run e2e:prepare: pass.
  • Focused self-host E2E: 5 distinct scenarios passed (no full suite):
    • New pinned many() and plain-account cases: member unlinks own pinned account; zero links and another linked account; HTTP list/index/get/call; MCP call/search/describe; no labels disclosed; browser needs-account status and save-to-relink recovery; unrestricted calls still work; genuine app-access denial remains OrganizationForbidden.
    • Existing selected-account/bare-account scenario.
    • Existing mid-execution account replacement scenario, updated to assert the new accurate error instead of GrantForbidden.
    • Existing dashboard create/revoke scenario.
  • Regression proof: temporarily disabled the new check; the many-account scenario failed on the original 403 OrganizationForbidden versus expected 409. Restored the fix and reran successfully.
  • Inspected the captured connection-editor state; the UI detector reported no findings.
  • Cloud execution was not run locally. No production access, deployments or merge performed. The incident's original unlink cause remains uninvestigated; the resolver filters links by current usability, so a missing usable link alone does not establish how it disappeared.

Evidence (worktree-relative):

  • Regression: .local/e2e/2026-11-10T01-12-34.419Z-self-host-181685
  • New scenarios, final: .local/e2e/2026-11-10T01-17-20.773Z-self-host-80042a
  • Account selection / mid-execution: .local/e2e/2026-11-10T01-15-22.143Z-self-host-f8d709
  • Dashboard: .local/e2e/2026-11-10T01-16-20.375Z-self-host-986121

The unrelated preparatory commit 3dc3d7c6d refreshes the stale app-authoring/SKILL.md digest left by baseline commit 3a60a9928; the unchanged generator produced it, allowing bun run check to pass.

connection-account-unlinked.spec.ts
mcp-connections.spec.ts
hosted-app-accounts.spec.ts
app-account-links.spec.ts
app-account-app-links.spec.ts
local-mcp-connections.spec.ts

@shuv1337
shuv1337 merged commit 5c4afbe into v2 Nov 10, 2026
4 checks passed
@shuv1337
shuv1337 deleted the fix/pinned-account-unlinked-error branch November 10, 2026 01:36
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.

1 participant