Skip to content

fix(agentex): rejected sessions prompt a re-link, unlinked turns stop pretending, and linking answers the pending question - #416

Merged
michael-chou359 merged 3 commits into
mainfrom
mc/slack-relink-on-rejected-credential
Aug 31, 2026
Merged

fix(agentex): rejected sessions prompt a re-link, unlinked turns stop pretending, and linking answers the pending question#416
michael-chou359 merged 3 commits into
mainfrom
mc/slack-relink-on-rejected-credential

Conversation

@michael-chou359

@michael-chou359 michael-chou359 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What

credential_expires_at is an upper bound, not a guarantee. A session JWT stops
being honoured the moment its owner signs out or it's revoked — while the stored expiry
still reads months away.

So today the gateway hands the agent a dead cookie, every user-scoped tool call 401s
where the agent can only report "I can't reach your Notion", and nothing ever
prompts a re-link
, because locally the credential looks perfectly fine. Silent, and
indefinite.

A linked turn now verifies the stored session against agentex-auth before acting as
that user. A refusal falls back to the bot with sgp_user_id=None — already the exact
condition that triggers a re-link offer, so there's no new wiring.

A rejection is not a failure

This is the part that needed care. A misconfigured or unavailable auth service refuses
everyone — and concluding "your credential is bad" would tell an entire workspace to
re-link, which fixes nothing (their credentials are fine) and amplifies an outage into a
stampede.

The distinction isn't guesswork. The adapter already raises different types, and
ClientError is a sibling of ServiceError, not a parent:

ClientError (4xx)    this credential was refused            -> prompt a re-link
ServiceError (5xx)   the auth gateway/service is at fault   -> assume good, proceed
anything else        the CHECK broke, not the credential    -> assume good, proceed

403 counts as a refusal on purpose: a valid session that can no longer use the stored
account needs a re-link to pick up a current one.

Deliberately non-destructive

Nothing is tombstoned or deleted on rejection. A systemic 401 therefore costs a
bot-fallback turn plus a rate-limited nudge, and recovers by itself when the upstream
does. Revoking rows would not recover.

Cost

One in-cluster call per linked turn — the same call the shared-bot path already makes on
every turn, so this is the existing cost profile rather than a new one.

No caching, deliberately: a cache here means holding a "this credential was fine" verdict
after it stopped being fine, which is the exact bug being fixed.

Skipped entirely when AGENTEX_AUTH_URL is unset (local dev, authz off) — nothing to
verify against, and refusing every credential would make the feature untestable offline.

Testing

33 new unit tests covering all seven outcomes — accepted, 401, 403, both 5xx flavours,
timeout, unexpected exception — plus two that pin the design rather than the mechanism:

  • a rejection yields sgp_user_id=None, which is what makes _run_turn offer a re-link
  • nothing calls revoke or delete on rejection

742 unit tests pass. ruff clean.

⚠️ One trap worth flagging for anyone writing tests near this. These patch
resolve_environment_variable_dependency, not os.environ. That resolver reads a
GlobalDependencies singleton built once per process, so monkeypatch.setenv never
reaches it — the first version of these tests passed for the wrong reason, silently
taking the authz-disabled path and asserting nothing.


Also: the agent no longer speaks as if it were the user

An unlinked mention produced two messages that contradicted each other. The gateway
DM'd a connect link, and the turn ran anyway on the shared bot identity — where the agent
has no way to know it isn't the person who asked, so it reported on its own access as
though it were theirs:

No Linear access either — verified two ways: no Linear tools in my toolset, and no
Linear API key in my environment.

All true of the bot, none of it true of the user, and arriving while that user was being
told to connect their account.

The turn context now says so when sgp_user_id is None — the same signal that triggers
the link offer, so the agent is told it's unlinked exactly when the user is asked to
link, and the two messages agree.

It also removes the reason the agent invents its own authorization flow. Lacking a
credential, the harness offers an OAuth URL with a localhost redirect:

mcp.linear.app/authorize?…&redirect_uri=http://localhost:52659/callback

