diff --git a/doc/plans/2026-10-13-company-ui-test-audit.md b/doc/plans/2026-10-13-company-ui-test-audit.md new file mode 100644 index 00000000000..39ca109f028 --- /dev/null +++ b/doc/plans/2026-10-13-company-ui-test-audit.md @@ -0,0 +1,73 @@ +# Company UI test consolidation proof + +Proof base: PR #16111 exact head `06c1b2035df44fa03c065ddd69e8bf8df8513b86`. Proof used GPT-6.1-Sol. The changes affect only test files. + +| Owner | Original | Candidate | Full original-file restore | Candidate restored | +|---|---:|---:|---:|---:| +| `ui/src/pages/CompanyImport.test.tsx` | 29/29 | 28/28 | 29/29 | 28/28 | +| `ui/src/pages/CompanyEnvironments.test.tsx` | 32/32 | 31/31 | 32/32 | 31/31 | + +All complete-owner runs exited 0, with no failures or skips. Both production mutations below exited 1 with exactly one failure in the keeper. The mutated production files were restored byte for byte before the final candidate runs. The reduction removes 2 test registrations and 15 net test lines. The isolated runs were about 7 seconds and 4 seconds per owner; this is not a CI timing claim. + +## Reproduce + +Use a clean checkout at the proof base with dependencies installed. Run commands from the repository root. + +```sh +pnpm --filter @paperclipai/ui exec vitest run src/pages/CompanyImport.test.tsx --maxWorkers=1 +pnpm --filter @paperclipai/ui exec vitest run src/pages/CompanyEnvironments.test.tsx --maxWorkers=1 +``` + +Expected original counts: 29 and 32 passing tests. Apply the PR's two test-file changes and rerun the same commands: 28 and 31 passing tests. Restore both complete original test files from the proof-base commit and rerun: 29 and 32 passing. Reapply the candidate changes and rerun: 28 and 31 passing. + +## Production mutation controls + +For CompanyImport, apply the candidate test change, then in `ui/src/pages/CompanyImport.tsx` replace the success-path invalidation below: + +```ts +await queryClient.invalidateQueries({ queryKey: queryKeys.companies.all }); +``` + +with: + +```ts +await queryClient.invalidateQueries({ queryKey: ["audit-unrelated-company-query"] }); +``` + +Run only the keeper: + +```sh +pnpm --filter @paperclipai/ui exec vitest run src/pages/CompanyImport.test.tsx --maxWorkers=1 -t 'submits the import as an async job, then activates selected agents and routines' +``` + +Expected result: exit 1 at the `invalidateQueries` query-key assertion. The control observed the unrelated key in the real QueryClient call. Restore `CompanyImport.tsx` from the proof-base commit and verify its SHA-256 is `872d67109ab968fada24f126ad92974c5d060f7552dc4e8299a6185d2b87fe6c`. + +For CompanyEnvironments, apply the candidate test change, then in `ui/src/pages/CompanyEnvironments.tsx` replace the Local option: + +```tsx + +``` + +with: + +```tsx +null +``` + +Run only the keeper: + +```sh +pnpm --filter @paperclipai/ui exec vitest run src/pages/CompanyEnvironments.test.tsx --maxWorkers=1 -t 'opens the add-environment form on a standalone page and closes it on cancel' +``` + +Expected result: exit 1 at the migrated `Local` option assertion. Restore `CompanyEnvironments.tsx` from the proof-base commit and verify its SHA-256 is `871d93af7394090ccdb6ffaaa518c7462055ffb75766e6b9736afdd34e18884b`. + +After both restores, rerun the complete candidate-owner commands. They must pass 28/28 and 31/31. Keep the candidate changes in the two test files; do not leave either production mutation applied. + +## Retained behavior + +CompanyImport retains the full async import and activation assertions. The keeper adds a QueryClient invalidation spy and confirms the companies list is invalidated before activation. Its `finally` path restores the spy. + +CompanyEnvironments moves the implicit Local-option assertion into the existing add-environment scenario after the same initial render and React flush, before opening Add. The same scenario retains the form and cancel assertions. + +The proof also checked that retained test names, fixtures, provider state, actions, hooks, and assertions were unchanged beyond these migrations. There are no skips or production changes in this pull request. diff --git a/ui/src/pages/CompanyEnvironments.test.tsx b/ui/src/pages/CompanyEnvironments.test.tsx index e36be4e0ac9..e0513d3225b 100644 --- a/ui/src/pages/CompanyEnvironments.test.tsx +++ b/ui/src/pages/CompanyEnvironments.test.tsx @@ -705,6 +705,9 @@ describe("CompanyEnvironments — test provider button", () => { }); await flushReact(); + const options = Array.from(container.querySelectorAll("option")); + expect(options.some((option) => option.textContent?.trim() === "Local")).toBe(true); + await act(async () => { click(container.querySelector('[aria-label="Add environment"]')); }); @@ -1569,19 +1572,6 @@ describe("CompanyEnvironments — test provider button", () => { } }); - it("offers the implicit Local option in the default picker by default", async () => { - root = createRoot(container); - const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); - - await act(async () => { - root!.render(renderCompanyEnvironments(queryClient)); - }); - await flushReact(); - - const options = Array.from(container.querySelectorAll("option")); - expect(options.some((option) => option.textContent?.trim() === "Local")).toBe(true); - }); - it("hides the implicit Local option in the default picker under managed-sandbox-only", async () => { mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableEnvironments: true, diff --git a/ui/src/pages/CompanyImport.test.tsx b/ui/src/pages/CompanyImport.test.tsx index 6562f328dde..d8c9286963f 100644 --- a/ui/src/pages/CompanyImport.test.tsx +++ b/ui/src/pages/CompanyImport.test.tsx @@ -390,26 +390,33 @@ describe("CompanyImport", () => { } it("submits the import as an async job, then activates selected agents and routines", async () => { - await renderPageAndImport(); + const invalidateSpy = vi.spyOn(QueryClient.prototype, "invalidateQueries"); + try { + await renderPageAndImport(); - expect(mockCompaniesApi.importBundleAsync).toHaveBeenCalledWith( - expect.objectContaining({ pauseAutomations: true }), - ); - // The page polls the job it was handed and runs the same activation path - // the synchronous response used to drive. - expect(mockCompaniesApi.getImportJob).toHaveBeenCalledWith("job-1"); - expect(container.textContent).toContain("Import complete"); - expect(container.textContent).toContain("Activate imported agents and routines"); - expect(container.textContent).toContain("Coder"); - expect(container.textContent).toContain("Weekly Report"); + expect(mockCompaniesApi.importBundleAsync).toHaveBeenCalledWith( + expect.objectContaining({ pauseAutomations: true }), + ); + // The page polls the job it was handed and runs the same activation path + // the synchronous response used to drive. + expect(mockCompaniesApi.getImportJob).toHaveBeenCalledWith("job-1"); + expect(container.textContent).toContain("Import complete"); + expect(container.textContent).toContain("Activate imported agents and routines"); + expect(container.textContent).toContain("Coder"); + expect(container.textContent).toContain("Weekly Report"); - await clickButton((text) => text.startsWith("Activate selected")); + expect(invalidateSpy).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ["companies"] })); - expect(mockAgentsApi.resume).toHaveBeenCalledWith("agent-1"); - expect(mockRoutinesApi.update).toHaveBeenCalledWith("routine-1", { status: "active" }); - expect(container.textContent).toContain("activated"); - expect(container.textContent).not.toContain("failed:"); - expect(findButton((text) => text === "Go to dashboard")).toBeTruthy(); + await clickButton((text) => text.startsWith("Activate selected")); + + expect(mockAgentsApi.resume).toHaveBeenCalledWith("agent-1"); + expect(mockRoutinesApi.update).toHaveBeenCalledWith("routine-1", { status: "active" }); + expect(container.textContent).toContain("activated"); + expect(container.textContent).not.toContain("failed:"); + expect(findButton((text) => text === "Go to dashboard")).toBeTruthy(); + } finally { + invalidateSpy.mockRestore(); + } }); it("keeps activating remaining items and surfaces per-item failures", async () => { @@ -1012,18 +1019,6 @@ describe("CompanyImport", () => { expect(mockPushToast).toHaveBeenCalledWith(expect.objectContaining({ tone: "error" })); }); - it("refreshes the company list on a full import success", async () => { - const invalidateSpy = vi.spyOn(QueryClient.prototype, "invalidateQueries"); - try { - await renderPageAndImport(); - - expect(container.textContent).toContain("Import complete"); - expect(invalidateSpy).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ["companies"] })); - } finally { - invalidateSpy.mockRestore(); - } - }); - it("resumes watching a stored import job on mount", async () => { // A previous page load persisted a running job; reloading must resume // watching it rather than showing the stale form.