Skip to content

feat(land)!: --bump finds its targets from the App's installations - #709

Merged
kixelated merged 6 commits into
mainfrom
quest/m0/pin-bumps/discover
Nov 13, 2026
Merged

kixelated merged 6 commits into
mainfrom
quest/m0/pin-bumps/discover

Conversation

@kixelated

@kixelated kixelated commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m0/pin-bumps/discover.md.

Models

  • Implementer: Claude Code, claude-opus-5-5, low
  • Implementer: Claude Code, claude-opus-5-5, low effort (iterate, S)
  • Implementer: Claude Code, claude-opus-5-5, low effort (iterate, S)

Summary

  • quest land watch --bump is 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/repositories with an installation-wide contents: read token). It reads each one's default-branch flake.lock through 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 into appRequest in land-daemon.ts (where feat(land)!: retire the batching host gate, quest land serve #707 moved the watcher's shared helpers). A new installations() helper is added next to it.
  • questInput now uses a shared questInputs, which discoverTargets also uses.
  • README's "Pin bumps" section and --help describe the switch.
  • Discovery isolates failures: one repository's failed lock request or one installation's failed token or listing is recorded, the rest are still searched and bumped, the partial result is not cached, and the pass then throws so the daemon reports and retries it.
  • New tests: a repository that locks quest is found, a non-locker is never connected or cloned, a repository locking quest more than once is reported and skipped, and an uninstalled repository gets no more bumps once the base moves.

Choices made without asking

  • Discovery reruns only when the base SHA changes. The result is cached in memory per watcher process. A newly installed repository therefore gets its first bump on the next merge, not at once.
  • Archived repositories and the watched repository itself are skipped. A repository with no 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 a bump skips line.
  • A target that is uninstalled while its bump is still checking stops being settled after the next base move.

Follow-ups

  • None. Turning on --bump on the host is already planned in kixel-ai/dotfiles.

Expect a conflict with #707, which moves land-serve.ts helpers into land-daemon.ts. appRequest and installations should move with them.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5 with low)

Review

Rewritten by quest review after each round.

  • Head: 6b1d1a2

  • Base: origin/main at b62403e

  • Reviewer: Codex, gpt-6.1-sol, medium effort

  • Session: 01a1d74f-22e9-7a90-97a2-50c8f0f5d7fb, round 1

  • Score: 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-f2b98ca04a23

  • Rounds: 2; findings 1 (medium 1): fixed 1, refuted 0, open 0

kixelated and others added 2 commits November 13, 2026 14:21
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title The bump pass finds its own targets feat(land)!: --bump finds its targets from the App's installations Nov 13, 2026
…very

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 13, 2026 22:29
@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fa012dc9-fa72-4ca4-bac8-7378ad36961f
Commits

Reviewing files that changed from the base of the PR and between b62403e and 6b1d1a2.

Files selected for processing: 7

  • README.md
  • quest/m0/pin-bumps/discover.md
  • src/cli/land-bump.ts
  • src/cli/land-daemon.ts
  • src/cli/land-watch.ts
  • src/cli/main.ts
  • test/land-bump.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ 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 discoverTargets lists each installation's repositories and reads each default-branch flake.lock through 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: bumpPass reruns discovery only when the base SHA changes, and caches the result in BumpContext.discovered.
  • App plumbing: JWT signing moves into a private appRequest. A new installations() pages /app/installations, drops suspended ones, and returns one installation-wide token minter per installation.
  • CLI/README: --bump is now a boolean switch. The watched-repository guard and the same helper 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.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using claude-opus-5-5 | 𝕏

Comment thread src/cli/land-bump.ts Resolved
kixelated and others added 2 commits November 13, 2026 15:04
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pullfrog[bot]
pullfrog Bot previously approved these changes Nov 13, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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: discoverTargets now catches a failed contents/flake.lock request, such as a 403, records it in a new failures parameter, and moves on to the next repository. A lock that cannot be decoded is still reported as a bump skips line.
  • Per-installation isolation: if one installation's token mint or /installation/repositories listing fails, that failure is recorded as discovery in installation N and the other installations are still searched.
  • Retry on partial discovery: bumpPass bumps 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.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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: appRequest and installations now sit in land-daemon.ts next to installationToken, with options typed { appId: string } in place of Pick<ServeOptions, "appId">. Their logic is unchanged.
  • Import updates: land-watch.ts imports installations from ./land-daemon, and main.ts drops the Repository import that the old --bump <owner/repo> parser used. InvalidArgumentError is still used elsewhere in main.ts.

Typecheck passes, and the land-bump and land-watch suites pass (57 tests).

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 13, 2026
Merged via the queue into main with commit 2a0f466 Nov 13, 2026
20 checks passed
@kixelated
kixelated deleted the quest/m0/pin-bumps/discover branch November 13, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant