feat(imports): consumer-declared read-only repo snapshots mounted into every spawn #264

Merged

dominik.polakovics merged 3 commits from afk/261 into main 2026-08-03 01:41:30 +02:00

Conversation 2 Commits 3 Files changed 38 +4311 -95

dominik.polakovics commented 2026-08-03 00:58:58 +02:00

Owner

Copy link

Closes #261

Implements the full read-only-imports feature end-to-end per PRD #260 and the triage brief: a repo's settings declare other lab repos its instances may read; every spawn (all run kinds, by construction through the single instance.Launch path) materializes a .git-less snapshot of each import's freshly-fetched origin/<default> into the per-run directory outside the worktree, mounted read-only at a host-identical path; /pull-base refreshes the snapshots in place and digests per-import changes.

What landed

Store + migrations — migrations/{sqlite,postgres}/0021_repo_imports.sql: repo_imports (repo_id, target_repo_id) PK on the pair, consumer FK cascades, target FK carries no delete action as the backstop behind the store-level guard. store.DeleteRepo refuses while importers exist with an ImportersError naming them — force structurally cannot bypass it (guard sits below the layer force is read at; pinned by tests).

API + UI — GET/POST/DELETE /api/v1/repos/{id}/imports; self-import and unknown target are 400s from reposvc; delete-refusal maps to 409 naming the importers. New repo-settings Imports section (section-registry pattern, co-located vitest suite; still-cloning candidates stay selectable — the spawn-time refusal is the guard).

Materializer — gitx.Engine.MaterializeSnapshot: fetch → resolve → only then clear-and-extract (git archive streamed through pure-Go tar extraction). The destination inode is preserved across re-materialization (the dir is bind-mounted into a live container) and a refresh against a dead remote never destroys the existing snapshot. Integration tests with real bare repos: fresh export, upstream change, upstream deletion, idempotent same-commit, inode stability, write-protected previous snapshot, fetch failure, no worktree leaked.

Spawn path — parallel materialization into <state>/instances/<runID>/imports/<name> sits after per-run tree creation and before AddWorktree (the claim), so a failing target refuses the spawn with read-only import "<name>": … and nothing is claimed — for an AFK spec the issue stays claimable, never parked (tested: no worktree, no branch, no run row, per-run tree wiped). Container runner renders one -v path:path:ro bind per import — the mount inventory's first :ro bind, consuming the contract ADR-0053 reserved. Host runner applies best-effort chmod a-w. Every spawn kind carries imports by construction; there is no per-spawn toggle.

Context file — new "Read-only imports" section (name, absolute path, snapshotted commit, and the read-only-snapshot-outside-the-working-repo statement); new golden; the import-less render stays byte-identical to the existing goldens.

/pull-base — re-materializes every spawn-materialized import in place (declared-mid-run targets are skipped: the mount inventory is fixed at spawn) and the injected digest gains one line per import (unchanged, <old12>..<new12> (N commits), refreshed to <new12>, or refresh failed — <reason>); a refresh failure is reported and logged, never fatal to the pull; base-up-to-date-but-import-moved still injects a digest. Host-runner write protection is reapplied after refresh.

Teardown — snapshots live inside the per-run tree, so instancehome.Wipe/SweepAll remove them for free; both are hardened to clear write-protected trees (verified through the real Stop path).

Docs — CONTEXT.md gains the read-only import term with its avoid-list plus cross-references (Runner mount inventory, /pull-base, Dev image, Relationships); ADR-0063 records the grant model, materialization, refusal, refresh, and delete-guard decisions, citing ADR-0052 (Containerized runner), ADR-0053 (Per-repo dev images), ADR-0062 (Schedules), ADR-0035.

Verification

• go build ./..., go vet ./..., gofmt -l clean; go test -count=1 ./... fully green (store tests run both dialect paths; postgres subtests skip without LAB_TEST_POSTGRES_DSN, as in CI)
• web: npm test 982/982 green, npm run lint and npm run build clean
• golangci-lint was not available in this environment — deferring to CI for that check

🤖 Generated with Claude Code

