fix(ui): stop the selection ping-pong on archived company URLs (#11300)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The UI keeps a selected company in `CompanyProvider` with two writers: a bootstrap effect that repairs invalid selections, and a Layout route-sync effect that selects the company the URL prefix names > - The route-sync matches the URL against the full company list (archived included), while the bootstrap resolver only accepted companies from the sidebar-filtered non-archived list > - On any archived company's URL the two effects overwrite each other's selection in a synchronous loop until React throws error #185 ("Maximum update depth exceeded") and unmounts the root to a blank page — armed by remembered last-visited paths, back/forward navigation, or bookmarks, on first load and client navigation alike > - This pull request makes an already-selected company only need to exist, keeping the sidebar filter for fresh-boot resolution where no explicit selection exists > - The benefit is that archived company URLs render instead of blanking the entire app ## Linked Issues or Issue Description No existing issue. Description follows the bug template: **What happened?** Opening (or back-navigating to) a URL whose company prefix belongs to an archived company blanked the whole app with `Minified React error #185`. Console in dev mode: "Maximum update depth exceeded. This can happen when a component calls setState inside useEffect…". A workspace whose first/seeded company was archived hit this on every load of its remembered URL. **Expected behavior** An archived company's URL renders its pages (the company still exists and its API routes serve data). The sidebar simply does not feature archived companies, and fresh boots still land on a non-archived company. **Steps to reproduce** 1. Create two companies; archive one (`PATCH /api/companies/:id` with `status: "archived"`). 2. Navigate to `/{archivedPrefix}/dashboard` — direct load or client-side back-navigation. 3. Before this fix: React #185 and an unmounted blank page (reproduced deterministically by the new e2e test). ## What Changed - `ui/src/context/CompanyContext.tsx`: `resolveBootstrapCompanySelection` keeps an explicitly selected company that exists in the full company list; stored-id and default resolution still prefer sidebar (non-archived) companies. - `ui/src/context/CompanyContext.test.tsx`: resolver keeps an archived-but-existing selection; a truly deleted selection is still replaced. - `tests/e2e/archived-company-url.spec.ts`: end-to-end regression driving both field shapes (direct load and back-navigation onto an archived company URL); it failed with the exact #185 console errors before the fix and passes after. ## Verification - `pnpm vitest run src/context …` in `ui/` — 122 tests pass (includes the new resolver cases). - `npx playwright test --config tests/e2e/playwright.config.ts archived-company-url` — fails before the fix (captured "Maximum update depth exceeded" console errors), passes after. - `pnpm typecheck` in `ui/` — clean. ## Risks Low risk. The only behavioral change is that a selection naming an archived-but-existing company survives the bootstrap repair — previously that state was unreachable without crashing. Boots with no valid selection behave exactly as before (non-archived preferred), covered by the existing and new resolver tests. ## Model Used - Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code CLI with extended thinking and tool use (code search, edit, test execution, Playwright-driven crash reproduction). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
parent
276730d63e
commit
61a5b7c6f9
|
|
@ -0,0 +1,67 @@
|
|||
import { expect, test, type Page } from "@playwright/test";
|
||||
|
||||
/**
|
||||
* Regression: landing on an archived company's URL crashed the app with React
|
||||
* error #185. The Layout route-sync matched the URL against the full company
|
||||
* list and selected the archived company; the CompanyProvider bootstrap
|
||||
* resolver only accepted non-archived companies and immediately re-selected
|
||||
* an active one; the two effects ping-ponged the selection until React blew
|
||||
* the nested-update limit and unmounted the tree.
|
||||
*
|
||||
* Field shape: a workspace whose seeded primary company was archived — every
|
||||
* revisit of its remembered `/PREFIX/...` URL (first load and back/forward
|
||||
* navigations alike) produced a blank page.
|
||||
*/
|
||||
|
||||
async function createCompany(page: Page, name: string): Promise<{ id: string; prefix: string }> {
|
||||
const res = await page.request.post("/api/companies", { data: { name } });
|
||||
expect(res.ok(), `create company failed ${res.status()}: ${await res.text()}`).toBe(true);
|
||||
const company = await res.json();
|
||||
return { id: company.id, prefix: company.issuePrefix ?? company.prefix ?? "E2E" };
|
||||
}
|
||||
|
||||
function collectFatalErrors(page: Page): string[] {
|
||||
const fatal: string[] = [];
|
||||
page.on("pageerror", (err) => {
|
||||
fatal.push(`PAGEERROR: ${err.message}`);
|
||||
});
|
||||
page.on("console", async (msg) => {
|
||||
if (msg.type() !== "error") return;
|
||||
const text = msg.text();
|
||||
if (!/App shell crashed|Page render failed|Minified React error #185|Maximum update depth/.test(text)) {
|
||||
return;
|
||||
}
|
||||
fatal.push(`CONSOLE: ${text.slice(0, 300)}`);
|
||||
});
|
||||
return fatal;
|
||||
}
|
||||
|
||||
test("landing on an archived company's URL does not crash the app", async ({ page }) => {
|
||||
const fatal = collectFatalErrors(page);
|
||||
|
||||
const active = await createCompany(page, "Archived Loop Active");
|
||||
const archived = await createCompany(page, "Archived Loop Archived");
|
||||
const archiveRes = await page.request.patch(`/api/companies/${archived.id}`, {
|
||||
data: { status: "archived" },
|
||||
});
|
||||
expect(archiveRes.ok(), `archive failed ${archiveRes.status()}: ${await archiveRes.text()}`).toBe(true);
|
||||
|
||||
// Field repro #1: a fresh load straight onto the archived company's
|
||||
// remembered URL (the first-open blank screen).
|
||||
await page.goto(`/${archived.prefix}/dashboard`);
|
||||
await page.waitForTimeout(2_500);
|
||||
expect(fatal, `crash on direct load of archived company URL:\n${fatal.join("\n")}`).toEqual([]);
|
||||
|
||||
// Field repro #2: visit the active company, then return to the archived
|
||||
// company's URL with client-side history navigation (the "switched back"
|
||||
// crash).
|
||||
await page.goto(`/${active.prefix}/dashboard`);
|
||||
await page.waitForLoadState("networkidle");
|
||||
await page.goBack();
|
||||
await page.waitForTimeout(2_500);
|
||||
expect(fatal, `crash on back-navigation to archived company URL:\n${fatal.join("\n")}`).toEqual([]);
|
||||
|
||||
// The app must have landed somewhere real, not a blank unmounted root.
|
||||
const rootContent = await page.locator("#root").innerText().catch(() => "");
|
||||
expect(rootContent.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
|
@ -107,6 +107,28 @@ describe("resolveBootstrapCompanySelection", () => {
|
|||
storedCompanyId: "archived-company",
|
||||
})).toBe("company-1");
|
||||
});
|
||||
|
||||
it("keeps an explicitly selected archived company that still exists", () => {
|
||||
// The Layout route-sync selects the company the URL names, archived
|
||||
// included. Vetoing that selection here re-selected an active company,
|
||||
// the route-sync selected the archived one again, and the two effects
|
||||
// ping-ponged until React threw #185 and unmounted the app.
|
||||
expect(resolveBootstrapCompanySelection({
|
||||
companies: [archivedCompany, activeCompany],
|
||||
sidebarCompanies: [activeCompany],
|
||||
selectedCompanyId: "archived-company",
|
||||
storedCompanyId: null,
|
||||
})).toBe("archived-company");
|
||||
});
|
||||
|
||||
it("still replaces a selected company that no longer exists at all", () => {
|
||||
expect(resolveBootstrapCompanySelection({
|
||||
companies: [activeCompany],
|
||||
sidebarCompanies: [activeCompany],
|
||||
selectedCompanyId: "deleted-company",
|
||||
storedCompanyId: null,
|
||||
})).toBe("company-1");
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldClearStoredCompanySelection", () => {
|
||||
|
|
|
|||
|
|
@ -46,7 +46,15 @@ export function resolveBootstrapCompanySelection(input: {
|
|||
const selectableCompanies = input.sidebarCompanies.length > 0
|
||||
? input.sidebarCompanies
|
||||
: input.companies;
|
||||
if (input.selectedCompanyId && selectableCompanies.some((company) => company.id === input.selectedCompanyId)) {
|
||||
// An already-selected company only needs to EXIST — not to be featured in
|
||||
// the sidebar. The Layout route-sync selects whatever company the URL names
|
||||
// (archived included, since archived pages are still routable); if this
|
||||
// resolver vetoed that selection against the sidebar-filtered list, the two
|
||||
// effects would re-select against each other forever and blow React's
|
||||
// nested-update limit (the archived-company blank-screen crash). The
|
||||
// sidebar filter keeps shaping fresh boots below, where no explicit
|
||||
// selection exists yet.
|
||||
if (input.selectedCompanyId && input.companies.some((company) => company.id === input.selectedCompanyId)) {
|
||||
return input.selectedCompanyId;
|
||||
}
|
||||
if (input.storedCompanyId && selectableCompanies.some((company) => company.id === input.storedCompanyId)) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue