feat(slack): refuse unresolved own-team members as their own state - #628
Open
yzxcj797 wants to merge 1 commit into
Open
feat(slack): refuse unresolved own-team members as their own state#628yzxcj797 wants to merge 1 commit into
yzxcj797 wants to merge 1 commit into
Conversation
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
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes defects 1 and 2 of #626 (the classification collapse and the invisible refusal); the
ok:falsepropagation (1b) and event-driven refresh (3) are separate follow-ups per the issue's fix ladder.Defect 1 — the third state
classifyUserfolded 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
ActorAssertioncarriesprincipalUnresolved: truefor that third case: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.Defect 2 — the refusal is its own state, and observable
The turn-handler gate refuses
principalUnresolvedahead 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:principalUnresolvedrefusal warns with user/channel/timestamp, and the external-guest refusal (which previously left no trace unlesspostEphemeralthrew) 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
isExternalGuest: false+principalUnresolved: true. Fails onmain(guest).principalUnresolvedunset. Pins that the third state cannot mask the flags.slack-identity27/27; neighborsexternal-slack-participants/slack-index.integration/identity-offboarding53/53 (all three suites identical on cleanmain);tsc --noEmitclean. 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:falsefromusers.infopreserved atclassifyActorand the direct-intake path, distinguishing "lookup failed" from "resolved but email-less" — the transient-failure half of defect 1.user_change/team_joinmanifest subscriptions + cache eviction — defect 3; needs the app-reinstall note the issue mentions.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.