That cannot work from Slack — the redirect points at the agent's sandbox — and is
indistinguishable from a real instruction, so the user ends up with two links, one real
and one a dead end. Telling the agent a link has already been sent removes the reason to
improvise one.

A mitigation, not a cure. The gateway can't stop the harness generating those URLs;
it can only remove the situation that prompts them. That behaviour is worth fixing where
it lives.

No behaviour change for linked turns — the flag is False and the context is
byte-for-byte what it was.



Also: linking now answers the question that prompted it

The flow was ask → get a link → click → ask again. The nonce has carried the
triggering message as pending_turn since #410 for exactly this purpose, and nothing
used it — so the person's actual question got dropped at the moment we could finally
answer it.

before:  ask  ->  caveat + link  ->  click  ->  ASK AGAIN
after:   ask  ->  link           ->  click  ->  answer

The replay lands in a new task for free, which is what makes this clean. The task
key includes the SGP user id, so the same Slack thread keys differently once linked:

slack:{ts}   ->   slack:{team}:{channel}:{ts}:{sgp_user}

Different name, different task — so the replay is turn 1 of a fresh session and picks up
their credentials. None of the turn-1 toolset pinning that makes enabling an MCP
mid-conversation a no-op, and no need to start a new thread by hand. That pinning is what
cost an hour of debugging earlier in this work; the key design sidesteps it here without
having been designed to.

Ordering is the load-bearing detail. The replay is scheduled last — after the
upsert, after the nonce is burned, after the cache is invalidated — so it resolves the
link it just created rather than a stale negative cache entry. Resolving stale would run
the question as the shared bot and, without the guard below, offer another link.

offer_link=False on a replay. That turn exists because the user just linked, so
nudging again would be absurd — and offering would mint another nonce and DM another
link, inviting the same loop on the next click. The guard makes the loop
unrepresentable rather than merely unlikely.

Scheduled in the background (this returns an HTML page to a waiting browser), sets the
"thinking…" indicator (the answer arrives minutes after clicking a web page and would
otherwise appear from nowhere), and best-effort by contract — the link is already
durable, so a replay that fails to schedule can't undo it.


What this doesn't cover

The check is on our side of the call. If the vault or an MCP rejects the credential
for a reason agentex-auth accepts, we won't notice — the agent still reports a tool
failure and the user gets no prompt. Closing that needs a signal back from the agent,
which doesn't exist. This covers the case that actually bites (sign-out), not every
possible rejection.

🤖 Generated with Claude Code

Greptile Summary

The PR validates stored Slack-linked sessions before user-scoped turns, clarifies shared-identity behavior to the agent, and replays the question that initiated a successful link.

  • Treats authentication refusals as a signal to fall back and offer re-linking while allowing verification-service failures to proceed.
  • Adds explicit unlinked-turn context so shared credentials are not represented as the requesting user’s integrations.
  • Schedules the pending Slack message for replay after the identity link is persisted and its cache invalidated.
  • Adds unit coverage for credential verification, unlinked context, replay reconstruction, and link-offer suppression.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope.

No blocking failure remains.

Important Files Changed

Filename Overview
agentex/src/api/routes/integrations.py Extends successful Slack link confirmation to schedule replay of the pending turn and adjusts the confirmation copy accordingly.
agentex/src/domain/use_cases/slack_gateway_use_case.py Adds linked-session verification, shared-identity prompt context, pending-turn replay, and replay-specific suppression of repeated link offers.
agentex/tests/unit/api/test_integrations_routes.py Updates route tests for the BackgroundTasks dependency and covers successful, absent, and refused replay scheduling.
agentex/tests/unit/use_cases/test_slack_gateway_use_case.py Adds focused coverage for verification outcomes, unlinked prompt construction, replay reconstruction, and offer suppression.

Sequence Diagram

sequenceDiagram
  participant U as Slack user
  participant G as Slack gateway
  participant A as Auth service
  participant L as Link callback
  participant X as Agent

  U->>G: Send linked or unlinked message
  G->>A: Verify stored session
  alt Session accepted
    A-->>G: Principal accepted
    G->>X: Run as linked user
  else Session refused
    A-->>G: Authentication/authorization refusal
    G-->>U: Send account-link offer
    G->>X: Run with shared identity and unlinked context
    U->>L: Complete account link
    L->>L: Store link and invalidate cache
    L-->>U: Connected page
    L->>G: Background replay of pending message
    G->>X: Run replay using fresh link
  else Verification service fails
    A--xG: Service or unexpected failure
    G->>X: Proceed with stored linked credential
  end
