fix(careers): scan the repo for install locations instead of guessing - #189
Merged
Merged
Conversation
The careers PR flow blind-probed ten hardcoded paths at a single root (astro.config.*, app/layout.*, src/app/layout.*). Anything else — a site at web/, frontend/, packages/site/, or any monorepo path the customer didn't type into the subdirectory box — came back "No Next.js App Router or Astro site found there", which reads as a missing feature rather than a missed guess. The stats tracker installer already solved this: findInstallCandidates scans, ranks, and hands the UI a picker with a manual-path escape hatch. This brings the careers flow to the same shape. - repos: add listRepoTree, one recursive git-trees request for the whole file list. The contents API can only confirm paths you already know. - install-careers: careersCandidatesFromTree derives every possible route directory from that listing and ranks it with the tracker's heuristics (apps/ and sites/ up, examples/fixtures down, root-level app first). findCareersCandidates wraps it and still falls back to the direct probe when the tree is unavailable or GitHub truncated it, so a huge repo degrades to the old behaviour rather than claiming the site is missing. Locations that already have a careers page are listed and flagged, not hidden — "you already have this" beats an empty list. - route: add mode=candidates (read-only, no PR, no run row) and accept target_dir on submit. A directory chosen in the browser is user input, so verifyCareersDir re-probes for the framework marker before any write; the installer's promise is that it only writes where it can positively see a site. - ui: replace the single detect line with the ranked list, preselecting the best match, plus a manual directory box for truncated repos. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan54 finding(s) HIGH/CRITICAL: 10 | MEDIUM: 44
…and 4 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
On
/projects/:id/stats/careers, "Add to my repo" → Check repo blind-probes ten hardcoded paths at a single root:Any other layout — a site at
web/,frontend/,packages/site/, or any monorepo path the customer didn't type into the subdirectory box — returnsnulland the UI says "No Next.js App Router or Astro site found there." That reads as a missing feature rather than a missed guess.The stats tracker installer already solved this.
findInstallCandidatesscans, ranks, and hands the UI a picker with a manual-path escape hatch. This brings the careers flow to the same shape.Changes
lib/github/repos.ts— newlistRepoTree: one recursive git-trees request for the whole file list. The contents API can only confirm paths you already know.lib/github/install-careers.tscareersCandidatesFromTreederives every possible route directory from that listing and ranks it with the tracker's heuristics (apps/andsites/up,examples//fixtures down, root-levelappfirst). Nested route layouts (app/blog/layout.tsx,app/(marketing)/layout.tsx) are correctly ignored — only a root layout marks an app dir.findCareersCandidateswraps it and still falls back to the direct probe when the tree is unavailable or GitHub truncated it, so a huge repo degrades to the old behaviour rather than claiming the site is missing.verifyCareersDirre-probes for the framework marker before any write.app/api/.../install-careers/route.ts—mode=candidates(read-only: no PR, noproject_pr_runsrow) andtarget_diron submit.careers-install.tsx— the ranked list replaces the single detect line, best match preselected, with a manual directory box for truncated repos. Button is now Scan repo / Rescan.Security note
A directory chosen in the browser is user input.
submitnever truststarget_dir: it re-probes for the framework marker server-side and 422s if there isn't one, and rejects..traversal. The installer's promise — it only writes where it can positively see a site — is unchanged.Testing
17 new cases in
tests/careers-install-pr.test.tscovering monorepo discovery, ranking order, Astro config → pages mapping, nested-layout rejection, vendored-path skipping, rootPath filtering, existing-page flagging, the truncated/unavailable-tree fallbacks, andverifyCareersDiraccept/reject/traversal.npm test— 1374 passed, 7 skipped, 0 failures (112 files)npm run typecheck— cleanNot verified against a live GitHub repo; the tree API interaction is covered by mocks only.
🤖 Generated with Claude Code