Skip to content

chore(flags): retire phoenix-admin-console — requireAdmin becomes the sole gate (#3671)#3728

Merged
usirin merged 1 commit into
mainfrom
umut/3671-retire-admin-console-flag
Jul 20, 2026
Merged

chore(flags): retire phoenix-admin-console — requireAdmin becomes the sole gate (#3671)#3728
usirin merged 1 commit into
mainfrom
umut/3671-retire-admin-console-flag

Conversation

@usirin

@usirin usirin commented Jul 20, 2026

Copy link
Copy Markdown
Member

Retires the released phoenix-admin-console flag (admin-console shell, #2740), which serves on@100% in prod. Only the served-on path remains.

Closes #3671

Authorization — unchanged, still server-authoritative

This gates an admin surface, so the central question is what still gates it. The flag was only ever an additional rollout gate on top of a real capability check, at both of its two authorization-relevant sites:

1. admin.probe resolver (worker/features/admin-console/queries.ts). Had two gates: the flag AND requireAdmin (Admin.over(platform)). Only the flag conjunct is removed. requireAdmin survives as the sole gate — yield* Admin keeps the probe row unreachable without a discharged platform-admin grant, and its denial is still the invisible künye Denied, so a non-admin cannot distinguish "not an admin" from "not signed in" (ADR 0107 / ADR 0098 §2). This is the same shape as PR #3699 (requireFunnelAccess + Denied) and PR #3706 (server-side isModerator).

2. overridesAuthorized (worker/features/flagship/override-authz.ts). The flag ANDed into the admin arm: dev || (consoleOn && Admin.over(platform)). The arm collapses to dev || Admin.over(platform)the Admin conjunct survives, so a non-admin's phoenix_flag_overrides cookie stays inert on a deployed stage and an anonymous request still resolves false.

Removing the flag also removes the reason overridesAuthorized had to read its gate against the baseline (no-cookie) context: the verdict now derives only from the environment and the actor's stored platform-admin relation, never from anything the request carries. The "a cookie cannot self-authorize the gate" invariant (#622) is now structural rather than sequenced.

Client side, useAdminProbe drops useFlag. The grant is the server's requireAdmin verdict and nothing else; the hook cannot widen it, and the console chunk is still lazy-imported only past the gate. Net effect: no admin surface becomes reachable without a real permission check.

Gate-parameter decision

overridesAuthorized(baseline) keeps its parameter — baseline.environment === "development" is a live non-flag conjunct (the #622 dev convenience). This is the PR #3725 case, not the PR #3714 case.

CI-vs-prod flag default (#3720)

defaultVariation was "off" (dark-ship), so integration/e2e stages have been asserting the flag-OFF world. Traced blast radius:

  • tests/e2e/31-kullanicilar-darkship.spec.ts — the one e2e spec touching the admin console. It asserts /admin renders no admin-console element. It runs anonymous (page.goto("/admin"), no session), and its own docblock states it does not depend on a signed-in admin. With the flag gone the probe still denies an anonymous caller via requireAdmin, so the assertion holds — for a different (and stronger) reason. Docblock reworded to say so rather than rewriting the assertions. No phoenix_flag_overrides cookie in this suite.
  • override-authz.unit.test.ts — the prod, admin + flag OFF ⇒ NOT authorized case tested a world that no longer exists; deleted. The other four rows survive with the flag knob removed.
  • shell-boot-parity.unit.test.ts — the adminConsoleOn stub knob was the only ON signal for the gate; removed, the Flagship stub now returns the supplied default for every key.
  • src/admin/flags/flags-module.test.ts — asserted DECLARED_FLAGS contains phoenix-admin-console; repointed (below).

One genuine CI-only behavior change, called out for the reviewer: a signed-in non-admin now issues an admin.probe request that returns Denied, where previously the flag short-circuited the wire. In prod this is a no-op (flag was on); no spec exercises a signed-in non-admin against /admin. Nothing was preemptively rewritten beyond the observed cross-references.

Cross-reference repoints

Deleting admin-console.invariant.test.ts orphaned one docblock: user-admin.invariant.test.ts's "mirrors" pointer, repointed to the durable member-mute.invariant.test.ts (#3112). flags-module.test.ts's enumeration assertion repointed to MEMBER_MUTE. Neither target is slated for retirement.

Removed

flags/keys.ts const + DECLARED_FLAGS entry; flagship/resources.ts config + factory + import; alchemy.run.ts import + stack wiring; the flag's invariant test. Stale docblocks reworded across queries.ts, probe-view.ts, fate-module.ts, FlagsContext.ts, request-flags-context.ts, fate/layers.ts, fate/route.ts, App.tsx, AdminConsoleRoute.tsx, useAdminProbe.ts, and the .glossary/TERMS.md "admin console" row.

git grep shows zero surviving references to the flag string / const / factory under apps/web/. The only repo-wide mention left is the deliberate retirement-history sentence in the glossary row.

Verification

  • pnpm typecheck --force — 33/33 successful, 0 cached (no Turbo replay)
  • pnpm lint — clean
  • pnpm vitest run --project unit — 285 files / 2352 tests passed
  • No already-retired flag revived (checked against the ten prior retirements)

🤖 Generated with Claude Code

… sole gate (#3671)

The admin-console shell flag (#2740) serves on@100% in prod, so rip the rollout
gate and keep only the served-on path.

Authorization is unchanged and stays server-authoritative. The `admin.probe`
resolver kept two gates: the flag AND `requireAdmin` (`Admin.over(platform)`).
Only the flag conjunct is removed — the capability check, whose denial is the
invisible `Denied`, was always the authorization and remains the sole gate.
Same shape in `overridesAuthorized`, where the flag ANDed into the admin arm:
the arm collapses to `dev || Admin.over(platform)`, so a non-admin's
`phoenix_flag_overrides` cookie stays inert on a deployed stage. The `baseline`
parameter survives — `baseline.environment` is a live non-flag conjunct.

Removing the flag also removes the reason `overridesAuthorized` had to read its
gate against the baseline (no-cookie) context: the verdict now derives only from
the environment and the actor's stored platform-admin relation, never from
anything the request carries, so a cookie cannot self-authorize by construction.

Client-side, `useAdminProbe` drops `useFlag`; the grant is the server verdict
alone and the console chunk is still lazy-imported only past it.

Deletes the IaC declaration (key const + DECLARED_FLAGS entry, flag config +
factory, alchemy stack wiring) and the flag's invariant test. Repoints the
orphaned `user-admin.invariant.test.ts` cross-reference at the durable
`member-mute.invariant.test.ts`, and the flags-module enumeration assertion at
MEMBER_MUTE.

Closes #3671

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

🚀 Preview deployed

  • web — Stage pr-3728 torn down.

@usirin

usirin commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ 51afecb — merge-ready

Verified PR #3728 against the acceptance criteria of #3671, one at a time.

# Criterion Verdict Evidence
1 phoenix-admin-console FlagGate/useFlag consumers + the dead fallback branch removed; only the live path remains PASS Every consumer collapsed to its live arm: admin-console/queries.ts drops the consoleOn short-circuit so requireAdmin(probeGated()) is the sole gate; src/admin/useAdminProbe.ts drops useFlag(PHOENIX_ADMIN_CONSOLE) from both enabled and granted; flagship/override-authz.ts drops the flag conjunct. No fallback branch survives.
2 IaC declaration + key constant deleted; no phoenix-admin-console reference under apps/web/ (grep-clean) PASS keys.ts (constant + DECLARED_FLAGS row), flagship/resources.ts (ADMIN_CONSOLE_FLAG + adminConsoleFlag), alchemy.run.ts (yield* adminConsoleFlag) all removed. Whole-tree sweep at head: zero phoenix-admin-console hits under apps/web/; the only repo hit is the deliberate historical note in .glossary/TERMS.md. Sweep proven live by a control probe (a must-exist identifier returned hits).
3 The flag's own FlagGate/fallback tests removed; existing behavior tests stay green PASS flagship/admin-console.invariant.test.ts deleted (it asserted only the retired flag's IaC default). Full unit project at head: 285 files / 2352 tests, 2352 passed, 0 failed — identical to the run-evidence bundle.
4 pnpm typecheck and pnpm lint clean PASS Forced (cache-bypassed) turbo run typecheck --force --filter=@kampus/web in the head worktree: exit 0. pnpm lint:worktree: 22 changed files checked, no diagnostics.

Security-critical check — the override-authz.ts gate collapse (graded explicitly)

The gate went from dev || (consoleOn && Admin.over(platform)) to dev || Admin.over(platform). The Admin conjunct genuinely survives on every path, and a non-admin's phoenix_flag_overrides cookie remains inert. Traced through the real call sites, not the diff's framing:

  • Only one non-dev path exists. overridesAuthorized is if (environment === "development") return true; then return Admin.over(platform).pipe(Effect.match({onFailure: () => false, onSuccess: () => true})). There is no third arm — every non-dev request is decided by Admin.over.
  • Admin.over(platform) is relation-tuple-backed authorization, not a flag. Per kunye/admin.ts it discharges a Grant iff (actor, "admin", platform) is in the relation_tuple store, checked fresh per call; denial is the invisible künye Denied, and the anonymous arm is denied before minting.
  • Both real call sites are covered. flagship/request-flags-context.ts (serving /api/flags/evaluate + the edge __BOOT__ injection) and fate/route.ts (the /fate edge) each resolve the verdict against the caller's baseline, no-cookie context and thread it as the third makeRequestFlagsContext arg.
  • The consumption gate is untouched. makeRequestFlagsContext still computes honorOverrides = environment === "development" || overridesAllowed, with overridesAllowed defaulting false; a non-admin on a deployed stage never parses the cookie, so overrides stays undefined.
  • The removed conjunct was the flag, and the widening is admin-only. Authorized set moves from {dev} ∪ {admin ∧ consoleOn} to {dev} ∪ {admin}. Since prod served the flag on@100%, the two are equal in prod; the only delta is where the flag served OFF (CI/preview), and it admits admins only. No non-admin principal is added on any path.
  • The self-authorization invariant is strictly stronger, not weaker. The old code needed care to read the gate flag against the baseline context so a cookie could not flip the flag deciding whether the cookie is honored. With no flag read at all, the verdict now derives solely from the environment and the actor's stored admin relation — nothing the request carries can influence it.
  • The proving tests survive. override-authz.unit.test.ts still asserts prod, non-admin: NOT authorized — an attacker cookie stays inert, prod, anonymous: NOT authorized, and the consumption-side prod, verdict false: the cookie is dropped — no overrides. Ran explicitly at head: 6/6 pass. Only the two rows on the now-nonexistent flag axis were dropped.

No non-flag conjunct was collapsed

Checked every gate expression the diff touches — in each, the removed conjunct is the flag read and the authorization/identity conjunct survives:

  • override-authz.ts: consoleOn && Admin.over(platform)Admin.over(platform) — capability check survives.
  • admin-console/queries.ts: consoleOn guard + requireAdminrequireAdminprobeGated still requires yield* Admin, so the row is unreachable without a discharged grant (compile-enforced).
  • useAdminProbe.ts: enabled: flagOn && signedInenabled: signedIn — the non-flag signedIn conjunct survives; granted drops only flagOn and still rests on the server probe result. Client-side only; the server requireAdmin remains authoritative.

Union-of-removals across the campaign registries

Simulated the merge with git merge-tree --write-tree origin/main <head>: no conflicts, and the merged keys.ts / flagship/resources.ts / alchemy.run.ts are byte-identical to the PR head's (main has not touched those three files since the merge base).

Evidence

Run-evidence bundle cited for 51afecb5 (manifest.commit matches head): checks[] all pass; tests 2352 total / 2352 passed / 0 failed / 0 skipped. CI at head: every check completed, zero failures, none still running — including ci-required, lint / format / typecheck, unit + client tests, integration tests, and the blocking e2e (reads + authed + flows). No allowlisted infra noise was encountered, so no re-run was needed.

Read the PR head (§HEAD): all files under review sourced from 51afecb56ebb7677077aaf7e6185741a90351d93 via a per-run ref + an isolated throwaway worktree (instruction denylist removed and asserted absent), never the launched checkout's working copy.

Advisory (non-blocking)

  • apps/web/worker/features/fate/layers.ts still carries a stale doc comment describing the verdict as "(dev, or admin + flag)" — the sibling comment 120 lines above it was updated to "(dev, or an admin)". Cosmetic only: it holds no flag literal, so criterion 2's grep-clean is unaffected, and no behavior depends on it. Worth a one-line follow-up, not a blocker.

All criteria pass. This PR is merge-ready. review-code does not mergeship-it is the authorized merge step; merging will auto-close #3671 via Fixes #3671.

@usirin

usirin commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

review-design: PASS @ 51afecb — merge-ready

Design-gated PR #3728 over its preview deploy, judging the locally captured bytes (ADR 0165).

Dispatch rationale. class-probe classify --namespaces names this diff has-code + has-ui: it changes apps/web/src/App.tsx, apps/web/src/admin/AdminConsoleRoute.tsx, apps/web/src/admin/useAdminProbe.ts and apps/web/src/flags/keys.ts, all matching the live UI_RE (^apps/web/src/) re-resolved from ship-it/SKILL.md on origin/main. So this gate is dispatched alongside review-code, in lockstep with what ship-it requires.

Preview freshness (load-bearing). The sticky preview comment stamps commit 686be40; that is refs/pull/3728/merge, whose parents are 105ea0b5 (current origin/main) and 51afecb5 (the pinned head). The preview therefore renders this head merged into current main — not a stale build.

Surfaces captured (Playwright over the preview via the @kampus/design-capture seam, desktop viewport):

Surface Rendered Uncaught page errors
/admin — the one route whose behavior this diff changes ordinary not-found page (invisible denial holds) 0
/ — control for the touched App.tsx home renders in full 0

Rendered-surface delta is nil for every reachable visitor on the preview. The App.tsx and AdminConsoleRoute.tsx edits are comment-only. The one behavioral UI change is useAdminProbe dropping the flag conjunct: an anonymous visitor still never probes (the signedIn conjunct survives) and a signed-in non-admin still gets the invisible Denied — both land on the identical not-found page. No new visual surface is introduced.

The six prohibitions (ADR 0162)

# Prohibition Verdict
1 Faint-for-meaning PASS — the bulunamadı heading renders at full strength and the supporting sentence at a muted (AA-safe) step, not --text-faint.
2 Missing focus ring N/A — the diff changes no interactive control, styling, or token; no focus-ring surface is added or altered.
3 Off-grid spacing / type PASS — no measurable break from the 4px lattice on either captured surface.
4 Void empty state PASS/admin resolves to a designed not-found treatment (heading, explanatory sentence, three recovery links), not a blank void or a bare label.
5 Sub-36px tap target PASS — no interactive control is introduced or resized by this diff.
6 Colour-alone meaning PASS — the recovery links carry both colour and underline; no state is signalled by hue alone.

Render-exception check (#2594)

PASS, deterministic. pageErrors is empty on both captured surfaces — zero pageerror, zero console.error. Notably /admin is exactly the route whose mount condition this diff rewrites, and it renders clean.

Golden-deviation class (calibration B, #2945)

N/A. The committed packages/design-capture/golden-pointer.json carries "surfaces": {} — no blessed surfaces exist, so no changed surface has a golden baseline. This class is skipped, not failed.

Evidence note

The screenshot upload to the GitHub user-attachments endpoint returned HTTP 400 (Invalid name for request) for both surfaces, so hostedUrl is null. Per ADR 0165 this is a tolerated, display-only degradation: the verdict is judged on the locally captured bytes, which I read directly as multimodal input, and the upload is out of the decision path. Filing this capture-helper upload fault is a follow-up, not a gate condition.

No blocking violation, no render exception, no golden deviation. This PR is merge-ready on the design class. review-design does not mergeship-it is the authorized merge step.

@usirin
usirin added this pull request to the merge queue Jul 20, 2026
Merged via the queue into main with commit b0beb39 Jul 20, 2026
42 checks passed
@usirin
usirin deleted the umut/3671-retire-admin-console-flag branch July 20, 2026 07:51
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.

Retire phoenix-admin-console flag — rip FlagGate + fallback + IaC declaration (ADR 0136)

1 participant