feat(resident): onboard repositories without a resident - #2798
justinhelmer wants to merge 1 commit into
Conversation
|
Warning Polylane could not verify the production impact of this pull request. Re-checked the same single commit rebased onto current main; Also considered · 5 refuted
Analysed against 7 cloud accounts and 1 repository
Polylane could not find the cloud resources this repository manages, so this review looked at the entire cloud account. Connect this repository to its resources and the next review will focus on exactly what this code deploys to. Polylane analysed Did this help? React 👍 or 👎 so the next review is sharper. |
There was a problem hiding this comment.
Changes requested: Cold registrations lose their default branch during execution, cannot update command overrides, and contradict the About-block contract.
Warning
Changes requested · head 378b64c · 3 findings: 1 major, 2 minor
| Severity | Finding | Where |
|---|---|---|
| major | F1 Carry the registered default ref into actual cold checkout preparation | src/execution/factory.ts:1133 |
| minor | F2 Include cold registrations when merging reconfigure command overrides | src/core/commands/repo.ts:670 |
| minor | F3 Spec contradiction — routing-and-config.md item 11: onboarding can now be cold, not always warm | docs/reference/specs/routing-and-config.md:27 |
F1 invariant: A cold registration's effective ref must reach the actual checkout: an authoritative requested ref wins, otherwise the registered default ref is used. Passing it only through unused executor options is insufficient.
- Fresh coding or PR-less review on Cloudflare, cold registration defaultRef=develop, GitHub default branch=main, no requested ref. → Checkout preparation or the authoritative model target specifies develop; the run does not silently clone main. CloudflareSandboxExecutor does not consume its repo/ref options.
- Fresh coding or PR-less review on E2B, cold registration defaultRef=develop, GitHub default branch=main, no requested ref. → The effective develop ref reaches checkout preparation or the authoritative model target; E2BExecutor.open does not consume its repo/ref options.
- One-shot local CLI run with a configured resident registry, a cold registration on develop, and no requested ref. → The effective develop ref reaches checkout preparation or the authoritative model target; LocalExecutor creation does not consume PerThreadInputs.ref.
- Cold registration with an explicitly resolved task ref on any per-thread backend. → The requested ref takes priority over defaultRef and remains available to the checkout and run context.
- Cold registration targeted by a PR review or a coding Ship child with an authoritative bound branch. → Existing exact-PR-head preparation and owned-Ship-branch preparation retain their authoritative target; registration defaults cannot replace it.
- Resume with a recorded per-thread workspace binding after the registration default changes. → Reuse the recorded backend and workspace/ref without applying the new registration default or provisioning a replacement.
- A cold registration reaches selection with a ready-environment requirement or with a reclaimed run owner. → Refuse before admitting cold model work; resolving a default branch cannot waive readiness or ownership checks.
- Successful ordinary cold-registration selection. → Use the per-thread path without a resident attach or snapshot seed, while still carrying its effective ref beyond selection.
Full review
F1 — High confidence. Registering a repo with --no-resident --ref develop does not make a fresh coding task use develop. The factory passes that ref only into executor options that Cloudflare and E2B do not consume; local execution also ignores it. No selected ref reaches the run context or prompt, so ordinary cloning uses GitHub’s default branch instead. Carry the effective ref into checkout preparation and verify the resulting branch, rather than only asserting private executor options.
F2 — High confidence. Cold entries appear under /residents.repositories, but command reconfiguration searches only /residents.residents. Consequently, repo reconfigure acme/api --test "new-test" falsely reports an existing cold registration as not onboarded; adding --ref does not help. Merge both collections before looking up the current command table, preserving untouched keys.
F3 — High confidence. Item 11 and its validation criterion still promise “onboarded = warm,” and selfDescriptionBlock injects that statement into every resident-enabled agent prompt. The new cold-registration mode makes it false. Update the About-block source, owning spec, and associated proofs to distinguish registration from resident provisioning.
|
Production cutover scope: deploy and verify the containing release first. The operator-selected repository already has a resident and active development depends on it. Keep its current registration and environment until an agreed brief cutover window, with fresh idle/workspace checks before any offboard. No production registration, teardown or conversion has been performed by this shepherd. |
eee888e to
ee86e47
Compare
ee86e47 to
e0d98b2
Compare
There was a problem hiding this comment.
Changes requested: Prior ref propagation, reconfiguration, and About/spec fixes are present, but fresh Cloudflare Ship setup discards the prepared sibling checkout.
Warning
Changes requested · head e0d98b2 · 1 finding: 1 major
| Severity | Finding | Where |
|---|---|---|
| major | F1 Ship preparation discards the fresh cold checkout and reuses its predecessor | src/execution/factory.ts:1175 |
F1 invariant: A fresh cold registration's verified checkout must remain the selected source through subsequent preparation, durable binding, prompt composition and model execution; an unbound retained predecessor must not replace it. Authoritative expected heads and recorded recovery bindings must remain independently enforced.
- Ordinary fresh Cloudflare coding or PR-less review with a registered default ref and an existing checkout. → Prepare a sibling at the registered default, leave the predecessor unchanged, and carry the sibling into the binding, prompt and model checkout.
- Ordinary fresh E2B coding or PR-less review with a registered default ref and an existing checkout. → Prepare and retain the verified sibling on E2B without replacing or modifying the predecessor.
- Ordinary fresh local CLI coding or PR-less review with a registered default ref and an existing checkout. → Prepare and retain the verified sibling in the local thread workspace without replacing or modifying the predecessor.
- Explicit task ref on any of the three per-thread backends, different from the registered default. → Prepare the explicit ref and preserve that selected checkout through admission and execution; the default must not substitute.
- Fresh Cloudflare coding Ship child with no existing checkout and no resolved exact head. → The factory's verified owned-branch checkout remains the source when the dispatcher performs publication preparation and records the initial fetched head.
- Fresh Cloudflare coding Ship child after ordinary cold coding left checkout on main, while the owned Ship branch is plan/p/u1. → Subsequent Ship preparation verifies the already-prepared sibling on plan/p/u1, not the retained main checkout, and admits the correctly prepared task.
- Fresh Cloudflare coding Ship child with a retained checkout on the owned branch but at a stale remote tip or with unpublished commits. → Use the sibling at the freshly fetched owned-branch tip; do not fail against, reset, or borrow the predecessor's local head.
- Fresh Cloudflare coding Ship child whose retained checkout belongs to another repository. → Verify and retain the new sibling for the authorized repository; never substitute or execute model work in the predecessor.
- Fresh Cloudflare coding Ship child whose retained checkout has an obsolete Git Door origin. → Verification uses the new sibling's exact current Door endpoint without probing through or repointing the retained origin.
- Fresh Cloudflare coding Ship child whose predecessor has the correct branch, current remote HEAD and origin, but dirty tracked, untracked or ignored private files. → Publication preparation, durable binding and model execution continue to use the clean sibling, leaving the predecessor's private bytes outside the new task checkout.
- Fresh local or E2B coding Ship selection with a retained checkout. → Retain the factory-prepared sibling; the Cloudflare-only second-preparation condition must not change these paths.
- Exact PR review or coding continuation with an authoritative expected head. → Keep caller-owned exact-target preparation and strict expected-head validation; fresh registration preparation cannot replace that authority.
- Recorded cold workspace recovery after registry defaults change. → Observe the recorded backend, ref and workspace without a fresh clone, second fresh-Ship preparation, or replacement of the original publication base.
- Ready-environment requirement or reclaimed owner during cold selection/preparation. → Refuse before model admission; retaining a prepared checkout cannot bypass readiness or ownership fences.
| Prior finding | Resolution | Evidence at this head |
|---|---|---|
| review:5422820975:F1 | fixed | Verified the original effective-ref invariant at this head: ResidentExecutor.probeStatus carries defaultRef; factory.ts cold selection chooses ctx.ref before the validated registration default and runs prepareColdPublicationCheckout on Cloudflare, E2B and local instead of relying on unused executor options. selection.cold reaches workspaceBindingFor, dispatcher context/meta and makeSystemComposer's checkout instruction. Exact PR heads bypass registration cloning for authorizeAttachedHead; owned Ship refs and caller expected-head checks remain authoritative rather than being replaced by defaults. Recorded recovery bypasses registry selection and uses its backend/ref/workspace without cloning or renewing publicationBaseSha. Ready-environment and owner rechecks still refuse cold admission; ordinary cold selection attaches no resident and seeds no snapshot. The distinct downstream checkout-provenance defect is reported as new F1. |
| review:5422820975:F2 | fixed | Checked repoReconfigure in src/core/commands/repo.ts: it combines residents and repositories before resource lookup, merges overrides over record.commands, and adds defaultRef only when supplied. The new optional-ref test covers test-only and test-plus-ref cold updates preserving build/install; the existing ref-only path does not read or replace commands. ResidentRegistryDO.updateConfig spreads the original registration and updates only supplied fields. |
| review:5422820975:F3 | fixed | Checked src/core/selfDescription.ts, its test and capability snapshot, and docs/reference/specs/routing-and-config.md item 11 plus its validation row: all now distinguish default warm resident provisioning from --no-resident cold registration, describe reconfiguration of either mode, and state that cold registrations consume no resident slots. The old onboarded=warm assertion is explicitly replaced by independent warm/cold and resident-cap assertions. |
Full review
F1 — High confidence. Fresh cold setup correctly clones beside a retained checkout, but dispatcher.ts:3735 then prepares the Ship checkout again using literal checkout, ignoring the verified sibling and overwriting selection.cold. If the predecessor is on main or an outdated tip, Ship fails before model work despite having prepared the correct branch. If its branch, HEAD and origin match but it contains private edits, the new task instead runs in that predecessor. Make subsequent verification honor the selected sibling while retaining strict expected-head checks, and cover this through dispatch rather than only the factory.
Test-guard dispositions: the self-description replacement preserves verification with explicit warm/cold and resident-cap assertions; the readiness test parameterization retains the original not-onboarded refusal and adds the cold case.
Co-Authored-By: coreplane-switchboard[bot] <318072483+coreplane-switchboard[bot]@users.noreply.github.com>
e0d98b2 to
457fd91
Compare
Repositories can register with
repo onboard owner/name --no-residentand run tasks in per-thread workspaces on their effective branch. Registration avoids warm compute while preserving access checks, retained work and authoritative PR targets.Why: Repo discovery currently depends on resident onboarding, making a warm environment mandatory even when occasional cold execution is sufficient. Registration should retain the same access checks while allowing operators to choose whether to provision compute.
Where to look
Feedback wanted: Check preservation of the selected checkout through Ship admission, effective branch preparation, cancellation semantics and resident lifecycle exclusion.
Risk: Registry and checkout changes stay together to make cold registrations usable. Bot/resident rollout requires existing preservation gates. Rollback must retain recognition of cold registrations.
Verified: npm run verify passed: 16,775 tests (6 skipped), typechecks, lint, formatting, consistency, package/Worker builds and docs checks. Production rollout remains held.
Decisions (4)
Validation (9 criteria)
For agents
The original Ship ownership, two-round history and rejected native publication settlement remain preserved. A separate explicitly authorized local publication uses the same PR and branch. Standalone MCP review at e0d98b2 posted review 5423620561, confirmed the earlier five behavior fixes and reported one new selected-checkout integration finding. This revision addresses that finding with strict prepared-path/head rechecking and real Git dispatch regression proofs. Production deployment and resident conversion remain pending with the existing lifecycle owner.
🤖 Generated with Claude Code