Closes #261 Implements the full read-only-imports feature end-to-end per PRD #260 and the triage brief: a repo's settings declare other lab repos its instances may read; every spawn (all run kinds, by construction through the single `instance.Launch` path) materializes a `.git`-less snapshot of each import's freshly-fetched `origin/<default>` into the per-run directory outside the worktree, mounted read-only at a host-identical path; `/pull-base` refreshes the snapshots in place and digests per-import changes. ## What landed **Store + migrations** — `migrations/{sqlite,postgres}/0021_repo_imports.sql`: `repo_imports (repo_id, target_repo_id)` PK on the pair, consumer FK cascades, target FK carries no delete action as the backstop behind the store-level guard. `store.DeleteRepo` refuses while importers exist with an `ImportersError` naming them — `force` structurally cannot bypass it (guard sits below the layer force is read at; pinned by tests). **API + UI** — `GET/POST/DELETE /api/v1/repos/{id}/imports`; self-import and unknown target are 400s from reposvc; delete-refusal maps to 409 naming the importers. New repo-settings **Imports** section (section-registry pattern, co-located vitest suite; still-cloning candidates stay selectable — the spawn-time refusal is the guard). **Materializer** — `gitx.Engine.MaterializeSnapshot`: fetch → resolve → only then clear-and-extract (`git archive` streamed through pure-Go tar extraction). The destination inode is preserved across re-materialization (the dir is bind-mounted into a live container) and a refresh against a dead remote never destroys the existing snapshot. Integration tests with real bare repos: fresh export, upstream change, upstream deletion, idempotent same-commit, inode stability, write-protected previous snapshot, fetch failure, no worktree leaked. **Spawn path** — parallel materialization into `<state>/instances/<runID>/imports/<name>` sits after per-run tree creation and *before* `AddWorktree` (the claim), so a failing target refuses the spawn with `read-only import "<name>": …` and nothing is claimed — for an AFK spec the issue stays claimable, never parked (tested: no worktree, no branch, no run row, per-run tree wiped). Container runner renders one `-v path:path:ro` bind per import — the mount inventory's first `:ro` bind, consuming the contract ADR-0053 reserved. Host runner applies best-effort `chmod a-w`. Every spawn kind carries imports by construction; there is no per-spawn toggle. **Context file** — new "Read-only imports" section (name, absolute path, snapshotted commit, and the read-only-snapshot-outside-the-working-repo statement); new golden; the import-less render stays byte-identical to the existing goldens. **/pull-base** — re-materializes every spawn-materialized import in place (declared-mid-run targets are skipped: the mount inventory is fixed at spawn) and the injected digest gains one line per import (`unchanged`, `<old12>..<new12> (N commits)`, `refreshed to <new12>`, or `refresh failed — <reason>`); a refresh failure is reported and logged, never fatal to the pull; base-up-to-date-but-import-moved still injects a digest. Host-runner write protection is reapplied after refresh. **Teardown** — snapshots live inside the per-run tree, so `instancehome.Wipe`/`SweepAll` remove them for free; both are hardened to clear write-protected trees (verified through the real Stop path). **Docs** — CONTEXT.md gains the **read-only import** term with its avoid-list plus cross-references (Runner mount inventory, `/pull-base`, Dev image, Relationships); ADR-0063 records the grant model, materialization, refusal, refresh, and delete-guard decisions, citing ADR-0052 (Containerized runner), ADR-0053 (Per-repo dev images), ADR-0062 (Schedules), ADR-0035. ## Verification - `go build ./...`, `go vet ./...`, `gofmt -l` clean; `go test -count=1 ./...` fully green (store tests run both dialect paths; postgres subtests skip without `LAB_TEST_POSTGRES_DSN`, as in CI) - web: `npm test` 982/982 green, `npm run lint` and `npm run build` clean - `golangci-lint` was not available in this environment — deferring to CI for that check 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dominik.polakovics added 1 commit 2026-08-03 00:58:58 +02:00

feat(imports): consumer-declared read-only repo snapshots mounted into every spawn ... 477ed4338b

A repo's settings now declare read-only imports: other lab repos whose
code its instances may read (ADR-0063, PRD #260).

- migrations 0021 (both dialects): repo_imports keyed (repo_id,
  target_repo_id), consumer FK cascades, target FK backstops the
  store-level delete guard that names the importers (force never bypasses)
- reposvc/httpapi: repo-scoped imports list/add/remove; self-import and
  unknown target are 400s; delete-refusal maps to 409 naming importers
- web: repo-settings Imports section (picker over registered repos)
- gitx.MaterializeSnapshot: fetch -> resolve -> clear-and-extract in
  place, dest inode preserved for the live bind mount, no .git
- instance.Launch: parallel materialization into
  <state>/instances/<runID>/imports/<name> before the claim; a failure
  refuses the spawn naming the target, so no issue is parked; host
  runner best-effort chmod a-w; sidecar <name>.commit for /pull-base
- podmanx: ImportDirs render one `-v path:path:ro` bind each at
  host-identical paths - the mount inventory's first :ro bind
- seeder: context-file "Read-only imports" section (name, absolute
  path, snapshotted commit), new golden, import-less render
  byte-identical
- pull: /pull-base re-materializes spawn-materialized imports in place
  and digests per-import changes; a failed refresh is reported, not
  fatal
- instancehome: ImportsPath third sibling of home/runtime; Wipe/SweepAll
  hardened to remove write-protected trees
- CONTEXT.md: the read-only import term and cross-references

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

dominik.polakovics referenced this pull request from a commit 2026-08-03 01:18:32 +02:00

chore(seeder): apply De Morgan's law in the ordering assertion (QF1001)

dominik.polakovics added 1 commit 2026-08-03 01:18:32 +02:00

chore(seeder): apply De Morgan's law in the ordering assertion (QF1001) ... faabf8a795

golangci-lint 2.12.2 (staticcheck QF1001) failed `ci / native` on PR #264
at internal/seeder/seeder_test.go:655 — the only issue in the run; every
other step was green. The negated conjunction becomes the equivalent
disjunction of negations; the assertion's meaning is unchanged.

The bump's own verification could not catch this: golangci-lint was not
available in the authoring environment, so the check was deferred to CI.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

dominik.polakovics commented 2026-08-03 01:27:34 +02:00

Author

Owner

Copy link

[autoland] verdict: pass

This was generated by AI while landing a PR.

PASS — validated, awaiting the human's merge confirmation.

Signal relied on: ci / native (pull_request) — aggregate success, 8m27s, run 360. Not re-run locally (no Go/Node toolchain in the landing session; CI is the repo's required check and it already exercised the full gate: SPA lint/format/test/build, go build -tags ui, go test -tags ui ./..., and the untagged golangci-lint 2.12.2 pass).

One fix pushed while landing (faabf8a, see the comment below): run 358 failed on a single staticcheck QF1001 at internal/seeder/seeder_test.go:655 — the check the PR body flagged as unverifiable in the authoring environment. Rewritten as the equivalent disjunction; run 360 is green.

Conventions: Conventional Commits title ✓ · Closes #261 present and resolving ✓ · no textual conflict against main (git merge-tree clean).

Diff scope: 38 files, +4311/−95, mapping onto issue #261's acceptance criteria with no drive-by changes. Spot-checked and sound: repo names are [A-Za-z0-9_-] and globally UNIQUE, so imports/<name> and its <name>.commit sidecar structurally cannot collide or traverse; materializeImports sits before AddWorktree so a target-side failure refuses the spawn with nothing claimed; refreshImports runs on the context.WithoutCancel context, so a dropped HTTP connection cannot kill an extraction midway; the UpToDate semantics change is handled correctly by its one consumer (internal/httpapi/chat.go:464), and gitx guarantees NewHead == OldHead on that path so renderDigest's collapsed branch fires exactly when intended.

CONCERNS (non-blocking, 1):

internal/gitx/materialize.go:57-65 — MaterializeSnapshot clears the destination before extracting, so a failure during extraction (timeout on git archive, disk full, I/O error) leaves a partial tree while the <name>.commit sidecar still names the old commit. /pull-base then reports refresh failed — <reason>, and both internal/pull/pull.go ("the snapshot on disk is still the one the run was spawned with") and ADR-0063 ("keeps its still-valid snapshot") state the snapshot is intact — which holds for fetch/resolve failures, the ordering the design deliberately buys, but not for this window. The agent would be told it still holds the old snapshot while actually holding a truncated one.

Test coverage matches that boundary: TestMaterializeSnapshot_fetchFailureLeavesSnapshotIntact pins the pre-clear failure; there is no post-clear equivalent. Narrow window and not a merge blocker, but worth a follow-up — either extract to a sibling directory and swap contents in (keeping the inode), or downgrade the "intact" claim in both the doc comment and the digest wording.

[autoland] verdict: pass > *This was generated by AI while landing a PR.* **PASS** — validated, awaiting the human's merge confirmation. **Signal relied on:** `ci / native (pull_request)` — aggregate `success`, 8m27s, run 360. Not re-run locally (no Go/Node toolchain in the landing session; CI is the repo's required check and it already exercised the full gate: SPA lint/format/test/build, `go build -tags ui`, `go test -tags ui ./...`, and the untagged golangci-lint 2.12.2 pass). **One fix pushed while landing** (`faabf8a`, see the comment below): run 358 failed on a single staticcheck QF1001 at `internal/seeder/seeder_test.go:655` — the check the PR body flagged as unverifiable in the authoring environment. Rewritten as the equivalent disjunction; run 360 is green. **Conventions:** Conventional Commits title ✓ · `Closes #261` present and resolving ✓ · no textual conflict against `main` (`git merge-tree` clean). **Diff scope:** 38 files, +4311/−95, mapping onto issue #261's acceptance criteria with no drive-by changes. Spot-checked and sound: repo names are `[A-Za-z0-9_-]` and globally UNIQUE, so `imports/<name>` and its `<name>.commit` sidecar structurally cannot collide or traverse; `materializeImports` sits before `AddWorktree` so a target-side failure refuses the spawn with nothing claimed; `refreshImports` runs on the `context.WithoutCancel` context, so a dropped HTTP connection cannot kill an extraction midway; the `UpToDate` semantics change is handled correctly by its one consumer (`internal/httpapi/chat.go:464`), and `gitx` guarantees `NewHead == OldHead` on that path so `renderDigest`'s collapsed branch fires exactly when intended. **CONCERNS (non-blocking, 1):** `internal/gitx/materialize.go:57-65` — `MaterializeSnapshot` clears the destination *before* extracting, so a failure **during** extraction (timeout on `git archive`, disk full, I/O error) leaves a partial tree while the `<name>.commit` sidecar still names the old commit. `/pull-base` then reports `refresh failed — <reason>`, and both `internal/pull/pull.go` ("the snapshot on disk is still the one the run was spawned with") and ADR-0063 ("keeps its still-valid snapshot") state the snapshot is intact — which holds for fetch/resolve failures, the ordering the design deliberately buys, but not for this window. The agent would be told it still holds the old snapshot while actually holding a truncated one. Test coverage matches that boundary: `TestMaterializeSnapshot_fetchFailureLeavesSnapshotIntact` pins the pre-clear failure; there is no post-clear equivalent. Narrow window and not a merge blocker, but worth a follow-up — either extract to a sibling directory and swap contents in (keeping the inode), or downgrade the "intact" claim in both the doc comment and the digest wording.

dominik.polakovics commented 2026-08-03 01:27:47 +02:00

Author

Owner

Copy link

This was generated by AI while landing a PR.

Lint fix pushed to afk/261 while landing: faabf8a — chore(seeder): apply De Morgan's law in the ordering assertion (QF1001).

Run 358 (ci / native) failed after 9m7s on exactly one issue:

internal/seeder/seeder_test.go:655:5: QF1001: could apply De Morgan's law (staticcheck)
	if !(bodyIdx < secretsIdx && secretsIdx < importsIdx && importsIdx < skillsIdx) {
	   ^
1 issues:
* staticcheck: 1

Every other step in that run was green — SPA lint/format/test/build (67 test files), go build -tags ui, and go test -tags ui ./... across every package. This was the check the PR body deferred ("golangci-lint was not available in this environment").

The one-line change, behaviour-identical:

-	if !(bodyIdx < secretsIdx && secretsIdx < importsIdx && importsIdx < skillsIdx) {
+	if bodyIdx >= secretsIdx || secretsIdx >= importsIdx || importsIdx >= skillsIdx {

Run 360 is green (8m27s). Ordinary push to the head branch — no force-push, no rewrite.

Note on labctl pr logs 264: it could not fetch these logs, reporting "forge log route did not answer the shape lab's log adapter expects" and advising a local repro. The forge actually returns 200 with the full log on attempt/1 and 500 on attempt/2 — labctl is requesting the wrong attempt, not hitting a response-shape mismatch, and its message misdiagnoses the cause. Compare 84f8fc5, which recorded the same attempt/2-has-no-logs behaviour. Worth its own issue.

> *This was generated by AI while landing a PR.* **Lint fix pushed to `afk/261` while landing:** `faabf8a` — `chore(seeder): apply De Morgan's law in the ordering assertion (QF1001)`. Run 358 (`ci / native`) failed after 9m7s on exactly one issue: ``` internal/seeder/seeder_test.go:655:5: QF1001: could apply De Morgan's law (staticcheck) if !(bodyIdx < secretsIdx && secretsIdx < importsIdx && importsIdx < skillsIdx) { ^ 1 issues: * staticcheck: 1 ``` Every other step in that run was green — SPA lint/format/test/build (67 test files), `go build -tags ui`, and `go test -tags ui ./...` across every package. This was the check the PR body deferred ("golangci-lint was not available in this environment"). The one-line change, behaviour-identical: ```go - if !(bodyIdx < secretsIdx && secretsIdx < importsIdx && importsIdx < skillsIdx) { + if bodyIdx >= secretsIdx || secretsIdx >= importsIdx || importsIdx >= skillsIdx { ``` Run 360 is green (8m27s). Ordinary push to the head branch — no force-push, no rewrite. **Note on `labctl pr logs 264`:** it could not fetch these logs, reporting *"forge log route did not answer the shape lab's log adapter expects"* and advising a local repro. The forge actually returns **200 with the full log on `attempt/1`** and **500 on `attempt/2`** — labctl is requesting the wrong attempt, not hitting a response-shape mismatch, and its message misdiagnoses the cause. Compare `84f8fc5`, which recorded the same attempt/2-has-no-logs behaviour. Worth its own issue.

dominik.polakovics referenced this pull request 2026-08-03 01:30:42 +02:00

MaterializeSnapshot: a failure during extraction leaves a partial snapshot while the sidecar and the digest both claim it is intact #265

dominik.polakovics referenced this pull request 2026-08-03 01:31:07 +02:00

labctl pr logs requests the wrong run attempt and misreports the cause as a log-adapter shape mismatch #266

dominik.polakovics added 1 commit 2026-08-03 01:32:34 +02:00

Merge branch 'main' into afk/261 623c88792a

dominik.polakovics merged commit 8bf9c864cf into main 2026-08-03 01:41:30 +02:00

dominik.polakovics deleted branch afk/261 2026-08-03 01:41:30 +02:00

dominik.polakovics referenced this pull request from a commit 2026-08-03 01:41:31 +02:00

Merge pull request 'feat(imports): consumer-declared read-only repo snapshots mounted into every spawn' (#264) from afk/261 into main

dominik.polakovics referenced this pull request 2026-08-03 21:07:57 +02:00

PRD: read-only imports — repos grant read-only code access to other repos #260

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something is broken

  

chore

  

enhancement

New feature or improvement

  

needs-info

Waiting on reporter for more information

  

needs-triage

Maintainer needs to evaluate this issue

  

ready-for-agent

Fully specified, ready for an AFK agent

  

ready-for-human

Requires human implementation

  

wontfix

Will not be actioned

No labels

bug

chore

enhancement

needs-info

needs-triage

ready-for-agent

ready-for-human

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Cloonar/coding-lab!264

No description provided.