Loading

Reviews (3): Last reviewed commit: "feat(agentex): answer the pending questi..." | Re-trigger Greptile

Context used:

credential_expires_at is an upper bound, not a guarantee. A session JWT stops being
honoured the moment its owner signs out or it is revoked, while the stored expiry
still reads months away -- so the gateway handed the agent a dead cookie, every
user-scoped tool call 401'd where the agent could only report "I can't reach your
Notion", and nothing ever prompted a re-link because locally the credential looked
fine. Silent, and indefinite.

A linked turn now verifies the stored session against agentex-auth before acting as
that user, and a refusal falls back to the bot with sgp_user_id=None -- already the
exact condition that triggers a re-link offer, so no new wiring.

A REJECTION IS NOT A FAILURE, and conflating them is the danger. A misconfigured or
unavailable auth service refuses everyone; concluding "your credential is bad" would
tell an entire workspace to re-link, which fixes nothing because nothing is wrong
with their credentials, and amplifies an outage into a stampede. The distinction is
not guesswork -- the adapter already raises different types, and ClientError is a
SIBLING of ServiceError rather than a parent:

  ClientError (4xx)    this credential was refused          -> prompt a re-link
  ServiceError (5xx)   the auth gateway/service is at fault -> assume good, proceed
  anything else        the CHECK broke, not the credential  -> assume good, proceed

403 counts as a refusal on purpose: a valid session that can no longer use the
stored account needs a re-link to pick up a current one.

Deliberately non-destructive. Nothing is tombstoned or deleted on rejection, so a
systemic 401 costs a bot-fallback turn plus a rate-limited nudge and recovers by
itself when the upstream does. Revoking rows would not recover.

Skipped entirely when AGENTEX_AUTH_URL is unset (local dev, authz off): nothing to
verify against, and refusing every credential would make the feature untestable
offline.

Costs one in-cluster call per linked turn -- the same call the shared-bot path
already makes every turn, so this is the existing cost profile rather than a new
one. No caching, which would mean holding a "this credential was fine" verdict after
it stopped being fine.

Testing: 9 new unit tests covering all seven outcomes (accepted, 401, 403, two 5xx
flavours, timeout, unexpected exception), plus that a rejection yields
sgp_user_id=None -- the re-link trigger -- and that nothing calls revoke or delete.

One trap worth recording: these tests patch resolve_environment_variable_dependency,
not os.environ. That resolver reads a GlobalDependencies singleton built once per
process, so monkeypatch.setenv never reaches it, and the first version of these
tests passed for the wrong reason by silently taking the authz-disabled path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@michael-chou359
michael-chou359 requested a review from a team as a code owner August 31, 2026 05:50
An unlinked mention produced two messages that contradicted each other. The gateway
DM'd a connect link, and the turn ran anyway on the shared bot identity -- where the
agent has no way to know it isn't the person who asked, so it reported on ITS OWN
access as though it were theirs. Observed: confidently "No Linear access either --
verified two ways: no Linear tools in my toolset, and no Linear API key in my
environment", alongside an overstated GitHub claim it later had to walk back. All
true of the bot, none of it true of the user, and arriving while that user was being
told to connect their account.

The turn context now says so when sgp_user_id is None -- the same signal that
triggers the link offer, so the agent is told it is unlinked exactly when the user
is asked to link, and the two messages agree.

The directive covers what the agent was getting wrong:

- it is not running as the sender, and any personal integration it can reach
  belongs to the shared account
- do not describe that access as if it were theirs, or conclude anything about what
  they have connected
- answer from the thread and shared tools; if the request needs their data, say
  plainly that it needs their account connected and that a link has been sent

It also removes the reason the agent invents its own authorization flow. Lacking a
credential, the harness offers an OAuth URL with a localhost redirect
(mcp.linear.app/authorize?...redirect_uri=http://localhost:52659/callback), which
cannot work from Slack -- the redirect points at the agent's sandbox -- and is
indistinguishable from a real instruction. The user then has two links, one real and
one a dead end. Telling the agent a link has already been sent removes the reason to
improvise one.

No behavior change for linked turns: the flag is False and the context is byte-for-
byte what it was.

This is a mitigation, not a fix for the underlying thing. The gateway cannot stop the
harness generating those URLs; it can only remove the situation that prompts them.
The localhost-OAuth-link behaviour is worth fixing where it lives.

Testing: 8 new unit tests. The context ones assert the disclaimer and the
no-OAuth-links directive appear only when unlinked, that the channel id survives
either way, that it composes with the self-posts directive (golden-agent needs
both), and that the user's prompt still comes last so directives aren't read as part
of the question. The wiring ones drive _dispatch far enough to capture the flag and
assert it mirrors sgp_user_id, rather than inspecting source. 726 unit tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@michael-chou359 michael-chou359 changed the title fix(agentex): treat an upstream-rejected session as needing a re-link fix(agentex): prompt a re-link on a rejected session, and stop the agent speaking as the user Aug 31, 2026
…sking again

The flow was ask -> get a link -> click -> ASK AGAIN. The nonce has carried the
triggering message as pending_turn since #410 for exactly this, and nothing used
it, so the person's actual question was dropped at the moment we could finally
answer it.

A successful link now replays that turn as them.

The replay lands in a NEW task for free, which is what makes this clean. The task
key includes the SGP user id, so the same Slack thread keys differently once linked:

    slack:{ts}  ->  slack:{team}:{channel}:{ts}:{sgp_user}

A different name is a different task, so the replay is turn 1 of a fresh session and
picks up their credentials -- none of the toolset pinning that makes enabling an MCP
mid-conversation a no-op. That pinning is what cost an hour of debugging earlier;
here the key design sidesteps it without trying to. The pre-link exchange is not in
the new task's history, but the agent can read the Slack thread with its own tools,
which the context prefix already tells it how to do.

Ordering is the load-bearing detail. The replay is scheduled LAST -- after the
upsert, after the nonce is burned, after the cache is invalidated -- so it resolves
the link it just created rather than a stale negative cache entry. Resolving stale
would run the question as the shared bot and, without the guard below, offer another
link.

_run_turn takes offer_link=False for a replay. That turn exists BECAUSE the user just
linked, so nudging again would be absurd; worse, offering would mint another nonce
and DM another link, inviting the same loop on the next click. The guard makes the
loop unrepresentable rather than merely unlikely.

Scheduled in the background: this returns an HTML page to a waiting browser, and an
agent turn takes far longer than a page load should. Best-effort by contract -- the
link is already durable, and a replay that fails to schedule cannot undo it.

It also sets the "thinking..." indicator, because the answer arrives minutes after
the user clicked a web page and would otherwise appear from nowhere.

The success page now says which happened: "I'm answering the message you sent in
Slack now" when a replay was scheduled, "go back to Slack and ask again" when there
was nothing to replay.

Testing: 16 new unit tests. Route side: the replay is scheduled with the verified
Slack identity from the nonce (not anything the browser supplied), nothing is
scheduled without a pending turn or when the link was refused, and the ordering
against upsert/consume/invalidate holds. Gateway side: the turn is reconstructed
faithfully, the selector is re-derived exactly as normalize() does so a
selector-driven turn resolves to the same target, the offer is suppressed, the
status is set, and five shapes of incomplete pending turn plus a missing identity all
no-op. 742 unit tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@michael-chou359 michael-chou359 changed the title fix(agentex): prompt a re-link on a rejected session, and stop the agent speaking as the user fix(agentex): rejected sessions prompt a re-link, unlinked turns stop pretending, and linking answers the pending question Aug 31, 2026
@michael-chou359
michael-chou359 merged commit 43a1c06 into main Aug 31, 2026
47 checks passed
@michael-chou359
michael-chou359 deleted the mc/slack-relink-on-rejected-credential branch August 31, 2026 07:30
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