mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-20 04:20:38 +09:00
[codex] Ignore stale stored company selections (#4602)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - The board UI is the operator’s control surface for selecting the active company > - A company id stored in localStorage can become stale across resets, imports, or deleted companies > - Exposing that stale id before companies load can briefly put downstream UI in an invalid company scope > - This pull request defers selected-company exposure until the loaded company list validates the stored id > - The benefit is a cleaner company-selection bootstrap path and fewer transient invalid API requests ## What Changed - Initialized `CompanyProvider` selection as `null` until companies finish loading. - Reused a stored company id only when it exists in the loaded selectable company list. - Cleared storage and selected state when no companies are available. - Added jsdom regression coverage for stale stored ids before and after company loading. ## Verification - `pnpm exec vitest run --project @paperclipai/ui ui/src/context/CompanyContext.test.tsx` ## Risks - Low risk. The change only affects selection bootstrap and keeps valid stored selections intact. - There may be a slightly longer initial `null` selected-company state while the company list is loading. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5 coding agent, tool-enabled terminal/GitHub workflow, reasoning mode active. Context window not exposed in this environment. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
15c0ce3722
commit
d95968a9f8
2 changed files with 124 additions and 12 deletions
71
ui/src/context/CompanyContext.test.tsx
Normal file
71
ui/src/context/CompanyContext.test.tsx
Normal file
|
|
@ -0,0 +1,71 @@
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { resolveBootstrapCompanySelection, shouldClearStoredCompanySelection } from "./CompanyContext";
|
||||||
|
|
||||||
|
const activeCompany = { id: "company-1" };
|
||||||
|
const secondActiveCompany = { id: "company-2" };
|
||||||
|
const archivedCompany = { id: "archived-company" };
|
||||||
|
|
||||||
|
describe("resolveBootstrapCompanySelection", () => {
|
||||||
|
it("does not expose a stale stored company id before companies load", () => {
|
||||||
|
expect(resolveBootstrapCompanySelection({
|
||||||
|
companies: [],
|
||||||
|
sidebarCompanies: [],
|
||||||
|
selectedCompanyId: null,
|
||||||
|
storedCompanyId: "stale-company",
|
||||||
|
})).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("replaces a stale stored company id with the first loaded company", () => {
|
||||||
|
expect(resolveBootstrapCompanySelection({
|
||||||
|
companies: [activeCompany],
|
||||||
|
sidebarCompanies: [activeCompany],
|
||||||
|
selectedCompanyId: null,
|
||||||
|
storedCompanyId: "stale-company",
|
||||||
|
})).toBe("company-1");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps a valid selected company ahead of stored bootstrap state", () => {
|
||||||
|
expect(resolveBootstrapCompanySelection({
|
||||||
|
companies: [activeCompany],
|
||||||
|
sidebarCompanies: [activeCompany],
|
||||||
|
selectedCompanyId: "company-1",
|
||||||
|
storedCompanyId: "stale-company",
|
||||||
|
})).toBe("company-1");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps a valid stored company id instead of falling back to the first company", () => {
|
||||||
|
expect(resolveBootstrapCompanySelection({
|
||||||
|
companies: [activeCompany, secondActiveCompany],
|
||||||
|
sidebarCompanies: [activeCompany, secondActiveCompany],
|
||||||
|
selectedCompanyId: null,
|
||||||
|
storedCompanyId: "company-2",
|
||||||
|
})).toBe("company-2");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("uses selectable sidebar companies before archived companies", () => {
|
||||||
|
expect(resolveBootstrapCompanySelection({
|
||||||
|
companies: [archivedCompany, activeCompany],
|
||||||
|
sidebarCompanies: [activeCompany],
|
||||||
|
selectedCompanyId: null,
|
||||||
|
storedCompanyId: "archived-company",
|
||||||
|
})).toBe("company-1");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("shouldClearStoredCompanySelection", () => {
|
||||||
|
it("does not clear the stored company selection during an unauthorized company list response", () => {
|
||||||
|
expect(shouldClearStoredCompanySelection({
|
||||||
|
companies: [],
|
||||||
|
isLoading: false,
|
||||||
|
unauthorized: true,
|
||||||
|
})).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears the stored company selection when an authorized company list is empty", () => {
|
||||||
|
expect(shouldClearStoredCompanySelection({
|
||||||
|
companies: [],
|
||||||
|
isLoading: false,
|
||||||
|
unauthorized: false,
|
||||||
|
})).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -14,6 +14,7 @@ import { ApiError } from "../api/client";
|
||||||
import { queryKeys } from "../lib/queryKeys";
|
import { queryKeys } from "../lib/queryKeys";
|
||||||
import type { CompanySelectionSource } from "../lib/company-selection";
|
import type { CompanySelectionSource } from "../lib/company-selection";
|
||||||
type CompanySelectionOptions = { source?: CompanySelectionSource };
|
type CompanySelectionOptions = { source?: CompanySelectionSource };
|
||||||
|
type CompanyListResult = { companies: Company[]; unauthorized: boolean };
|
||||||
|
|
||||||
interface CompanyContextValue {
|
interface CompanyContextValue {
|
||||||
companies: Company[];
|
companies: Company[];
|
||||||
|
|
@ -35,25 +36,55 @@ const STORAGE_KEY = "paperclip.selectedCompanyId";
|
||||||
|
|
||||||
const CompanyContext = createContext<CompanyContextValue | null>(null);
|
const CompanyContext = createContext<CompanyContextValue | null>(null);
|
||||||
|
|
||||||
|
export function resolveBootstrapCompanySelection(input: {
|
||||||
|
companies: Array<Pick<Company, "id">>;
|
||||||
|
sidebarCompanies: Array<Pick<Company, "id">>;
|
||||||
|
selectedCompanyId: string | null;
|
||||||
|
storedCompanyId: string | null;
|
||||||
|
}) {
|
||||||
|
if (input.companies.length === 0) return null;
|
||||||
|
|
||||||
|
const selectableCompanies = input.sidebarCompanies.length > 0
|
||||||
|
? input.sidebarCompanies
|
||||||
|
: input.companies;
|
||||||
|
if (input.selectedCompanyId && selectableCompanies.some((company) => company.id === input.selectedCompanyId)) {
|
||||||
|
return input.selectedCompanyId;
|
||||||
|
}
|
||||||
|
if (input.storedCompanyId && selectableCompanies.some((company) => company.id === input.storedCompanyId)) {
|
||||||
|
return input.storedCompanyId;
|
||||||
|
}
|
||||||
|
return selectableCompanies[0]?.id ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function shouldClearStoredCompanySelection(input: {
|
||||||
|
companies: Array<Pick<Company, "id">>;
|
||||||
|
isLoading: boolean;
|
||||||
|
unauthorized: boolean;
|
||||||
|
}) {
|
||||||
|
return !input.isLoading && !input.unauthorized && input.companies.length === 0;
|
||||||
|
}
|
||||||
|
|
||||||
export function CompanyProvider({ children }: { children: ReactNode }) {
|
export function CompanyProvider({ children }: { children: ReactNode }) {
|
||||||
const queryClient = useQueryClient();
|
const queryClient = useQueryClient();
|
||||||
const [selectionSource, setSelectionSource] = useState<CompanySelectionSource>("bootstrap");
|
const [selectionSource, setSelectionSource] = useState<CompanySelectionSource>("bootstrap");
|
||||||
const [selectedCompanyId, setSelectedCompanyIdState] = useState<string | null>(() => localStorage.getItem(STORAGE_KEY));
|
const [selectedCompanyId, setSelectedCompanyIdState] = useState<string | null>(null);
|
||||||
|
|
||||||
const { data: companies = [], isLoading, error } = useQuery({
|
const { data: companiesResult = { companies: [], unauthorized: false }, isLoading, error } = useQuery<CompanyListResult>({
|
||||||
queryKey: queryKeys.companies.all,
|
queryKey: queryKeys.companies.all,
|
||||||
queryFn: async () => {
|
queryFn: async () => {
|
||||||
try {
|
try {
|
||||||
return await companiesApi.list();
|
return { companies: await companiesApi.list(), unauthorized: false };
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (err instanceof ApiError && err.status === 401) {
|
if (err instanceof ApiError && err.status === 401) {
|
||||||
return [];
|
return { companies: [], unauthorized: true };
|
||||||
}
|
}
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
retry: false,
|
retry: false,
|
||||||
});
|
});
|
||||||
|
const companies = companiesResult.companies;
|
||||||
|
const companyListUnauthorized = companiesResult.unauthorized;
|
||||||
const sidebarCompanies = useMemo(
|
const sidebarCompanies = useMemo(
|
||||||
() => companies.filter((company) => company.status !== "archived"),
|
() => companies.filter((company) => company.status !== "archived"),
|
||||||
[companies],
|
[companies],
|
||||||
|
|
@ -61,18 +92,28 @@ export function CompanyProvider({ children }: { children: ReactNode }) {
|
||||||
|
|
||||||
// Auto-select first company when list loads
|
// Auto-select first company when list loads
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (companies.length === 0) return;
|
if (isLoading) return;
|
||||||
|
if (companies.length === 0) {
|
||||||
|
if (shouldClearStoredCompanySelection({ companies, isLoading: false, unauthorized: companyListUnauthorized })) {
|
||||||
|
if (selectedCompanyId !== null) {
|
||||||
|
setSelectedCompanyIdState(null);
|
||||||
|
}
|
||||||
|
localStorage.removeItem(STORAGE_KEY);
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
const selectableCompanies = sidebarCompanies.length > 0 ? sidebarCompanies : companies;
|
const next = resolveBootstrapCompanySelection({
|
||||||
const stored = localStorage.getItem(STORAGE_KEY);
|
companies,
|
||||||
if (stored && selectableCompanies.some((c) => c.id === stored)) return;
|
sidebarCompanies,
|
||||||
if (selectedCompanyId && selectableCompanies.some((c) => c.id === selectedCompanyId)) return;
|
selectedCompanyId,
|
||||||
|
storedCompanyId: localStorage.getItem(STORAGE_KEY),
|
||||||
const next = selectableCompanies[0]!.id;
|
});
|
||||||
|
if (next === null || next === selectedCompanyId) return;
|
||||||
setSelectedCompanyIdState(next);
|
setSelectedCompanyIdState(next);
|
||||||
setSelectionSource("bootstrap");
|
setSelectionSource("bootstrap");
|
||||||
localStorage.setItem(STORAGE_KEY, next);
|
localStorage.setItem(STORAGE_KEY, next);
|
||||||
}, [companies, selectedCompanyId, sidebarCompanies]);
|
}, [companies, companyListUnauthorized, isLoading, selectedCompanyId, sidebarCompanies]);
|
||||||
|
|
||||||
const setSelectedCompanyId = useCallback((companyId: string, options?: CompanySelectionOptions) => {
|
const setSelectedCompanyId = useCallback((companyId: string, options?: CompanySelectionOptions) => {
|
||||||
setSelectedCompanyIdState(companyId);
|
setSelectedCompanyIdState(companyId);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue