Repository navigation
feat(land)!: --bump finds its targets from the App's installations - #709
Conversation
Quest-Claim: fcb8b0242209d115
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…very Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 17 minutes. View limit details
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. |
There was a problem hiding this comment.
ℹ️ No critical issues. One robustness gap is noted inline.
Reviewed changes
This is the first review of the switch from a static --bump <owner/repo> list to discovering targets from the App's installations.
- Discovery: the new
discoverTargetslists each installation's repositories and reads each default-branchflake.lockthrough the contents API. It keeps repositories that lock the source through exactly one root input and reports the ones that lock it more than once. - Caching:
bumpPassreruns discovery only when the base SHA changes, and caches the result inBumpContext.discovered. - App plumbing: JWT signing moves into a private
appRequest. A newinstallations()pages/app/installations, drops suspended ones, and returns one installation-wide token minter per installation. - CLI/README:
--bumpis now a boolean switch. The watched-repository guard and thesamehelper are removed, since discovery skips the source itself. - Tests: new cases cover a repository found across installations, the source being skipped, a missing or malformed lock, a repository locking the source twice, a non-locker never being connected, and rediscovery once the base moves.
claude-opus-5-5 | 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found. The discovery-isolation gap from the prior review is addressed cleanly.
Reviewed changes
This run covers the two commits since the prior review, which isolate discovery failures.
- Per-repository isolation:
discoverTargetsnow catches a failedcontents/flake.lockrequest, such as a 403, records it in a newfailuresparameter, and moves on to the next repository. A lock that cannot be decoded is still reported as abump skipsline. - Per-installation isolation: if one installation's token mint or
/installation/repositorieslisting fails, that failure is recorded asdiscovery in installation Nand the other installations are still searched. - Retry on partial discovery:
bumpPassbumps the targets it did find, does not cache a discovery that had failures, and then throws so the daemon reports the failure and retries. - Tests: added a case where one installation returns 403 and one repository's lock returns 403, and the remaining target is still found.
ℹ️ A persistent lock failure re-runs full discovery on every pass
A failure that never clears, such as an org whose IP allow list always blocks the lock read, keeps context.discovered unset. Every daemon pass then lists every installation again, reads every lock again, and throws again. The PR treats such failures as possibly transient, which is reasonable. If one stuck repository turns out to be noisy on the host, it could be cached as a skip for that SHA instead of a failure.
claude-opus-5-5 | 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found. The merge carries the discovery helpers into their new home unchanged.
Reviewed changes
This run covers the merge of main since the prior review, which moves the watcher's App helpers to follow #707.
- Relocated App helpers:
appRequestandinstallationsnow sit inland-daemon.tsnext toinstallationToken, withoptionstyped{ appId: string }in place ofPick<ServeOptions, "appId">. Their logic is unchanged. - Import updates:
land-watch.tsimportsinstallationsfrom./land-daemon, andmain.tsdrops theRepositoryimport that the old--bump <owner/repo>parser used.InvalidArgumentErroris still used elsewhere inmain.ts.
Typecheck passes, and the land-bump and land-watch suites pass (57 tests).
claude-opus-5-5 | 𝕏

Finishes
quest/m0/pin-bumps/discover.md.Models
Summary
quest land watch --bumpis now a switch. The repeatable--bump <owner/repo>is gone. Each time the base moves, a pass lists the repositories of every App installation (GET /app/installations, then/installation/repositorieswith an installation-widecontents: readtoken). It reads each one's default-branchflake.lockthrough the contents API without cloning anything, and keeps as targets the repositories that lock the watched repository through exactly one root input.installationToken's JWT signing is moved intoappRequestinland-daemon.ts(where feat(land)!: retire the batching host gate, quest land serve #707 moved the watcher's shared helpers). A newinstallations()helper is added next to it.questInputnow uses a sharedquestInputs, whichdiscoverTargetsalso uses.--helpdescribe the switch.Choices made without asking
flake.lock(404) or with zero quest inputs is skipped without a message. One with more than one quest input is skipped and reported on abump skipsline.checkingstops being settled after the next base move.Follow-ups
--bumpon the host is already planned in kixel-ai/dotfiles.Expect a conflict with #707, which moves
land-serve.tshelpers intoland-daemon.ts.appRequestandinstallationsshould move with them.🤖 Generated with Claude Code
(Written by Claude Opus 5.5 with low)
Review
Rewritten by
quest reviewafter each round.Head: 6b1d1a2
Base: origin/main at b62403e
Reviewer: Codex, gpt-6.1-sol, medium effort
Session:
01a1d74f-22e9-7a90-97a2-50c8f0f5d7fb, round 1Score: 9/10,
Static review found no actionable regressions; discovery, caching, CLI changes, and failure isolation appear consistent with the intended behavior, though builds and tests were not run as instructed.Rounds: 5; findings 3 (medium 3): fixed 3, refuted 0, open 0
Findings: none
Reviewer: Codex, gpt-6.1-sol, low effort, shadow
Session:
01a1d704-e993-71d2-aea4-f2b98ca04a23Rounds: 2; findings 1 (medium 1): fixed 1, refuted 0, open 0