Rebuild FostGen on Next 16 with a layered architecture - #2
Conversation
Upgrade every dependency to the current stable release, restructure the app into domain/hook/component layers, move GitHub access to the server, and replace the placeholder test suite with real coverage. Upgrades - Next 15.3.8 -> 16.2.12, React 19.1 -> 19.2, Tailwind 4.1 -> 4.3, lucide-react 0.487 -> 1.28, Jest 29 -> 30 (via next/jest), TypeScript 5.9. - ESLint moved to a native flat config using eslint-config-next 16 (next lint no longer exists in Next 16). - tsconfig hardened: noUncheckedIndexedAccess, noImplicitOverride, noFallthroughCasesInSwitch, ES2022 target. - Pinned to TypeScript 5.9: Next 16 does not yet detect TypeScript 7. Removals - winston + @types/express: src/utils/logger.ts registered Express middleware in a Next.js app, so it could never run. - The turbopack @svgr/webpack rule referenced a package that was never installed and would fail on the first SVG import. - turbo: no workspaces, and no script ever invoked it. - src/components/__tests__/InputForm.test.tsx asserted an API the component never had (it passed only onSubmit and expected internal validation). Architecture - src/lib/github: URL parser (shorthand, SSH, raw, api, deep links), typed API client with optional GITHUB_TOKEN, rate-limit surfacing and ref widening for branch names containing slashes. - src/lib/tree: build -> filter -> sort -> render as pure, tested stages, composed by deriveStructure. - GET /api/structure resolves repositories server-side with a zod-validated query, cache headers and a structured error taxonomy (AppError). Correctness fixes - Directories are identified by GitHub's type field instead of being inferred from "has children", so empty directories stay directories. - Submodules are a distinct node type rather than being shown as files. - A truncated tree now degrades with a warning instead of throwing. - Notification timeouts are cleared, so no state updates after unmount. Features - Five output formats (ASCII, Markdown, JSON, YAML, flat paths), optional code fence, sizes with directory aggregation, depth limit, folders-only mode, three sort orders and gitignore-style ignore patterns with negation. - Repository summary, live stats, recent repositories, light/dark/system theme with no first-paint flash, and keyboard shortcuts. - Option changes re-render from data already in memory; no refetch. Accessibility - Labelled controls, role="switch" toggles, live-region toasts that can be dismissed, and validation errors wired to their field. Testing and CI - 169 tests across 13 suites, ~80% coverage with a 70% floor. - GitHub Actions workflow running lint, typecheck, tests and a build. Co-authored-by: Irma Raihan Setiawan <officialelsa21@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughFostGen v3 adds GitHub repository parsing, structure retrieval, tree transformation, multiple output formats, a client generator interface, theme and notification systems, application metadata, and CI verification. ChangesFostGen application rebuild
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant StructureGenerator
participant StructureAPI
participant GitHubAPI
participant TreePipeline
participant OutputPanel
User->>StructureGenerator: Enter repository URL and options
StructureGenerator->>StructureAPI: Request repository structure
StructureAPI->>GitHubAPI: Fetch metadata and tree
GitHubAPI-->>StructureAPI: Return repository and tree data
StructureAPI->>TreePipeline: Build and derive structure
TreePipeline-->>StructureAPI: Return rendered output and statistics
StructureAPI-->>StructureGenerator: Return structure payload
StructureGenerator->>OutputPanel: Render output and statistics
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 286fc4ceb7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'X-GitHub-Api-Version': '2022-11-28', | ||
| 'User-Agent': USER_AGENT, | ||
| }; | ||
| if (token) headers.Authorization = `Bearer ${token}`; |
There was a problem hiding this comment.
When GITHUB_TOKEN is configured with private-repository access, this unconditional Authorization header makes every anonymous /api/structure request run with that token; a visitor who guesses owner/private-repo can receive the private tree, and the route then marks successful responses public-cacheable. Please either require caller authorization for private repositories or restrict/reject token-backed private repo responses.
Useful? React with 👍 / 👎.
| }; | ||
| } catch (cause) { | ||
| const error = toAppError(cause); | ||
| if (error.code !== 'NOT_FOUND') throw error; |
There was a problem hiding this comment.
When a repository has both a ref named release and a ref named release/2026-07, a pasted GitHub URL like /tree/release/2026-07/src is parsed as ref=release and path=2026-07/src; fetchTree('release') succeeds, scopeToPath throws PATH_NOT_FOUND, and this check rethrows instead of trying the next widened candidate release/2026-07. Treat PATH_NOT_FOUND during widening as a candidate miss until the wider ref candidates are exhausted, otherwise valid GitHub deep links fail whenever a prefix ref exists.
Useful? React with 👍 / 👎.
| patterns: resolveIgnorePatterns(options), | ||
| }); | ||
|
|
||
| const sized = options.showSizes ? withAggregatedSizes(filtered) : filtered; |
There was a problem hiding this comment.
When the user selects Largest first but leaves Show sizes off (the default), directories still have undefined size here, so sortTree(..., 'size-desc') treats them as zero and can place large folders after small root files. Aggregate directory sizes whenever sorting by size, while still passing showSizes separately to rendering so the numbers can remain hidden.
Useful? React with 👍 / 👎.
PR Summary by QodoModernize FostGen to Next.js 16 with layered server GitHub pipeline
AI Description
Diagram
High-Level Assessment
Files changed (70)
|
Code Review by Qodo
1. Token allows private leakage
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| function readRateLimit(headers: Headers, authenticated: boolean): RateLimitInfo | null { | ||
| const limit = Number(headers.get('x-ratelimit-limit')); | ||
| const remaining = Number(headers.get('x-ratelimit-remaining')); | ||
| const reset = Number(headers.get('x-ratelimit-reset')); | ||
|
|
||
| if (!Number.isFinite(limit) || !Number.isFinite(remaining) || !Number.isFinite(reset)) { | ||
| return null; |
There was a problem hiding this comment.
1. Rate limit parsed as zero 🐞 Bug ≡ Correctness
readRateLimit() uses Number(headers.get(...)), so missing x-ratelimit-* headers become 0 (Number(null) === 0) and produce a fake RateLimitInfo instead of null. This can misclassify a 403 without rate-limit headers as RATE_LIMITED and/or show incorrect 0/0/0 rateLimit metadata.
Agent Prompt
### Issue description
`readRateLimit()` converts missing rate-limit headers to `0` and then treats them as valid because `Number.isFinite(0)` is true. This can cause incorrect `RateLimitInfo` and can trigger the `RATE_LIMITED` path for headerless 403 responses.
### Issue Context
- `Headers.get()` returns `null` when the header is absent.
- `Number(null)` evaluates to `0`, so the current finiteness checks don’t detect missing headers.
### Fix Focus Areas
- src/lib/github/client.ts[29-38]
- src/lib/github/client.ts[100-105]
### Suggested fix
- In `readRateLimit`, read raw header strings first and return `null` if any are `null`/empty.
- Parse using `Number.parseInt(value, 10)` and return `null` if any parsed number is `NaN`.
- Tighten the rate-limit detection to only treat 403 as rate-limited when `rateLimit !== null && rateLimit.remaining === 0`.
- Add/extend a Jest test for a 403 response with *no* x-ratelimit headers to ensure it maps to `UNAUTHORIZED` (not `RATE_LIMITED`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| return NextResponse.json(payload, { | ||
| headers: { | ||
| 'Cache-Control': `public, s-maxage=${GITHUB_CACHE_TTL_SECONDS}, stale-while-revalidate=${GITHUB_CACHE_TTL_SECONDS * 2}`, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
2. Token allows private leakage 🐞 Bug ⛨ Security
GET /api/structure is unauthenticated but uses a server-side GITHUB_TOKEN for GitHub requests, so any visitor can fetch structures for private repos that token can read. Successful responses are also marked Cache-Control: public, which can place private repo structure into shared caches.
Agent Prompt
### Issue description
The API route is publicly callable, yet GitHub requests are made with a privileged server-side `GITHUB_TOKEN` when configured. This allows anonymous callers to enumerate private repositories accessible by that token, and the response is explicitly cacheable as `public`, which can further leak private structures via shared caching.
### Issue Context
- The README explicitly states `GITHUB_TOKEN` “unlocks private repositories the token can read”.
- The route sets `Cache-Control: public, s-maxage=...` for all successful responses.
### Fix Focus Areas
- src/app/api/structure/route.ts[30-69]
- src/lib/github/client.ts[61-78]
- README.md[90-108]
### Suggested fix (pick a deliberate product/security stance)
1) **If private repos should NOT be accessible to anonymous users (recommended for hosted/public deployments):**
- Require authentication/authorization on `/api/structure` before using `GITHUB_TOKEN` (e.g., session, API key, or other access control).
- Alternatively, refuse private-repo access entirely: fetch repo metadata, detect `private: true`, and return `UNAUTHORIZED`/`NOT_FOUND` unless the caller is authenticated.
2) **Regardless of access control choice:**
- Do **not** mark responses as `Cache-Control: public` when the response could be derived from privileged access.
- At minimum, when `process.env.GITHUB_TOKEN` is set, return `Cache-Control: private, no-store` (or equivalent) for success responses.
- (Stronger) Add `private` (or `visibility`) to `RepositoryMeta` by mapping the GitHub `/repos` payload, and base cache headers on `repository.private`.
3) **Add tests:**
- Verify cache headers differ between public vs private repos (or when token is configured).
- Verify unauthorized callers cannot access private repos when the token can.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export function isApiErrorPayload(value: unknown): value is ApiErrorPayload { | ||
| if (typeof value !== 'object' || value === null) return false; | ||
| const candidate = (value as { error?: unknown }).error; | ||
| return ( | ||
| typeof candidate === 'object' && | ||
| candidate !== null && | ||
| typeof (candidate as { message?: unknown }).message === 'string' | ||
| ); |
There was a problem hiding this comment.
3. Api error validator incomplete 🐞 Bug ☼ Reliability
isApiErrorPayload() only checks error.message, but the client then trusts error.code and stores it in state. Malformed/non-conforming responses can therefore yield undefined or unknown error codes and degrade code-based error handling.
Agent Prompt
### Issue description
`isApiErrorPayload()` is used as a type guard but it doesn’t validate that `error.code` exists or is a member of the `ErrorCode` taxonomy. Downstream, `useRepoStructure` immediately trusts `body.error.code`, which can lead to invalid runtime state when the server/proxy returns a non-conforming payload.
### Issue Context
This is an HTTP boundary; even if the server normally returns the correct shape, clients should defensively validate all fields they rely on.
### Fix Focus Areas
- src/lib/api/schema.ts[44-51]
- src/hooks/useRepoStructure.ts[60-68]
- src/lib/errors.ts[7-23]
### Suggested fix
- Strengthen `isApiErrorPayload` to also validate:
- `typeof error.code === 'string'` and it is included in `ERROR_CODES`.
- (Optional) validate `hint` is string when present and `retryAfter` is number when present.
- Alternatively, define a zod schema for the error payload and use `safeParse`.
- In `readError`, if validation fails, fall back to the generic `UPSTREAM` error instead of trusting `code`.
- Add a unit test where the server returns `{ error: { message: 'x' } }` (missing `code`) to ensure the client falls back safely.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (13)
src/lib/github/__tests__/client.test.ts (1)
49-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnrestored
globalThis.fetchin both new test suites. Both suites assign a Jest mock to the globalfetchinbeforeEachand never restore the original, so the mock leaks to any later module in the same Jest worker.
src/lib/github/__tests__/client.test.ts#L49-L53: captureglobalThis.fetchin module scope and restore it in anafterAllhook.src/app/api/structure/__tests__/route.test.ts#L41-L44: apply the same capture-and-restore pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/github/__tests__/client.test.ts` around lines 49 - 53, Restore the original global fetch after each test suite to prevent mock leakage: in src/lib/github/__tests__/client.test.ts lines 49-53, capture globalThis.fetch at module scope and restore it in afterAll; apply the same capture-and-restore pattern in src/app/api/structure/__tests__/route.test.ts lines 41-44.src/app/api/structure/route.ts (1)
30-52: 📐 Maintainability & Code Quality | 🔵 TrivialConsider a per-caller quota for this endpoint.
The handler spends the server's GitHub quota on every uncached request, and
resolveStructurecan issue up to five upstream calls for one request through ref widening. One caller can exhaust the shared token quota for all visitors. Add rate limiting at the edge or in middleware, keyed by IP.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/structure/route.ts` around lines 30 - 52, Add IP-keyed rate limiting before the GET handler performs validation or calls resolveStructure, preferably through the endpoint’s edge layer or middleware. Ensure rejected callers receive the project’s standard rate-limit response, while allowed requests retain the existing structureQuerySchema and resolveStructure flow.src/lib/github/parse-repo-url.ts (1)
29-31: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider trimming instead of stripping all whitespace.
Line 30 removes every whitespace character, including internal ones. The input
owner/re pobecomesowner/repo, so the parser accepts a repository the user did not request. Strip only surrounding whitespace and reject the rest.♻️ Proposed change
- let value = raw.trim().replace(/\s+/g, ''); + let value = raw.trim(); if (!value) throw invalid('Paste a repository URL such as https://github.com/owner/repo.'); + if (/\s/.test(value)) throw invalid('Remove the spaces from the repository URL.');Note: the OpenGrep
command-injection.exec-jshint on line 35 is a false positive.execthere isRegExp.prototype.exec.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/github/parse-repo-url.ts` around lines 29 - 31, Update normaliseInput to trim only leading and trailing whitespace, removing the replace(/\s+/g, '') behavior. Validate and reject any remaining internal whitespace instead of silently altering repository names, while preserving the existing empty-input error.Source: Linters/SAST tools
src/lib/github/client.ts (1)
72-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a default timeout to GitHub requests.
This fetch has no deadline when the caller does not provide
signal, so a stalled GitHub connection can keep the route handler open until the platform kills it. Add a fallbackAbortSignal.timeout()and combine it with the caller signal usingAbortSignal.any().The proposed
ABORTEDcode maps to status 499 insrc/lib/errors.ts; add a separateTimeoutErrormapping if a distinct upstream time-out response is needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/github/client.ts` around lines 72 - 78, Update the fetch request in the GitHub client’s try block to always enforce a default timeout by creating a fallback AbortSignal.timeout() and combining it with options.signal via AbortSignal.any(). Preserve caller cancellation while ensuring requests without a signal still expire, and add a distinct TimeoutError mapping in the existing error handling if timeout responses must differ from the ABORTED/499 mapping.src/lib/config.ts (1)
11-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNormalize the explicit site URL scheme.
getSiteUrlreturnsNEXT_PUBLIC_SITE_URLverbatim. If the variable holds a host without a scheme, such asfostgen.app,sitemap.tsandrobots.tsemit non-absolute URLs. Add a scheme when it is missing.♻️ Proposed normalization
export function getSiteUrl(): string { const explicit = process.env.NEXT_PUBLIC_SITE_URL?.trim(); - if (explicit) return explicit.replace(/\/$/, ''); + if (explicit) { + const withScheme = /^https?:\/\//i.test(explicit) ? explicit : `https://${explicit}`; + return withScheme.replace(/\/$/, ''); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/config.ts` around lines 11 - 20, Update getSiteUrl to normalize NEXT_PUBLIC_SITE_URL before returning it: preserve http:// or https:// values, but prepend https:// when the trimmed explicit value lacks a scheme. Keep the existing trailing-slash removal and Vercel/localhost fallback behavior unchanged.src/lib/format/units.ts (1)
23-29: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse the
Intlformatters at module scope.Both formatters use fixed locale/options and are called on UI render paths (
formatCountinStatsRow;formatCountandformatRelativeTimeinRepositorySummary). Hoisting oneIntl.NumberFormatand oneIntl.RelativeTimeFormatinstance avoids repeated formatter construction.
[provide_code_example]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/format/units.ts` around lines 23 - 29, Hoist the fixed-locale formatter instances to module scope in units.ts: create one Intl.NumberFormat for formatCount and one Intl.RelativeTimeFormat for formatRelativeTime, then reuse them inside those functions instead of constructing formatters on each call. Preserve the existing locale, options, validation, and output behavior.src/components/layout/Header.tsx (1)
23-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: hide the decorative shortcut hint from assistive technology.
StructureGeneratoraccepts both Meta+K and Ctrl+K, but this hint always shows⌘K. Screen readers also announce the bare glyph without context. Mark the element decorative, and consider a platform-aware label.♿ Proposed change
- <kbd className="hidden rounded-md border border-line bg-elevated px-1.5 py-0.5 font-mono text-[0.6875rem] text-ink-subtle sm:inline-block"> + <kbd + aria-hidden + className="hidden rounded-md border border-line bg-elevated px-1.5 py-0.5 font-mono text-[0.6875rem] text-ink-subtle sm:inline-block" + > ⌘K </kbd>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Header.tsx` around lines 23 - 25, Update the shortcut hint kbd element in Header to be hidden from assistive technology because it is decorative; preserve its visual ⌘K display, and do not add a misleading accessibility label unless implementing platform-aware shortcut text.src/components/ui/Toaster.tsx (1)
75-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: clear timers for toasts dropped by the visibility cap.
slice(-MAX_VISIBLE)removes the oldest toasts from state, but their entries stay intimers.currentuntil each timeout fires. The laterdismisscall is then a no-op. The map is bounded and self-heals, so this is hygiene only.♻️ Proposed change
setToasts((current) => { const next: Toast = { id, tone, title: normalised.title, ...(normalised.description ? { description: normalised.description } : {}), }; - return [...current, next].slice(-MAX_VISIBLE); + const merged = [...current, next]; + const visible = merged.slice(-MAX_VISIBLE); + for (const dropped of merged.slice(0, merged.length - visible.length)) { + const timer = timers.current.get(dropped.id); + if (timer) { + clearTimeout(timer); + timers.current.delete(dropped.id); + } + } + return visible; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/Toaster.tsx` around lines 75 - 93, Optionally update the toast state update around setToasts and timers.current so IDs removed by the MAX_VISIBLE slice have their pending timer entries cleared. Preserve the existing visibility cap and dismissal behavior, and only remove timer entries for toasts dropped from state.src/components/generator/OutputPanel.tsx (1)
35-47: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCount lines without allocating an array.
output.split('\n')builds a full array for large trees only to readlength. A trailing newline also adds one phantom line to the displayed count. Count separators directly and ignore a single trailing newline.♻️ Proposed line-count refactor
- const lineCount = useMemo(() => (output ? output.split('\n').length : 0), [output]); + const lineCount = useMemo(() => { + if (!output) return 0; + const end = output.endsWith('\n') ? output.length - 1 : output.length; + let count = 1; + for (let index = 0; index < end; index += 1) { + if (output.charCodeAt(index) === 10) count += 1; + } + return count; + }, [output]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/generator/OutputPanel.tsx` around lines 35 - 47, Update the lineCount calculation in OutputPanel to count newline separators directly instead of calling output.split('\n'), and exclude one trailing newline from the displayed count. Preserve the zero count for empty output and the existing line-count display behavior.src/components/generator/StructureGenerator.tsx (1)
70-73: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDefer re-derivation so option changes stay responsive.
deriveStructureruns synchronously in render. The depth range input inOptionsPanelfiresonChangefor every step of a drag, so a large tree is rebuilt, filtered, sorted, and rendered on each intermediate value. Wrap the inputs inuseDeferredValueto keep the controls interactive.♻️ Proposed deferred derivation
-import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useDeferredValue, useEffect, useMemo, useRef, useState } from 'react';- const derived = useMemo( - () => (payload ? deriveStructure(payload.nodes, options.value, rootName) : null), - [payload, options.value, rootName], - ); + const deferredOptions = useDeferredValue(options.value); + const derived = useMemo( + () => (payload ? deriveStructure(payload.nodes, deferredOptions, rootName) : null), + [payload, deferredOptions, rootName], + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/generator/StructureGenerator.tsx` around lines 70 - 73, Update the derivation flow around deriveStructure in StructureGenerator so the options input is passed through useDeferredValue before being used for memoized computation. Use the deferred options for deriveStructure and its dependency tracking, while keeping the current payload and rootName behavior unchanged.src/components/theme/ThemeProvider.tsx (1)
86-99: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTear down the shared listeners only when the last subscriber leaves.
Each cleanup removes the media-query and
storagehandlers unconditionally. With two subscribers, the first unsubscribe stops updates for the one that remains. Also drop the cachedsnapshotwhen the Set empties, so a value written while unsubscribed is not served stale.subscribeToKeyinsrc/hooks/useLocalStorage.tsalready applies this guard.♻️ Proposed subscription lifecycle
return () => { listeners.delete(listener); + if (listeners.size > 0) return; query?.removeEventListener('change', invalidate); window.removeEventListener('storage', invalidate); + snapshot = null; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/theme/ThemeProvider.tsx` around lines 86 - 99, Update subscribe in ThemeProvider so its cleanup removes the listener from listeners first, then tears down the shared matchMedia and storage handlers only when listeners becomes empty. At that point also clear the cached snapshot, preserving shared subscriptions and ensuring the next subscription reads a fresh value.src/hooks/useClipboard.ts (1)
12-28: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHarden the legacy copy fallback.
Use
focus({ preventScroll: true })andsetSelectionRange(0, text.length)beforeexecCommand('copy'). On iOS Safari, an unfocusedreadonlytextarea may not create a valid selection, so the fallback can copy nothing and still returntrue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useClipboard.ts` around lines 12 - 28, Update legacyCopy to focus the readonly textarea with preventScroll enabled and set its selection range from 0 through text.length after selecting and before execCommand('copy'), preserving the existing cleanup and return behavior.src/components/ui/Button.tsx (1)
40-57: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider
aria-disabledinstead of nativedisabledwhile loading.The button sets the native
disabledattribute (Line 43) at the same time asaria-busy(Line 44). When the button that currently has focus (for example, right after a click) becomes natively disabled, most browsers remove it from the tab order and shift focus to<body>. This loses the keyboard/screen-reader user's position during the primary "Generate" action, andaria-busyon a disabled element has little effect because disabled elements are commonly excluded from the accessibility tree.Use
aria-disabledand block the click handler in the loading state instead, so focus stays on the button andaria-busyremains meaningful to assistive technology.♻️ Proposed fix to keep focus during loading
export function Button({ variant = 'secondary', size = 'md', loading = false, icon, className, children, disabled, type = 'button', + onClick, ...rest }: ButtonProps) { return ( <button type={type} - disabled={disabled || loading} + disabled={disabled} + aria-disabled={disabled || loading || undefined} aria-busy={loading || undefined} + onClick={(event) => { + if (loading) { + event.preventDefault(); + return; + } + onClick?.(event); + }} className={cn( 'inline-flex items-center justify-center rounded-xl font-medium transition-colors', 'disabled:cursor-not-allowed disabled:opacity-55', + loading && 'cursor-not-allowed opacity-55', VARIANTS[variant], SIZES[size], className, )} {...rest} > {loading ? <Spinner label="Working" /> : icon} {children} </button> ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/Button.tsx` around lines 40 - 57, Update the Button component’s loading behavior to avoid applying native disabled while loading: use aria-disabled to expose the state, preserve aria-busy, and prevent click handling during loading. Keep the native disabled behavior for the explicit disabled prop and retain the existing styling and button API.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 9-18: Update the workflow-level configuration in ci.yml to grant
only contents: read permissions, and set persist-credentials: false on both
actions/checkout@v4 steps, including the additional checkout step referenced by
the comment.
In `@src/app/api/structure/route.ts`:
- Around line 65-69: Update the success response in the structure route to use
shared `public` caching only when the response does not depend on
`GITHUB_TOKEN`; token-authenticated responses must not be marked public or
stored in shared caches. Preserve the existing cache durations for eligible
unauthenticated responses, and use an appropriate private/no-store policy for
authenticated responses.
In `@src/components/generator/OptionsPanel.tsx`:
- Around line 146-154: The ignore-pattern textarea in OptionsPanel must preserve
raw user input locally instead of deriving its value from
options.ignorePatterns.join('\n'). Add local text state initialized from the
current ignore patterns, update that state on each change while publishing
parsePatternList results, and synchronize or publish the parsed value on blur as
needed without removing whitespace, trailing separators, or empty rows during
typing.
In `@src/components/generator/RepoForm.tsx`:
- Around line 39-41: Update the advancedOpen state in RepoForm so it
synchronizes with changes to values.ref and values.path after mount, including
resets from StructureGenerator.handleReset. Add the appropriate effect keyed to
those fields while preserving user-controlled expansion between prop changes.
In `@src/components/generator/StructureGenerator.tsx`:
- Around line 156-166: Update the useEffect tracking lastReportedError so it
clears lastReportedError.current whenever state.status is not 'error', before
returning. Preserve the existing duplicate-error suppression while remaining in
the error state, and allow the same code/message to notify again after any
successful or other non-error state.
In `@src/components/ui/Switch.tsx`:
- Around line 17-63: Make the implementation match the Switch comment by making
the full row, including the label and description area, toggle through the
existing onCheckedChange behavior while preserving disabled behavior and
preventing duplicate toggles from button bubbling. Use the Switch component’s
wrapper and button handlers to ensure clicks on the visible row activate the
switch.
In `@src/hooks/useLocalStorage.ts`:
- Around line 115-121: Update setValue in useLocalStorage to resolve functional
updates from a fresh synchronous read via readRaw rather than the
render-captured value, so consecutive updates observe prior writes. Add and
reuse a shared decode helper alongside readRaw for consistent JSON parsing,
fallback handling, and optional validation in both the initial read path and
setValue.
In `@src/hooks/useRepoStructure.ts`:
- Around line 117-124: In the request flow around the success and failure
dispatches, verify that the request’s controller is still the current controller
before dispatching either result. Apply this identity check after readError and
response.json resolve, so superseded or reset requests return without
dispatching and cannot restore stale data.
In `@src/lib/__tests__/config.test.ts`:
- Around line 66-79: Update the expected array in the “keeps only non-empty
strings and caps the list” test to include all eight valid repository strings
from the input before applying MAX_RECENT_REPOSITORIES slicing, ensuring the
assertion remains independent of the current cap value.
In `@src/lib/api/schema.ts`:
- Around line 44-52: Update isApiErrorPayload to also validate candidate.code
against the existing ERROR_CODES collection, requiring a valid ErrorCode
alongside the string message before returning true; preserve the current false
result for non-object or null payloads.
In `@src/lib/github/client.ts`:
- Around line 29-39: Update readRateLimit to validate the raw x-ratelimit-limit,
x-ratelimit-remaining, and x-ratelimit-reset header values before converting
them, returning null when any value is null or empty; retain the finite-number
validation for malformed non-empty values. Add a client test exercising a 403
response with no rate-limit headers and verify it remains a plain 403 rather
than being classified as rate-limited.
In `@src/lib/tree/filter.ts`:
- Around line 16-49: Bound user-supplied glob complexity in globToRegExp before
constructing the RegExp: enforce a maximum pattern length and maximum number of
globstar (** or **/) segments, rejecting or safely handling patterns that exceed
either limit. Preserve existing matching behavior for patterns within the limits
and ensure filterTree cannot compile unbounded backtracking expressions from
ignorePatterns.
In `@src/lib/tree/pipeline.ts`:
- Around line 43-44: Update the size aggregation condition in the tree pipeline
around `withAggregatedSizes` so it also runs when `options.sort` is `size-desc`,
while retaining the existing `options.showSizes` behavior. Keep `renderTree` and
`computeStats` display gating unchanged so this only supplies sizes for sorting.
---
Nitpick comments:
In `@src/app/api/structure/route.ts`:
- Around line 30-52: Add IP-keyed rate limiting before the GET handler performs
validation or calls resolveStructure, preferably through the endpoint’s edge
layer or middleware. Ensure rejected callers receive the project’s standard
rate-limit response, while allowed requests retain the existing
structureQuerySchema and resolveStructure flow.
In `@src/components/generator/OutputPanel.tsx`:
- Around line 35-47: Update the lineCount calculation in OutputPanel to count
newline separators directly instead of calling output.split('\n'), and exclude
one trailing newline from the displayed count. Preserve the zero count for empty
output and the existing line-count display behavior.
In `@src/components/generator/StructureGenerator.tsx`:
- Around line 70-73: Update the derivation flow around deriveStructure in
StructureGenerator so the options input is passed through useDeferredValue
before being used for memoized computation. Use the deferred options for
deriveStructure and its dependency tracking, while keeping the current payload
and rootName behavior unchanged.
In `@src/components/layout/Header.tsx`:
- Around line 23-25: Update the shortcut hint kbd element in Header to be hidden
from assistive technology because it is decorative; preserve its visual ⌘K
display, and do not add a misleading accessibility label unless implementing
platform-aware shortcut text.
In `@src/components/theme/ThemeProvider.tsx`:
- Around line 86-99: Update subscribe in ThemeProvider so its cleanup removes
the listener from listeners first, then tears down the shared matchMedia and
storage handlers only when listeners becomes empty. At that point also clear the
cached snapshot, preserving shared subscriptions and ensuring the next
subscription reads a fresh value.
In `@src/components/ui/Button.tsx`:
- Around line 40-57: Update the Button component’s loading behavior to avoid
applying native disabled while loading: use aria-disabled to expose the state,
preserve aria-busy, and prevent click handling during loading. Keep the native
disabled behavior for the explicit disabled prop and retain the existing styling
and button API.
In `@src/components/ui/Toaster.tsx`:
- Around line 75-93: Optionally update the toast state update around setToasts
and timers.current so IDs removed by the MAX_VISIBLE slice have their pending
timer entries cleared. Preserve the existing visibility cap and dismissal
behavior, and only remove timer entries for toasts dropped from state.
In `@src/hooks/useClipboard.ts`:
- Around line 12-28: Update legacyCopy to focus the readonly textarea with
preventScroll enabled and set its selection range from 0 through text.length
after selecting and before execCommand('copy'), preserving the existing cleanup
and return behavior.
In `@src/lib/config.ts`:
- Around line 11-20: Update getSiteUrl to normalize NEXT_PUBLIC_SITE_URL before
returning it: preserve http:// or https:// values, but prepend https:// when the
trimmed explicit value lacks a scheme. Keep the existing trailing-slash removal
and Vercel/localhost fallback behavior unchanged.
In `@src/lib/format/units.ts`:
- Around line 23-29: Hoist the fixed-locale formatter instances to module scope
in units.ts: create one Intl.NumberFormat for formatCount and one
Intl.RelativeTimeFormat for formatRelativeTime, then reuse them inside those
functions instead of constructing formatters on each call. Preserve the existing
locale, options, validation, and output behavior.
In `@src/lib/github/__tests__/client.test.ts`:
- Around line 49-53: Restore the original global fetch after each test suite to
prevent mock leakage: in src/lib/github/__tests__/client.test.ts lines 49-53,
capture globalThis.fetch at module scope and restore it in afterAll; apply the
same capture-and-restore pattern in
src/app/api/structure/__tests__/route.test.ts lines 41-44.
In `@src/lib/github/client.ts`:
- Around line 72-78: Update the fetch request in the GitHub client’s try block
to always enforce a default timeout by creating a fallback AbortSignal.timeout()
and combining it with options.signal via AbortSignal.any(). Preserve caller
cancellation while ensuring requests without a signal still expire, and add a
distinct TimeoutError mapping in the existing error handling if timeout
responses must differ from the ABORTED/499 mapping.
In `@src/lib/github/parse-repo-url.ts`:
- Around line 29-31: Update normaliseInput to trim only leading and trailing
whitespace, removing the replace(/\s+/g, '') behavior. Validate and reject any
remaining internal whitespace instead of silently altering repository names,
while preserving the existing empty-input error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a6eb5e9b-498c-4a42-a5bc-e8696ee4e19a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (79)
.env.example.github/workflows/ci.yml.gitignoreREADME.mdeslint.config.mjsjest.config.jsjest.setup.jsjest.setup.tsnext.config.tspackage.jsonsrc/app/api/structure/__tests__/route.test.tssrc/app/api/structure/route.tssrc/app/error.tsxsrc/app/globals.csssrc/app/layout.tsxsrc/app/manifest.tssrc/app/not-found.tsxsrc/app/page.tsxsrc/app/robots.tssrc/app/sitemap.tssrc/components/Header.tsxsrc/components/InputForm.tsxsrc/components/Notification.tsxsrc/components/OutputDisplay.tsxsrc/components/__tests__/InputForm.test.tsxsrc/components/generator/ErrorNotice.tsxsrc/components/generator/OptionsPanel.tsxsrc/components/generator/OutputPanel.tsxsrc/components/generator/RecentRepositories.tsxsrc/components/generator/RepoForm.tsxsrc/components/generator/RepositorySummary.tsxsrc/components/generator/StructureGenerator.tsxsrc/components/generator/__tests__/RepoForm.test.tsxsrc/components/generator/__tests__/StructureGenerator.test.tsxsrc/components/icons/GithubMark.tsxsrc/components/layout/Footer.tsxsrc/components/layout/Header.tsxsrc/components/theme/ThemeProvider.tsxsrc/components/theme/ThemeToggle.tsxsrc/components/ui/Badge.tsxsrc/components/ui/Button.tsxsrc/components/ui/Select.tsxsrc/components/ui/Spinner.tsxsrc/components/ui/Switch.tsxsrc/components/ui/Toaster.tsxsrc/components/ui/__tests__/Toaster.test.tsxsrc/hooks/useClipboard.tssrc/hooks/useLocalStorage.tssrc/hooks/useRepoStructure.tssrc/lib/__tests__/config.test.tssrc/lib/api/schema.tssrc/lib/config.tssrc/lib/errors.tssrc/lib/format/__tests__/units.test.tssrc/lib/format/units.tssrc/lib/github/__tests__/client.test.tssrc/lib/github/__tests__/parse-repo-url.test.tssrc/lib/github/client.tssrc/lib/github/parse-repo-url.tssrc/lib/github/types.tssrc/lib/logger.tssrc/lib/tree/__tests__/build.test.tssrc/lib/tree/__tests__/filter.test.tssrc/lib/tree/__tests__/pipeline.test.tssrc/lib/tree/__tests__/render.test.tssrc/lib/tree/__tests__/sort.test.tssrc/lib/tree/build.tssrc/lib/tree/filter.tssrc/lib/tree/pipeline.tssrc/lib/tree/render.tssrc/lib/tree/sort.tssrc/lib/tree/stats.tssrc/lib/tree/types.tssrc/lib/utils/cn.tssrc/lib/utils/download.tssrc/utils/githubApi.tssrc/utils/logger.tstsconfig.jsonturbo.json
💤 Files with no reviewable changes (9)
- src/components/Header.tsx
- src/utils/logger.ts
- src/components/InputForm.tsx
- src/components/Notification.tsx
- src/components/OutputDisplay.tsx
- jest.setup.js
- turbo.json
- src/utils/githubApi.ts
- src/components/tests/InputForm.test.tsx
| concurrency: | ||
| group: ci-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| verify: | ||
| name: Lint, typecheck, test | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Set minimum workflow permissions and disable persisted checkout credentials.
The workflow inherits the repository default GITHUB_TOKEN permissions. Both actions/checkout steps retain the token in local Git configuration. Define permissions: contents: read at workflow scope. Set persist-credentials: false for both checkout steps.
Proposed fix
on:
push:
branches: [main]
pull_request:
workflow_dispatch:
+permissions:
+ contents: read
+
concurrency:
group: ci-${{ github.ref }}
cancel-in-progress: true
@@
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
+ with:
+ persist-credentials: false
@@
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
+ with:
+ persist-credentials: falseAlso applies to: 48-48
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 18-18: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 9 - 18, Update the workflow-level
configuration in ci.yml to grant only contents: read permissions, and set
persist-credentials: false on both actions/checkout@v4 steps, including the
additional checkout step referenced by the comment.
Source: Linters/SAST tools
| return NextResponse.json(payload, { | ||
| headers: { | ||
| 'Cache-Control': `public, s-maxage=${GITHUB_CACHE_TTL_SECONDS}, stale-while-revalidate=${GITHUB_CACHE_TTL_SECONDS * 2}`, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not mark token-authenticated responses as public.
Every success response is cached with public, s-maxage=.... When GITHUB_TOKEN grants access to private repositories, the private repository structure is stored in shared and intermediary caches. Restrict shared caching to responses that do not depend on the token.
🛡️ Proposed change
+ const shared = !process.env.GITHUB_TOKEN?.trim();
+
return NextResponse.json(payload, {
headers: {
- 'Cache-Control': `public, s-maxage=${GITHUB_CACHE_TTL_SECONDS}, stale-while-revalidate=${GITHUB_CACHE_TTL_SECONDS * 2}`,
+ 'Cache-Control': shared
+ ? `public, s-maxage=${GITHUB_CACHE_TTL_SECONDS}, stale-while-revalidate=${GITHUB_CACHE_TTL_SECONDS * 2}`
+ : `private, max-age=${GITHUB_CACHE_TTL_SECONDS}`,
},
});If the deployment only ever targets public repositories, keep the current header and document that constraint in .env.example.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return NextResponse.json(payload, { | |
| headers: { | |
| 'Cache-Control': `public, s-maxage=${GITHUB_CACHE_TTL_SECONDS}, stale-while-revalidate=${GITHUB_CACHE_TTL_SECONDS * 2}`, | |
| }, | |
| }); | |
| const shared = !process.env.GITHUB_TOKEN?.trim(); | |
| return NextResponse.json(payload, { | |
| headers: { | |
| 'Cache-Control': shared | |
| ? `public, s-maxage=${GITHUB_CACHE_TTL_SECONDS}, stale-while-revalidate=${GITHUB_CACHE_TTL_SECONDS * 2}` | |
| : `private, max-age=${GITHUB_CACHE_TTL_SECONDS}`, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/api/structure/route.ts` around lines 65 - 69, Update the success
response in the structure route to use shared `public` caching only when the
response does not depend on `GITHUB_TOKEN`; token-authenticated responses must
not be marked public or stored in shared caches. Preserve the existing cache
durations for eligible unauthenticated responses, and use an appropriate
private/no-store policy for authenticated responses.
| <textarea | ||
| id={ignoreId} | ||
| rows={2} | ||
| spellCheck={false} | ||
| placeholder="*.test.ts, docs/**, !docs/api" | ||
| value={options.ignorePatterns.join('\n')} | ||
| onChange={(event) => onChange({ ignorePatterns: parsePatternList(event.target.value) })} | ||
| className="resize-y rounded-xl border border-line bg-surface px-3 py-2 font-mono text-xs text-ink placeholder:font-sans placeholder:text-ink-subtle hover:border-line-strong" | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect parsePatternList to confirm whether it drops empty or untrimmed segments.
fd -t f 'filter.ts' src/lib/tree --exec rg -n -A 20 'export function parsePatternList'Repository: idugeni/fostgen
Length of output: 866
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the OptionsPanel component around the textarea implementation.
printf 'Files matching OptionsPanel.tsx:\n'
fd -t f '^OptionsPanel\.tsx$' src
printf '\nOptionsPanel outline:\n'
ast-grep outline src/components/generator/OptionsPanel.tsx --view expanded || true
printf '\nRelevant OptionsPanel lines:\n'
sed -n '1,240p' src/components/generator/OptionsPanel.tsx | cat -nRepository: idugeni/fostgen
Length of output: 8133
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
function parsePatternList(input) {
return input
.split(/[\n,]/)
.map((value) => value.trim())
.filter(Boolean);
}
function roundtrip(source) {
const before = source.split('\n');
const parsed = parsePatternList(source);
const rendered = parsed.join('\n');
return {
before,
parsed,
rendered,
identical: source === rendered
};
}
for (const [name, source] of [
['currently supports two lines', '*.test.ts\ndocs/**'],
['trailing newline at end', '*.test.ts\ndocs/**\n'],
['empty line between patterns', '*.test.ts\ndocs/**\n!important.ts'],
['leading space before second pattern', '*.test.ts\n docs/**'],
['comma with trailing space', '*.test.ts, docs/**'],
['comma-separated empty segment', '*.test.ts,\n,docs/**'],
]) {
console.log(JSON.stringify({ name, ...roundtrip(source) }, null, 2));
}
JSRepository: idugeni/fostgen
Length of output: 1430
Keep the ignore-pattern text in local textarea state.
parsePatternList() trims entries and filters empty segments, then value={options.ignorePatterns.join('\n')} rewrites user input after each keystroke. This makes trailing newlines, commas, leading/trailing whitespace, and empty rows disappear from the textarea. Store the raw text locally and publish the parsed list only when the parsed value changes or on blur.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/generator/OptionsPanel.tsx` around lines 146 - 154, The
ignore-pattern textarea in OptionsPanel must preserve raw user input locally
instead of deriving its value from options.ignorePatterns.join('\n'). Add local
text state initialized from the current ignore patterns, update that state on
each change while publishing parsePatternList results, and synchronize or
publish the parsed value on blur as needed without removing whitespace, trailing
separators, or empty rows during typing.
| const [advancedOpen, setAdvancedOpen] = useState( | ||
| () => values.ref.length > 0 || values.path.length > 0, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Sync advancedOpen with prop changes, not just the initial mount.
advancedOpen is computed once from values.ref and values.path through the lazy useState initializer. This value never updates after mount, so it does not track values changes coming from the parent.
StructureGenerator (see src/components/generator/StructureGenerator.tsx, handleReset) resets form to EMPTY_FORM but does not remount RepoForm. If a user expands "Branch & sub-directory", enters a value, then clicks "Clear", the section stays expanded even though values.ref and values.path are now empty. This is a stale-state glitch, not a functional blocker.
♻️ Proposed fix using an effect to resync on prop change
- const [advancedOpen, setAdvancedOpen] = useState(
- () => values.ref.length > 0 || values.path.length > 0,
- );
+ const [advancedOpen, setAdvancedOpen] = useState(
+ () => values.ref.length > 0 || values.path.length > 0,
+ );
+
+ useEffect(() => {
+ if (values.ref.length === 0 && values.path.length === 0) {
+ setAdvancedOpen(false);
+ }
+ }, [values.ref, values.path]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [advancedOpen, setAdvancedOpen] = useState( | |
| () => values.ref.length > 0 || values.path.length > 0, | |
| ); | |
| const [advancedOpen, setAdvancedOpen] = useState( | |
| () => values.ref.length > 0 || values.path.length > 0, | |
| ); | |
| useEffect(() => { | |
| if (values.ref.length === 0 && values.path.length === 0) { | |
| setAdvancedOpen(false); | |
| } | |
| }, [values.ref, values.path]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/generator/RepoForm.tsx` around lines 39 - 41, Update the
advancedOpen state in RepoForm so it synchronizes with changes to values.ref and
values.path after mount, including resets from StructureGenerator.handleReset.
Add the appropriate effect keyed to those fields while preserving
user-controlled expansion between prop changes.
| const lastReportedError = useRef<string | null>(null); | ||
| useEffect(() => { | ||
| if (state.status !== 'error' || !state.error) return; | ||
| const signature = `${state.error.code}:${state.error.message}`; | ||
| if (lastReportedError.current === signature) return; | ||
| lastReportedError.current = signature; | ||
| notify.error({ | ||
| title: state.error.message, | ||
| ...(state.error.hint ? { description: state.error.hint } : {}), | ||
| }); | ||
| }, [state.status, state.error, notify]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Repeated identical errors produce no toast.
lastReportedError holds the last code:message signature and is cleared only in handleReset. If a request fails, then succeeds, then fails again with the same code and message, the second failure emits no toast. Clear the signature when the status leaves error.
🐛 Proposed fix: clear the signature on non-error states
useEffect(() => {
- if (state.status !== 'error' || !state.error) return;
+ if (state.status !== 'error' || !state.error) {
+ lastReportedError.current = null;
+ return;
+ }
const signature = `${state.error.code}:${state.error.message}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const lastReportedError = useRef<string | null>(null); | |
| useEffect(() => { | |
| if (state.status !== 'error' || !state.error) return; | |
| const signature = `${state.error.code}:${state.error.message}`; | |
| if (lastReportedError.current === signature) return; | |
| lastReportedError.current = signature; | |
| notify.error({ | |
| title: state.error.message, | |
| ...(state.error.hint ? { description: state.error.hint } : {}), | |
| }); | |
| }, [state.status, state.error, notify]); | |
| const lastReportedError = useRef<string | null>(null); | |
| useEffect(() => { | |
| if (state.status !== 'error' || !state.error) { | |
| lastReportedError.current = null; | |
| return; | |
| } | |
| const signature = `${state.error.code}:${state.error.message}`; | |
| if (lastReportedError.current === signature) return; | |
| lastReportedError.current = signature; | |
| notify.error({ | |
| title: state.error.message, | |
| ...(state.error.hint ? { description: state.error.hint } : {}), | |
| }); | |
| }, [state.status, state.error, notify]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/generator/StructureGenerator.tsx` around lines 156 - 166,
Update the useEffect tracking lastReportedError so it clears
lastReportedError.current whenever state.status is not 'error', before
returning. Preserve the existing duplicate-error suppression while remaining in
the error state, and allow the same code/message to notify again after any
successful or other non-error state.
| it('keeps only non-empty strings and caps the list', () => { | ||
| const input = ['a/b', '', 3, null, 'c/d', 'e/f', 'g/h', 'i/j', 'k/l', 'm/n', 'o/p']; | ||
| const result = parseRecentRepositories(input); | ||
|
|
||
| expect(result).toEqual([ | ||
| 'a/b', | ||
| 'c/d', | ||
| 'e/f', | ||
| 'g/h', | ||
| 'i/j', | ||
| 'k/l', | ||
| ].slice(0, MAX_RECENT_REPOSITORIES)); | ||
| expect(result?.length).toBeLessThanOrEqual(MAX_RECENT_REPOSITORIES); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'MAX_RECENT_REPOSITORIES' src/lib/config.tsRepository: idugeni/fostgen
Length of output: 242
Fix the truncated expected list in this test.
The input contains 8 valid repository strings after filtering, but the expected array only lists 6 before applying .slice(0, MAX_RECENT_REPOSITORIES). Keep the expected value in sync with the parsed results so the test does not only pass due to the current MAX_RECENT_REPOSITORIES value being 6.
🔧 Proposed fix
it('keeps only non-empty strings and caps the list', () => {
const input = ['a/b', '', 3, null, 'c/d', 'e/f', 'g/h', 'i/j', 'k/l', 'm/n', 'o/p'];
const result = parseRecentRepositories(input);
- expect(result).toEqual([
- 'a/b',
- 'c/d',
- 'e/f',
- 'g/h',
- 'i/j',
- 'k/l',
- ].slice(0, MAX_RECENT_REPOSITORIES));
+ expect(result).toEqual(
+ ['a/b', 'c/d', 'e/f', 'g/h', 'i/j', 'k/l', 'm/n', 'o/p'].slice(0, MAX_RECENT_REPOSITORIES),
+ );
expect(result?.length).toBeLessThanOrEqual(MAX_RECENT_REPOSITORIES);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('keeps only non-empty strings and caps the list', () => { | |
| const input = ['a/b', '', 3, null, 'c/d', 'e/f', 'g/h', 'i/j', 'k/l', 'm/n', 'o/p']; | |
| const result = parseRecentRepositories(input); | |
| expect(result).toEqual([ | |
| 'a/b', | |
| 'c/d', | |
| 'e/f', | |
| 'g/h', | |
| 'i/j', | |
| 'k/l', | |
| ].slice(0, MAX_RECENT_REPOSITORIES)); | |
| expect(result?.length).toBeLessThanOrEqual(MAX_RECENT_REPOSITORIES); | |
| }); | |
| it('keeps only non-empty strings and caps the list', () => { | |
| const input = ['a/b', '', 3, null, 'c/d', 'e/f', 'g/h', 'i/j', 'k/l', 'm/n', 'o/p']; | |
| const result = parseRecentRepositories(input); | |
| expect(result).toEqual( | |
| ['a/b', 'c/d', 'e/f', 'g/h', 'i/j', 'k/l', 'm/n', 'o/p'].slice(0, MAX_RECENT_REPOSITORIES), | |
| ); | |
| expect(result?.length).toBeLessThanOrEqual(MAX_RECENT_REPOSITORIES); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/__tests__/config.test.ts` around lines 66 - 79, Update the expected
array in the “keeps only non-empty strings and caps the list” test to include
all eight valid repository strings from the input before applying
MAX_RECENT_REPOSITORIES slicing, ensuring the assertion remains independent of
the current cap value.
| export function isApiErrorPayload(value: unknown): value is ApiErrorPayload { | ||
| if (typeof value !== 'object' || value === null) return false; | ||
| const candidate = (value as { error?: unknown }).error; | ||
| return ( | ||
| typeof candidate === 'object' && | ||
| candidate !== null && | ||
| typeof (candidate as { message?: unknown }).message === 'string' | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Validate error.code in the guard.
The guard only checks message. A payload such as { error: { message: 'x' } } passes, so the consumer in src/hooks/useRepoStructure.ts assigns undefined to StructureError.code, which the type declares as ErrorCode. Any UI branching on the code then fails silently. Validate the code against ERROR_CODES.
🛡️ Proposed fix
-import { type ErrorCode } from '`@/lib/errors`';
+import { ERROR_CODES, type ErrorCode } from '`@/lib/errors`'; export function isApiErrorPayload(value: unknown): value is ApiErrorPayload {
if (typeof value !== 'object' || value === null) return false;
const candidate = (value as { error?: unknown }).error;
- return (
- typeof candidate === 'object' &&
- candidate !== null &&
- typeof (candidate as { message?: unknown }).message === 'string'
- );
+ if (typeof candidate !== 'object' || candidate === null) return false;
+ const { code, message } = candidate as { code?: unknown; message?: unknown };
+ return (
+ typeof message === 'string' &&
+ typeof code === 'string' &&
+ (ERROR_CODES as readonly string[]).includes(code)
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function isApiErrorPayload(value: unknown): value is ApiErrorPayload { | |
| if (typeof value !== 'object' || value === null) return false; | |
| const candidate = (value as { error?: unknown }).error; | |
| return ( | |
| typeof candidate === 'object' && | |
| candidate !== null && | |
| typeof (candidate as { message?: unknown }).message === 'string' | |
| ); | |
| } | |
| import { ERROR_CODES, type ErrorCode } from '`@/lib/errors`'; | |
| export function isApiErrorPayload(value: unknown): value is ApiErrorPayload { | |
| if (typeof value !== 'object' || value === null) return false; | |
| const candidate = (value as { error?: unknown }).error; | |
| if (typeof candidate !== 'object' || candidate === null) return false; | |
| const { code, message } = candidate as { code?: unknown; message?: unknown }; | |
| return ( | |
| typeof message === 'string' && | |
| typeof code === 'string' && | |
| (ERROR_CODES as readonly string[]).includes(code) | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/api/schema.ts` around lines 44 - 52, Update isApiErrorPayload to also
validate candidate.code against the existing ERROR_CODES collection, requiring a
valid ErrorCode alongside the string message before returning true; preserve the
current false result for non-object or null payloads.
| function readRateLimit(headers: Headers, authenticated: boolean): RateLimitInfo | null { | ||
| const limit = Number(headers.get('x-ratelimit-limit')); | ||
| const remaining = Number(headers.get('x-ratelimit-remaining')); | ||
| const reset = Number(headers.get('x-ratelimit-reset')); | ||
|
|
||
| if (!Number.isFinite(limit) || !Number.isFinite(remaining) || !Number.isFinite(reset)) { | ||
| return null; | ||
| } | ||
|
|
||
| return { limit, remaining, reset, authenticated }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix the absent-header check: Number(null) is 0, not NaN.
headers.get() returns null when a header is absent, and Number(null) is 0, which passes Number.isFinite. So when GitHub omits the rate-limit headers, this function returns { limit: 0, remaining: 0, reset: 0 } instead of null. Three consequences follow:
- Line 100:
response.status === 403 && rateLimit?.remaining === 0becomes true for every 403 that carries no rate-limit headers. A private-repository 403 is reported asRATE_LIMITEDwith status 429, so the user never receives theGITHUB_TOKENhint from line 105. - Lines 42-47:
resetof0producesretryAfter: 0and the message "Try again in about 0 min".src/app/api/structure/route.tsline 16 then sendsRetry-After: 0. StructurePayload.rateLimitreports a fabricated 0/0 quota to the client.
Number('') is also 0, so an empty header value behaves the same way. Read the header value first and reject null or empty strings.
🐛 Proposed fix
+function readHeaderNumber(headers: Headers, name: string): number | null {
+ const raw = headers.get(name);
+ if (raw === null || raw.trim() === '') return null;
+ const value = Number(raw);
+ return Number.isFinite(value) ? value : null;
+}
+
function readRateLimit(headers: Headers, authenticated: boolean): RateLimitInfo | null {
- const limit = Number(headers.get('x-ratelimit-limit'));
- const remaining = Number(headers.get('x-ratelimit-remaining'));
- const reset = Number(headers.get('x-ratelimit-reset'));
-
- if (!Number.isFinite(limit) || !Number.isFinite(remaining) || !Number.isFinite(reset)) {
- return null;
- }
+ const limit = readHeaderNumber(headers, 'x-ratelimit-limit');
+ const remaining = readHeaderNumber(headers, 'x-ratelimit-remaining');
+ const reset = readHeaderNumber(headers, 'x-ratelimit-reset');
+
+ if (limit === null || remaining === null || reset === null) {
+ return null;
+ }
return { limit, remaining, reset, authenticated };
}The existing test "distinguishes a plain 403 from a rate limit" cannot detect this, because the jsonResponse helper in src/lib/github/__tests__/client.test.ts always injects rate-limit headers. Add a case with no rate-limit headers.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function readRateLimit(headers: Headers, authenticated: boolean): RateLimitInfo | null { | |
| const limit = Number(headers.get('x-ratelimit-limit')); | |
| const remaining = Number(headers.get('x-ratelimit-remaining')); | |
| const reset = Number(headers.get('x-ratelimit-reset')); | |
| if (!Number.isFinite(limit) || !Number.isFinite(remaining) || !Number.isFinite(reset)) { | |
| return null; | |
| } | |
| return { limit, remaining, reset, authenticated }; | |
| } | |
| function readHeaderNumber(headers: Headers, name: string): number | null { | |
| const raw = headers.get(name); | |
| if (raw === null || raw.trim() === '') return null; | |
| const value = Number(raw); | |
| return Number.isFinite(value) ? value : null; | |
| } | |
| function readRateLimit(headers: Headers, authenticated: boolean): RateLimitInfo | null { | |
| const limit = readHeaderNumber(headers, 'x-ratelimit-limit'); | |
| const remaining = readHeaderNumber(headers, 'x-ratelimit-remaining'); | |
| const reset = readHeaderNumber(headers, 'x-ratelimit-reset'); | |
| if (limit === null || remaining === null || reset === null) { | |
| return null; | |
| } | |
| return { limit, remaining, reset, authenticated }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/github/client.ts` around lines 29 - 39, Update readRateLimit to
validate the raw x-ratelimit-limit, x-ratelimit-remaining, and x-ratelimit-reset
header values before converting them, returning null when any value is null or
empty; retain the finite-number validation for malformed non-empty values. Add a
client test exercising a 403 response with no rate-limit headers and verify it
remains a plain 403 rather than being classified as rate-limited.
| function globToRegExp(glob: string): RegExp { | ||
| let source = ''; | ||
| let index = 0; | ||
|
|
||
| while (index < glob.length) { | ||
| const char = glob[index] ?? ''; | ||
|
|
||
| if (char === '*') { | ||
| const isGlobstar = glob[index + 1] === '*'; | ||
|
|
||
| if (isGlobstar && glob[index + 2] === '/') { | ||
| // `**/foo` must also match a root-level `foo`, hence the optional group. | ||
| source += '(?:.*/)?'; | ||
| index += 3; | ||
| continue; | ||
| } | ||
|
|
||
| source += isGlobstar ? '.*' : '[^/]*'; | ||
| index += isGlobstar ? 2 : 1; | ||
| continue; | ||
| } | ||
|
|
||
| if (char === '?') { | ||
| source += '[^/]'; | ||
| index += 1; | ||
| continue; | ||
| } | ||
|
|
||
| source += char.replace(REGEX_SPECIALS, '\\$&'); | ||
| index += 1; | ||
| } | ||
|
|
||
| return new RegExp(`^${source}$`); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
[request_verification]Repository: idugeni/fostgen
Length of output: 213
🌐 Web query:
Does the picomatch npm package guard against ReDoS in glob pattern compilation?
💡 Result:
The picomatch npm package has historically been vulnerable to Regular Expression Denial of Service (ReDoS) due to catastrophic backtracking when processing certain crafted extglob patterns [1][2]. Specifically, patterns using extglob quantifiers like + and *, particularly when nested or containing overlapping alternatives, could cause significant CPU consumption and block the Node.js event loop [1][3]. This vulnerability was addressed in versions 4.0.4, 3.0.2, and 2.3.2 [1][2]. Users are strongly advised to upgrade to one of these versions (or later) to mitigate the risk [2][4]. In addition to upgrading, the package provides configuration options that can help guard against such issues, particularly when dealing with untrusted input [5][6]: - maxExtglobRecursion: This option allows you to set a limit on nested quantified extglobs. If the limit is exceeded, the extglob is treated as a literal string instead of being compiled into a potentially dangerous regular expression [5][6]. - noextglob: This option can be set to true to completely disable support for extglobs [5][6]. If you are processing untrusted glob patterns, it is recommended to use these safeguards or implement other mitigations such as validating input patterns, enforcing strict allowlists, or running matching operations in isolated processes with resource limits [1][7].
Citations:
- 1: GHSA-c2c7-rcm5-vvqj
- 2: https://app.opencve.io/cve/CVE-2026-33671
- 3: https://deps.dev/advisory/osv/GHSA-c2c7-rcm5-vvqj
- 4: GHSA-c2c7-rcm5-vvqj
- 5: https://www.npmjs.com/package/picomatch
- 6: https://github.com/micromatch/picomatch/blob/refs/heads/master/README.md
- 7: https://osv.dev/vulnerability/GHSA-c2c7-rcm5-vvqj
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | grep -E 'src/lib/tree/filter.ts|src/lib/__tests__/config.test.ts|GeneratorOptions|ignorePatterns' || true
echo
echo "== filter.ts =="
if [ -f src/lib/tree/filter.ts ]; then
wc -l src/lib/tree/filter.ts
sed -n '1,190p' src/lib/tree/filter.ts | cat -n
fi
echo
echo "== tests mentioning ignorePatterns/config =="
rg -n "ignorePatterns|GeneratorOptions|globToRegExp|filterTree" -S . --glob '!node_modules' --glob '!dist' --glob '!build' || true
echo
echo "== package files =="
git ls-files | grep -E '(^|/)(package.json|pnpm-lock.yaml|package-lock.json|yarn.lock)$' | sed -n '1,50p'
if [ -f package.json ]; then
sed -n '1,220p' package.json | cat -n
fiRepository: idugeni/fostgen
Length of output: 13388
🌐 Web query:
picomatch latest README options maxExtglobRecursion noextglob
💡 Result:
In picomatch, the options maxExtglobRecursion and noextglob control how extended glob patterns are processed [1][2]. maxExtglobRecursion This option is a number or boolean with a default value of 0 [1][2]. It serves as a safeguard against complex or potentially dangerous nested quantified extglobs (e.g., +(+(a))) [1][2]. If the recursion depth exceeds the specified limit, the extglob is treated as a literal string rather than being compiled into a regular expression [1][2]. To disable this safeguard, you can set the option to false [1][2]. noextglob This option is a boolean with a default value of false [1][2]. When set to true, it disables support for extended glob syntax (such as +(a|b)), causing those patterns to be treated as literal characters instead of special pattern syntax [1][3]. The option noext is also available as an alias for noextglob [1][2].
Citations:
- 1: https://www.npmjs.com/package/picomatch
- 2: https://github.com/micromatch/picomatch?tab=readme-ov-file
- 3: https://deepwiki.com/micromatch/picomatch/4.2-matching-behavior-options
🌐 Web query:
minimatch regular expression denial of service source option
💡 Result:
Minimatch has been subject to several Regular Expression Denial of Service (ReDoS) vulnerabilities due to its reliance on JavaScript regular expressions for glob matching [1][2][3]. These issues stem from the generation of complex, exponentially back-tracking regular expressions when processing specific types of untrusted input [4][5][6]. There is no single "source" option that acts as a universal defense against ReDoS in minimatch [1]. Instead, mitigation relies on updating to patched versions of the library and, in specific cases, using configuration options to disable vulnerable features [4][2][3]. Key Mitigation Strategies: 1. Update the Library: Several recent CVEs (e.g., CVE-2026-26996, CVE-2026-27904) have addressed these vulnerabilities. Patched versions (e.g., 10.2.1, 10.2.3 and various corresponding versions for older major releases) include fixes for the root causes of catastrophic backtracking [2][3][7][6]. 2. Disable Extglobs: For the vulnerability related to nested * extglobs, passing the option { noext: true } to the minimatch function can mitigate the risk by disabling extglob processing entirely [4]. 3. General Security Advice: The maintainer explicitly warns that any system taking user-supplied input and using it to generate regular expressions is inherently susceptible to ReDoS [1]. Developers are advised to: - Sanitize or validate user-provided glob patterns before passing them to minimatch [4][5]. - Avoid allowing users to influence patterns in high-risk contexts, such as CI/CD pipelines, build tools, or multi-tenant configuration systems [8][5][6]. The library's configuration interface does include options like maxExtglobRecursion and maxGlobstarRecursion, which are intended to limit resource consumption, though they are not a substitute for proper input sanitization [9][10]. The library maintainer has stated that because malicious patterns can always be crafted against a bash-compatible glob matcher using JavaScript regex, future ReDoS reports are considered "working as intended" and resolved by this standing warning [1].
Citations:
- 1: https://isaacs.github.io/minimatch/
- 2: https://nvd.nist.gov/vuln/detail/cve-2026-26996
- 3: https://nvd.nist.gov/vuln/detail/cve-2026-27904
- 4: https://osv.dev/vulnerability/GHSA-23c5-xmqv-rm74
- 5: https://osv.dev/vulnerability/GHSA-3ppc-4f35-3m26
- 6: https://osv.dev/vulnerability/CVE-2026-27903
- 7: https://security.snyk.io/vuln/SNYK-JS-MINIMATCH-15353387
- 8: GHSA-7r86-cg39-jmmj
- 9: https://isaacs.github.io/minimatch/interfaces/index.MinimatchOptions.html
- 10: https://registry.npmjs.org/minimatch
Bound glob pattern complexity before compiling ignore patterns.
globToRegExp turns repeated **/ segments into chained (?:.*/)? optional groups. A crafted pattern with many **/ segments followed by a non-matching literal can backtrack exponentially when filterTree tests it against generated node paths. Since ignorePatterns is editable by users, cap the number of ** segments per pattern, cap pattern length, or use a glob matcher with ReDoS-safe options.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 47-47: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${source}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/tree/filter.ts` around lines 16 - 49, Bound user-supplied glob
complexity in globToRegExp before constructing the RegExp: enforce a maximum
pattern length and maximum number of globstar (** or **/) segments, rejecting or
safely handling patterns that exceed either limit. Preserve existing matching
behavior for patterns within the limits and ensure filterTree cannot compile
unbounded backtracking expressions from ignorePatterns.
Source: Linters/SAST tools
| const sized = options.showSizes ? withAggregatedSizes(filtered) : filtered; | ||
| const sorted = sortTree(sized, options.sort); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Aggregate sizes when the sort mode is size-desc.
Line 43 computes directory sizes only when options.showSizes is true. If a user selects size-desc with showSizes off, directories carry no size. compare in src/lib/tree/sort.ts line 24 then reads them as 0 and places every directory after every file, which contradicts the "Largest first" label. Aggregate sizes whenever the ordering depends on them.
🐛 Proposed fix
- const sized = options.showSizes ? withAggregatedSizes(filtered) : filtered;
+ const needsSizes = options.showSizes || options.sort === 'size-desc';
+ const sized = needsSizes ? withAggregatedSizes(filtered) : filtered;
const sorted = sortTree(sized, options.sort);renderTree and computeStats still gate size display on options.showSizes, so the rendered output does not change.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sized = options.showSizes ? withAggregatedSizes(filtered) : filtered; | |
| const sorted = sortTree(sized, options.sort); | |
| const needsSizes = options.showSizes || options.sort === 'size-desc'; | |
| const sized = needsSizes ? withAggregatedSizes(filtered) : filtered; | |
| const sorted = sortTree(sized, options.sort); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/tree/pipeline.ts` around lines 43 - 44, Update the size aggregation
condition in the tree pipeline around `withAggregatedSizes` so it also runs when
`options.sort` is `size-desc`, while retaining the existing `options.showSizes`
behavior. Keep `renderTree` and `computeStats` display gating unchanged so this
only supplies sizes for sorting.
2 participants
This pull request was created by @kiro-agent on behalf of @idugeni 👻
Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro Web
Full modernisation pass: dependencies, architecture, correctness, features, accessibility and tests.
Upgrades
next/jest)next lintno longer exists in Next 16, soeslint.config.mjsnow consumes the flat-config arrays fromeslint-config-nextdirectly andnpm run lintcallseslintitself.tsconfighardened withnoUncheckedIndexedAccess,noImplicitOverride,noFallthroughCasesInSwitch, ES2022 target.Dead code removed
Each of these was not just unused but non-functional:
winston+@types/express—src/utils/logger.tsregistered Express middleware in a Next.js app. Replaced by a dependency-free isomorphic logger.@svgr/webpackrule innext.config.tsreferenced a package that was never installed; it would have failed on the first SVG import.turbo— no workspaces, and no script ever invoked it.src/components/__tests__/InputForm.test.tsx— asserted an API the component never had (rendered<InputForm onSubmit>only and expected internal URL validation). It could not compile, let alone pass.Architecture
src/lib/github— URL parser (shorthand, SSH remotes,raw./api.hosts,/tree/+/blob/deep links), typed API client with optionalGITHUB_TOKEN, rate-limit surfacing, and automatic ref widening for branch names containing slashes (release/2026-07).src/lib/tree—build->filter->sort->renderas pure, independently tested stages composed byderiveStructure.GET /api/structure— zod-validated query, cache headers, and a structured error taxonomy (AppErrorwith 12 codes).GitHub is now called from the server: the token stays private, responses are cached and shared, and anonymous visitors are not billed against their own per-IP quota. The response carries the unfiltered tree, so changing a format or depth limit re-renders instantly with no refetch.
Correctness fixes
typefield instead of being inferred from "has children", so empty directories stay directories (previously rendered as files).type: 'commit') are a distinct node type rather than being mislabelled as files.Repository is too large.useLocalStorageand the theme provider useuseSyncExternalStore, which removes the hydration mismatch and the cascading render fromsetState-in-effect.Features
*,**,?, trailing/,!negation) with a default noise list that can be switched off.robots.ts,sitemap.ts,manifest.ts, JSON-LD, OpenGraph metadata, error and not-found boundaries, and baseline security headers.Accessibility
Labelled controls,
role="switch"toggles, dismissible live-region toasts, and validation errors wired to their field viaaria-describedby/aria-invalid.Testing
169 tests across 13 suites, ~80% coverage (70% floor enforced via
coverageThreshold), replacing the one broken test file. Added a GitHub Actions workflow running lint, typecheck, coverage and a production build.Verification performed
npm run lint,npm run typecheck,npm test— all clean.npm run build— succeeds (Turbopack, 6 routes).next start:?url=idugeni/fostgen-> 200, 36 entries, correctCache-Control.?url=vercel/next.js&ref=canary&path=packages/next-env-> 200, scoped correctly.INVALID_REQUEST400,INVALID_URL400,NOT_FOUND404,BRANCH_NOT_FOUND404,PATH_NOT_FOUND404.x-powered-byabsent,/does-not-exist-> 404.Folders 6 / Files 29 / Depth 4 / Size 402.9 KB), summary panel, and format switching without a refetch.Notes for the reviewer
.gitignorenow un-ignores.env.example.next buildrewrotetsconfig.json(jsx: react-jsx,.next/dev/types); those edits are Next's own and are committed as-is.GITHUB_TOKENis optional. Without it the app still works at GitHub's 60 requests/hour anonymous limit, and the UI warns when the remaining quota gets low.Summary by CodeRabbit
New Features
Documentation
Tests