Let the installer that opens the PR see what the picker sees - #220
Merged
Conversation
Follow-on to deb65fe. The tree walk landed in findInstallCandidates, which is what the picker calls. installTracker — the function that actually opens the stats PR — still had only the canonical list and code search, so the two flows disagreed: the picker could show a publisher their shell at apps/web/src/components/Layout.jsx while a direct install insisted the repo had no template file at all. - installTracker walks the tree too, before code search, since search does not index private repos. Same ranking, same read budget. - It also stops running code search once the repo turns out to be installed already; a hit elsewhere would have injected a second copy into an unrelated file. - Drop `templates?` from the -100 path penalty. For Django, Flask, Jinja and Rails that directory is exactly where the shell lives, and the penalty buried the only installable file in the repo. A real layout still outranks it. - Read the basename two ways in the tree walk. Stripping one extension off application.html.erb leaves "application.html", which matches no shell name, so Rails and Laravel were invisible; add `footer` for WordPress themes, and name all three in the canonical list as well. - Fetch the tree once at the top of findInstallCandidates and reuse it for both the walk and the canonical probes. Knowing which files exist turns ~35 serial misses into one read per real file, and the probes now run in parallel. Tests: the mock in install-tracker.test.ts had no listRepoTree, so the tree walk was throwing on an undefined function, being swallowed by its own catch, and every test was exercising the old canonical path instead of the one that ships. Wired up, plus coverage for the tree fallback, the already-installed guard, the templates ranking, an unavailable tree, and a truncated one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LT2u8KFbyeicqE9z2kUHJR
ThreatCrush Security Scan40 finding(s) HIGH/CRITICAL: 3 | MEDIUM: 28 | LOW: 9
Snippets are redacted; ThreatCrush never prints matched credential material. |
An empty default return infers never[], so mockResolvedValueOnce with a real hit failed tsc --noEmit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LT2u8KFbyeicqE9z2kUHJR
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.
Follow-on to deb65fe, which landed the git-tree walk while this was in flight. That commit put the walk in
findInstallCandidates— what the picker calls.installTracker, the function that actually opens the stats PR, still had only the canonical list and code search, so the two flows disagreed: the picker could show a publisher their shell atapps/web/src/components/Layout.jsxwhile a direct install insisted the repo had no template file at all.What changed
installTrackerwalks the tree too, before code search, since search does not index private repos. Same ranking, same read budget.It stops running code search once the repo turns out to be installed already. A hit elsewhere would have had it inject a second copy into an unrelated file.
Dropped
templates?from the -100 path penalty. For Django, Flask, Jinja and Rails that directory is exactly where the document shell lives, so the penalty buried the only installable file in the repo underneath everything else. A realapp/layout.tsxstill outranks it.The tree walk now reads a basename two ways. Stripping one extension off
application.html.erbleavesapplication.html, which matches no shell name — so Rails and Laravel were invisible to the gate. Addedfooterfor WordPress themes, and named all three in the canonical list too.One tree fetch, used twice.
findInstallCandidatesfetched the tree for the walk but still probed the canonical list blind. Knowing which files exist turns ~35 serial misses into one read per real file, and the probes now run in parallel. A truncated tree is treated as proving nothing, so the full list is still probed there.Tests
The mock in
install-tracker.test.tshad nolistRepoTree, so the tree walk was throwing on an undefined function, getting swallowed by its owncatch, and every test was exercising the old canonical-probe path instead of the one that ships. Wired up, plus coverage for the tree fallback ininstallTracker, the already-installed guard, thetemplates/ranking, an unavailable tree, and a truncated one.Verification
pnpm installstill fails at HEAD (the@profullstack/autoblogpin), so there is novitestortscin this checkout. The changed source was bundled unmodified with theesbuildthat is innode_modulesand run against a fake GitHub API — 22 checks, all passing:templates/base.htmlis offered and no longer scores -100;app/layout.tsxstill outranks itrootPathstill scopes discovery, with no double-prefixed probedist/andnode_modules/never offeredinstallTrackeropens a PR at a tree-only shell, and leaves an already-installed repo alone even when code search offers another file</body>, one loaderThe existing
installTrackertests all pass an explicittargetPath, so they skip discovery and are unaffected.🤖 Generated with Claude Code
https://claude.ai/code/session_01LT2u8KFbyeicqE9z2kUHJR