Skip to content

feat(slack): refuse unresolved own-team members as their own state - #628

Open
yzxcj797 wants to merge 1 commit into
yc-software:mainfrom
yzxcj797:fix/email-identity-third-state-626
Open

feat(slack): refuse unresolved own-team members as their own state#628
yzxcj797 wants to merge 1 commit into
yc-software:mainfrom
yzxcj797:fix/email-identity-third-state-626

Conversation

@yzxcj797

@yzxcj797 yzxcj797 commented Aug 20, 2026

Copy link
Copy Markdown

Fixes defects 1 and 2 of #626 (the classification collapse and the invisible refusal); the ok:false propagation (1b) and event-driven refresh (3) are separate follow-ups per the issue's fix ladder.

Defect 1 — the third state

classifyUser folded three states into two: confirmed-external → guest; confirmed-internal-resolved → allowed; and own-team-but-email-invisible → also "guest". An org owner with a just-set email masqueraded as an external guest for the directory-propagation window, with every log line and the user-facing copy asserting "external".

Now ActorAssertion carries principalUnresolved: true for that third case:

  • Native flags still decide isExternalGuest — a restricted/other-workspace/stranger member with a visible email remains an email-keyed external guest exactly as before (pinned by the pre-existing tests, now joined by a "native evidence wins" case). The third state never masks the flags.
  • Email keying is unchanged for everyone with a visible email.
  • slack-id mode never sets it (no email resolution exists there).

Defect 2 — the refusal is its own state, and observable

The turn-handler gate refuses principalUnresolved ahead of the external-guest gate — equally fail-closed (never a fallback Slack-ID principal, which would mint a second internal identity with wrong scope/grants, exactly what email mode exists to prevent), but:

  • the sender sees "I couldn't verify your team membership (your email isn't visible to me). This usually resolves within a few minutes of the directory refreshing; try again shortly" — naming the actual condition and its actual remedy, instead of "this conversation isn't fully internal", which sent the owner looking for a channel-setting problem;
  • every refusal logs on the success path — the new principalUnresolved refusal warns with user/channel/timestamp, and the external-guest refusal (which previously left no trace unless postEphemeral threw) now warns too. In the incident, H1 (path never ran) / H2 (ephemeral posted but not displayed) / H3 (hung await before the ephemeral) were indistinguishable precisely because a successful refusal was invisible; the success-side log line makes the next one decidable.

Tests

  • Updated: "email mode fails closed to guest when a member has no visible email" → asserts isExternalGuest: false + principalUnresolved: true. Fails on main (guest).
  • New: "native external evidence still wins over unresolved" — restricted + email-less reads as external guest, principalUnresolved unset. Pins that the third state cannot mask the flags.
  • New: "slack-id mode never marks principalUnresolved".

slack-identity 27/27; neighbors external-slack-participants / slack-index.integration / identity-offboarding 53/53 (all three suites identical on clean main); tsc --noEmit clean. The refusal-gate copy and logging are exercised through the integration suite's mention fixtures.

Follow-ups (not included, per the issue's ladder)

  • ok:false from users.info preserved at classifyActor and the direct-intake path, distinguishing "lookup failed" from "resolved but email-less" — the transient-failure half of defect 1.
  • user_change/team_join manifest subscriptions + cache eviction — defect 3; needs the app-reinstall note the issue mentions.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

In email identity mode (the default posture of a standard deployment —
the CLI scaffold hardcodes SLACK_IDENTITY_EMAIL=1 for Fly and AWS), an
own-team member whose email the directory can't currently see was
classified isExternalGuest — an org owner with a just-set email
masqueraded as an external guest for the ~39-minute Slack propagation
window, and the refusal copy, logs, and audit all said "external" when
the truth was "own-team, principal unresolved" (yc-software#626).

classifyUser now keeps the third state distinct: email-less members
with no Slack-side external evidence carry principalUnresolved (native
flags still decide isExternalGuest, and email keying is unchanged — a
restricted member with a visible email remains an email-keyed guest).
The turn-handler gate refuses principalUnresolved members ahead of the
external-guest gate — equally fail-closed, never a fallback Slack-ID
principal — but with its own user-facing copy (names the directory
refresh, tells the sender to retry) and its own log line.

The external-guest refusal path now logs on success too: the incident
in yc-software#626 was undiagnosable precisely because a successful refusal left
no trace, leaving H1/H2/H3 indistinguishable from the record.

Not included: preserving users.info ok:false at callers (defect 1b)
and user_change/team_join event-driven refresh (defect 3) — separate
follow-ups per the issue's fix ladder.

Fixes defects 1 and 2 of yc-software#626
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