Conversation
Adds CompanyCard component with install button, loading state, and tag rendering. Wires @testing-library/react + jest-dom into vitest via a shared setup file so the components can use DOM-based tests while non-UI tests continue to run in the default node environment (CompanyCard.test.tsx opts into jsdom via a pragma).
Greptile SummaryAdds a template gallery — a curated registry of pre-built agent companies that users can install in one click. The implementation covers shared Zod types, a file-backed registry service with in-memory caching, three new API endpoints (
Confidence Score: 4/5One P1 bug (concurrent installs can create duplicate companies) should be fixed before merging; the two P2 findings are non-blocking. The concurrent-install defect in TemplatesPage is a present correctness issue — other cards stay clickable during an install and TanStack Query v5 calls onSuccess for each completing mutation, making duplicate company creation a realistic outcome. The remaining two findings (CWD-based path and error detail leakage) are P2 quality/hardening items that don't block the primary flow. ui/src/pages/TemplatesPage.tsx (P1 concurrent-install bug), server/src/app.ts (fragile CWD path), server/src/routes/templates.ts (error detail field) Important Files Changed
Prompt To Fix All With AIThis is a comment left during a code review.
Path: ui/src/pages/TemplatesPage.tsx
Line: 42-43
Comment:
**Concurrent installs not guarded — duplicate company creation possible**
`installing` is `false` for every card whose slug doesn't match the active mutation, so their buttons stay enabled while an install is in flight. In TanStack Query v5, calling `mutate()` while a prior mutation is pending fires both requests and calls `onSuccess` for each one that completes — which would create two companies and trigger two `navigate()` calls in quick succession.
Fix: gate the disabled state on `install.isPending` globally, while keeping the per-slug check only for the label:
```suggestion
onInstall={(slug) => install.mutate(slug)}
installing={install.isPending && install.variables === company.slug}
disabled={install.isPending}
```
Then update `CompanyCard` to accept and forward a separate `disabled` prop to `<Button>`, or simply change `installing={...}` to `installing={install.isPending && install.variables === company.slug}` and `disabled` on the card to `install.isPending` so all buttons are blocked until the current install resolves.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: server/src/routes/templates.ts
Line: 41
Comment:
**Error detail leaks absolute server file path**
When the registry file is missing, `createTemplateRegistry.get()` throws `"Template registry not found at /absolute/path/docs/templates/registry.json"`. That full path is surfaced verbatim to authenticated clients via the `detail` field here (and again on the `/refresh` 503 at line 93). Board-level auth limits exposure, but it's still an unnecessary information disclosure of the server's filesystem layout.
```suggestion
res.status(503).json({ error: "registry unavailable" });
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: server/src/app.ts
Line: 192-195
Comment:
**`process.cwd()` makes registry path fragile**
`path.resolve(process.cwd(), "docs/templates/registry.json")` only resolves correctly when the server is started from the repository root. Running from `server/` (e.g. `node dist/server.js` in the package directory) silently produces a wrong path and the registry will 503 on every request.
Using a path anchored to the source file is more robust:
```ts
import { fileURLToPath } from "node:url";
import { dirname } from "node:path";
const __dirname = dirname(fileURLToPath(import.meta.url));
const templateRegistry = createTemplateRegistry(
path.resolve(__dirname, "../../../docs/templates/registry.json"),
);
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "test(e2e): smoke test for template galle..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| onInstall={(slug) => install.mutate(slug)} | ||
| installing={install.isPending && install.variables === company.slug} |
There was a problem hiding this comment.
installing is false for every card whose slug doesn't match the active mutation, so their buttons stay enabled while an install is in flight. In TanStack Query v5, calling mutate() while a prior mutation is pending fires both requests and calls onSuccess for each one that completes — which would create two companies and trigger two navigate() calls in quick succession.
Fix: gate the disabled state on install.isPending globally, while keeping the per-slug check only for the label:
| onInstall={(slug) => install.mutate(slug)} | |
| installing={install.isPending && install.variables === company.slug} | |
| onInstall={(slug) => install.mutate(slug)} | |
| installing={install.isPending && install.variables === company.slug} | |
| disabled={install.isPending} |
Then update CompanyCard to accept and forward a separate disabled prop to <Button>, or simply change installing={...} to installing={install.isPending && install.variables === company.slug} and disabled on the card to install.isPending so all buttons are blocked until the current install resolves.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/pages/TemplatesPage.tsx
Line: 42-43
Comment:
**Concurrent installs not guarded — duplicate company creation possible**
`installing` is `false` for every card whose slug doesn't match the active mutation, so their buttons stay enabled while an install is in flight. In TanStack Query v5, calling `mutate()` while a prior mutation is pending fires both requests and calls `onSuccess` for each one that completes — which would create two companies and trigger two `navigate()` calls in quick succession.
Fix: gate the disabled state on `install.isPending` globally, while keeping the per-slug check only for the label:
```suggestion
onInstall={(slug) => install.mutate(slug)}
installing={install.isPending && install.variables === company.slug}
disabled={install.isPending}
```
Then update `CompanyCard` to accept and forward a separate `disabled` prop to `<Button>`, or simply change `installing={...}` to `installing={install.isPending && install.variables === company.slug}` and `disabled` on the card to `install.isPending` so all buttons are blocked until the current install resolves.
How can I resolve this? If you propose a fix, please make it concise.| const registry = await deps.registry.get(); | ||
| res.json({ companies: registry.companies }); | ||
| } catch (err) { | ||
| res.status(503).json({ error: "registry unavailable", detail: (err as Error).message }); |
There was a problem hiding this comment.
When the registry file is missing, createTemplateRegistry.get() throws "Template registry not found at /absolute/path/docs/templates/registry.json". That full path is surfaced verbatim to authenticated clients via the detail field here (and again on the /refresh 503 at line 93). Board-level auth limits exposure, but it's still an unnecessary information disclosure of the server's filesystem layout.
| res.status(503).json({ error: "registry unavailable", detail: (err as Error).message }); | |
| res.status(503).json({ error: "registry unavailable" }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/routes/templates.ts
Line: 41
Comment:
**Error detail leaks absolute server file path**
When the registry file is missing, `createTemplateRegistry.get()` throws `"Template registry not found at /absolute/path/docs/templates/registry.json"`. That full path is surfaced verbatim to authenticated clients via the `detail` field here (and again on the `/refresh` 503 at line 93). Board-level auth limits exposure, but it's still an unnecessary information disclosure of the server's filesystem layout.
```suggestion
res.status(503).json({ error: "registry unavailable" });
```
How can I resolve this? If you propose a fix, please make it concise.| companyDeletionEnabled: opts.companyDeletionEnabled, | ||
| }), | ||
| ); | ||
| api.use("/companies", companyRoutes(db, opts.storageService)); | ||
| const portabilityService = companyPortabilityService(db, opts.storageService); |
There was a problem hiding this comment.
process.cwd() makes registry path fragile
path.resolve(process.cwd(), "docs/templates/registry.json") only resolves correctly when the server is started from the repository root. Running from server/ (e.g. node dist/server.js in the package directory) silently produces a wrong path and the registry will 503 on every request.
Using a path anchored to the source file is more robust:
import { fileURLToPath } from "node:url";
import { dirname } from "node:path";
const __dirname = dirname(fileURLToPath(import.meta.url));
const templateRegistry = createTemplateRegistry(
path.resolve(__dirname, "../../../docs/templates/registry.json"),
);Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/app.ts
Line: 192-195
Comment:
**`process.cwd()` makes registry path fragile**
`path.resolve(process.cwd(), "docs/templates/registry.json")` only resolves correctly when the server is started from the repository root. Running from `server/` (e.g. `node dist/server.js` in the package directory) silently produces a wrong path and the registry will 503 on every request.
Using a path anchored to the source file is more robust:
```ts
import { fileURLToPath } from "node:url";
import { dirname } from "node:path";
const __dirname = dirname(fileURLToPath(import.meta.url));
const templateRegistry = createTemplateRegistry(
path.resolve(__dirname, "../../../docs/templates/registry.json"),
);
```
How can I resolve this? If you propose a fix, please make it concise.
|
🌱 gardener · 🆕
Context fitContext match
Tree nodes referenced
RecommendationWhy: The intent is squarely aligned with the tree: Company Portability explicitly calls out "sharing templates" and import/export as a V1 goal, and the install path reuses the portability bundle contract rather than inventing a parallel one. Where the tree is silent:
Suggested path forward:
Severity medium because this adds a visible product surface (install flow, new route, new admin endpoint) with a new trust/source-of-truth boundary that other contributors will want documented before extending. Reviewed commit: 🌱 Posted by repo-gardener — an open-source context-aware review bot built on First-Tree. Reviews this repo against serenakeyitan/paperclip-tree, a user-maintained context tree. Not affiliated with this project's maintainers. |
3 participants
Thinking Path
What Changed
packages/shared)docs/templates/registry.jsonwith four curated companiesGET /api/templates/companies— list templatesPOST /api/templates/companies/install— install selected templatePOST /api/templates/refresh— admin-only cache invalidateapp.tsuipackage):CompanyCardcomponent (with jest-dom / vitest setup for the UI package)TemplatesPagewith React Query + per-card install mutation → redirects to new company's dashboard/templatesregistered and nav link added to Sidebar (Company section)Verification
pnpm test:run— full UI test suite: 85/85 files, 446 tests passing on this branchpnpm typecheck— clean (requiresNODE_OPTIONS=--max-old-space-size=8192on the templates-gallery UI package due to project size, unchanged from master)pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/templates-gallery.spec.ts --list— spec registers correctlymaestro.trifti.ch) planned post-merge per spec §Task 13Risks
POST /api/templates/refreshis admin-gated. Non-admin boards cannot invalidate the registry cache.Model Used
Spec
Implements
/root/docs/superpowers/plans/2026-04-16-paperclip-templates-gallery.mdand the companion design spec2026-04-16-paperclip-templates-gallery-design.md.