fix(console): pin the create/reset flow against the build that offers it - #2121
fix(console): pin the create/reset flow against the build that offers it#2121sanil-23 wants to merge 1 commit into
Conversation
`main` has been red on the Console job since b8a3e2e. That commit moved company creation behind one funnel — `canCreateCompanies` is now `!COMPANY_SWITCHING_HIDDEN && carriesPlatformBearer` — and added the test that pins the hide. What it did not do is update the four older files that assert the presence of the very controls it had just hidden, so ten tests across them have been failing ever since: settings-lifecycle-reset-button the Reset button no longer renders create-company-wallet-mode the dialog's preflight no longer runs, create-company-dialog-preflight-race so the wallet field never appears and submit is never held connection-console-switch-known-status the no-company New trigger is gone `COMPANY_SWITCHING_HIDDEN` ships `true`, so those premises are now unreachable rather than wrong — the flow they cover is still in the tree, it is just not offered. So they declare the configuration they need instead of asserting against one the product does not ship. Mocking the flag here does not weaken the hide. That has its own deliberately-unmocked coverage in `product-scope-hidden-surfaces.test.ts` ("company creation is gone from every trigger, not just the switcher"), which is where a regression in the gate belongs and which still passes untouched. What these four keep testing is the flow underneath it — including the tinyhumansai#1894 pre-archive guard, whose whole job is to stop a reset archiving a company before it knows the replacement has a usable admin. That is worth failing a build over on the day the flag flips back, and deleting it to get to green would have thrown it away. No production code changes: the gate behaves as intended. npm run typecheck, typecheck:unit — clean npm test — 495 files, 4584 tests, 0 failures Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
How this change flows1 changed behaviour across 7 relationships. 5 surrounding behaviours are shown (60 graph nodes walked). 50 further behaviours left out to keep the diagram readable. flowchart LR
n0["client<br/>changed"]:::changed
n1["act"]:::impacted
n2["unmount"]:::impacted
n3["reject"]:::impacted
n4["statusSpy"]:::impacted
n5["render"]:::impacted
n0 -->|uses| n4
n2 -->|calls| n1
n2 -->|tests| n1
n4 -->|calls| n3
n4 -->|tests| n3
n5 -->|calls| n1
n5 -->|tests| n1
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughFour unit tests mock ChangesCompany flow test coverage
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This updates test setup so gated company creation and reset flows remain covered while production visibility stays unchanged. No current merge-blocking risk remains. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ed94f2cb4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| vi.mock("@/product-scope", async (importOriginal) => ({ | ||
| ...(await importOriginal<typeof import("@/product-scope")>()), | ||
| COMPANY_SWITCHING_HIDDEN: false, | ||
| })); |
There was a problem hiding this comment.
Keep an unmocked test for the reset-button gate
With this file-level mock, every LifecycleControls render runs with company switching enabled. The supposedly unmocked coverage in product-scope-hidden-surfaces.test.ts only asserts the return value of canCreateCompanies; it never mounts this surface. Consequently, if SettingsView later regresses to checking client.carriesPlatformBearer directly, both suites still pass while the destructive Reset control reappears in the shipped single-company build. Keep the flow tests mocked, but add an unmocked render assertion that the reset button is absent.
AGENTS.md reference: AGENTS.md:L93-L97
Useful? React with 👍 / 👎.
Summary
mainhas been red on theConsolejob since b8a3e2e —npm testreports4 failed | 492 passed, ten failures across four files. This is not a flake and not runner contention; it reproduces on a cleannpm ciand locally.b8a3e2e ("gate company creation where every trigger asks, not per button") moved company creation behind one funnel —
canCreateCompaniesbecame!COMPANY_SWITCHING_HIDDEN && client.carriesPlatformBearer— and added the test that pins the hide. What it did not do is update the four older files that assert the presence of the controls it had just hidden:settings-lifecycle-reset-buttoncreate-company-wallet-modecreate-company-dialog-preflight-raceconnection-console-switch-known-statusCOMPANY_SWITCHING_HIDDENshipstrue, socanCreateCompaniesisfalsefor every client. Note this reaches past trigger visibility:create-company-dialog.tsxalso consults it internally to arm and run the provisioning preflight, which is why two of these files fail on field-level assertions rather than on a missing button.The fix
Those four premises are now unreachable rather than wrong — the flow they cover is still in the tree, it is just not offered in the shipped build. So each file declares the configuration it needs (
COMPANY_SWITCHING_HIDDEN: false, other exports untouched viaimportOriginal) instead of asserting against one the product does not ship.This does not weaken the hide. That has its own deliberately-unmocked coverage in
product-scope-hidden-surfaces.test.ts— "company creation is gone from every trigger, not just the switcher", added by b8a3e2e itself — which is where a regression in the gate belongs, and which still passes untouched. I ran it alongside these four to confirm.What the four keep testing is the flow underneath the gate, including the #1894 pre-archive guard, whose whole job is to stop a reset archiving a company before it knows the replacement has a usable admin. Getting to green by deleting them would have thrown away coverage of a data-loss path that is still live code.
I considered the alternative — splitting
canCreateCompaniesinto a product-scope predicate for triggers and a capability-only one for the dialog's internals — and did not do it. With the flagtrueno trigger renders so the dialog never opens, and with itfalsethe two predicates are identical; the conflation is inert in production either way. That would be changing shipped code to suit the tests.No production code changes. The gate behaves as intended.
API Or Behavior Changes
None. Test-only.
Tests
No Rust changed, so the cargo lanes were not run for this diff.
npm run typecheck— cleannpm run typecheck:unit— cleannpm test— 495 files, 4584 tests, 0 failures (was4 failed | 492 passed)Documentation
None needed — no behaviour or API change.
Found while opening #2118, whose
Consolejob is red for this reason and nothing of its own. Once this lands I'll rebase that one; it should go green with no changes.Summary by CodeRabbit