crueber/walhub

Watch

Fork 0

Phase C: Pull requests (internal/pulls) #5

New issue

Closed

opened 2026-09-03 23:44:28 +00:00 by crueber · 4 comments

crueber commented

2026-09-03 23:44:28 +00:00

Owner

Phase C: Pull requests — `internal/pulls`

Spec: docs/features/03_pull_requests.md (normative) + P1–P9 in docs/features/README.md.
Rollout: Wave C in docs/features/09_rollout.md §3 (parallel with review; needs issues numbering + identity). Depends on 01, 02 (shared numbering via P2).

Recommended implementation (verified against the doc)

New package internal/pulls: RouteProvider (Seam 1, api.Lanes both lanes) + task kinds pull-merge / pull-fork / pull-mergeable (Seam 5) + event sink pulls (Seam 4, cursor repos/<o>/<r>/events/cursors/pulls.json). Core (store/wal/git) never learns the word "pull".
Objects: issues/<num>/thread.json with kind:"pr" (P2/P3 reuse; no PR fields leak into it) + events/ (P3) + pulls/<num>/pr.json sidecar (base/head ref+sha, fork, merge outcome — CAS'd) + pulls/<num>/mergeable.json stamped derived cache (base_ref, base_sha, head_sha) (CAS overwrite; 412 loser re-runs and converges). Shared issues/index.json filtered kind:"pr"; meta/forks.json + per-fork fork.json.
Open (03 §3): resolve base+head → P2 number → P3 thread + pr.json Create → P4 index → P8 fan-out (pull_opened) → publish refs/pull/<num>/head server-side through the WAL publish path ONLY if head commit reachable (rev-list … --not --all | cat-file --batch-check empty; else 422). refs/pull/** is server-managed via built-in protect default; client pushes rejected.
Mergeability (03 §4): merge-base --is-ancestor both directions + merge-tree --write-tree --name-only trial + rev-list --count head..base; git via bounded per-repo pool; sink enqueues dirty PRs → pull-mergeable single-flight recompute; thread fetch serves unknown + enqueues on stamp mismatch.
Merge task (03 §5): POST …/merge (maintain+), narrated P7 task, (repo, "pull-merge") single-flight; re-verify stamp → strategy argv (merge-tree/commit-tree with principal author + server committer; replay --onto for rebase) → explicit policy.json evaluation (protect + required-checks, bypass lists apply) → REF_UPDATE WAL publish (CAS arbitrates; never force-publish) → P3/P4 commit (merged event, pr.json, index) → ApplyClosingReferences (02 seam) → optional head delete via same publish path.
Forks (03 §7): pull-fork task reusing import --direct already-on-bucket mode (no pack copy; fresh manifest/refs); own policy/access/collab families; GC rule — pack removal consults fork-network manifests first (TryLock-or-defer preserved).

Acceptance criteria

Endpoints in 03 §8 (pulls CRUD, diff as text/plain unified patch, commits, merge/update-branch/delete-head, top-level forks); SSE pull actions + task envelope.
Merge gate consults 04 required-reviews + 05 required-checks before publish; narration names the shortfall (law 7).
Force-push to head allowed (policy-checked); head_force_pushed event; base only ever fast-forwards.
make cover ≥ 95% on internal/pulls; -race clean; e2e with real git per verification ladder step 6.

# Phase C: Pull requests — `internal/pulls` **Spec:** `docs/features/03_pull_requests.md` (normative) + P1–P9 in `docs/features/README.md`. **Rollout:** Wave C in `docs/features/09_rollout.md` §3 (parallel with review; needs issues numbering + identity). Depends on 01, 02 (shared numbering via P2). ## Recommended implementation (verified against the doc) - **New package `internal/pulls`**: `RouteProvider` (Seam 1, `api.Lanes` both lanes) + task kinds `pull-merge` / `pull-fork` / `pull-mergeable` (Seam 5) + event sink `pulls` (Seam 4, cursor `repos/<o>/<r>/events/cursors/pulls.json`). Core (`store`/`wal`/`git`) never learns the word "pull". - **Objects:** `issues/<num>/thread.json` with `kind:"pr"` (P2/P3 reuse; no PR fields leak into it) + `events/` (P3) + `pulls/<num>/pr.json` sidecar (base/head ref+sha, fork, merge outcome — CAS'd) + `pulls/<num>/mergeable.json` stamped derived cache `(base_ref, base_sha, head_sha)` (CAS overwrite; 412 loser re-runs and converges). Shared `issues/index.json` filtered `kind:"pr"`; `meta/forks.json` + per-fork `fork.json`. - **Open (03 §3):** resolve base+head → P2 number → P3 thread + `pr.json` Create → P4 index → P8 fan-out (`pull_opened`) → publish `refs/pull/<num>/head` server-side through the WAL publish path ONLY if head commit reachable (`rev-list … --not --all | cat-file --batch-check` empty; else 422). `refs/pull/**` is server-managed via built-in `protect` default; client pushes rejected. - **Mergeability (03 §4):** `merge-base --is-ancestor` both directions + `merge-tree --write-tree --name-only` trial + `rev-list --count head..base`; git via bounded per-repo pool; sink enqueues dirty PRs → `pull-mergeable` single-flight recompute; thread fetch serves `unknown` + enqueues on stamp mismatch. - **Merge task (03 §5):** `POST …/merge` (maintain+), narrated P7 task, `(repo, "pull-merge")` single-flight; re-verify stamp → strategy argv (`merge-tree`/`commit-tree` with principal author + server committer; `replay --onto` for rebase) → explicit `policy.json` evaluation (protect + required-checks, bypass lists apply) → REF_UPDATE WAL publish (CAS arbitrates; never force-publish) → P3/P4 commit (`merged` event, `pr.json`, index) → `ApplyClosingReferences` (02 seam) → optional head delete via same publish path. - **Forks (03 §7):** `pull-fork` task reusing `import --direct` already-on-bucket mode (no pack copy; fresh manifest/refs); own policy/access/collab families; GC rule — pack removal consults fork-network manifests first (TryLock-or-defer preserved). ## Acceptance criteria - [ ] Endpoints in 03 §8 (pulls CRUD, diff as `text/plain` unified patch, commits, merge/update-branch/delete-head, top-level forks); SSE `pull` actions + task envelope. - [ ] Merge gate consults 04 required-reviews + 05 required-checks before publish; narration names the shortfall (law 7). - [ ] Force-push to head allowed (policy-checked); `head_force_pushed` event; base only ever fast-forwards. - [ ] `make cover` ≥ 95% on `internal/pulls`; `-race` clean; e2e with real git per verification ladder step 6.

crueber commented

2026-09-04 01:47:13 +00:00

Author

Owner

Wave C1 starting: building internal/pulls on top of origin/main (99742ba). Note: worktree has uncommitted user changes on feat/issues, so I created the feat/pulls pointer without checking out to avoid touching dirty files; PR-owned files only. Will report at the end.

Wave C1 starting: building internal/pulls on top of origin/main (99742ba). Note: worktree has uncommitted user changes on feat/issues, so I created the feat/pulls pointer without checking out to avoid touching dirty files; PR-owned files only. Will report at the end.

crueber referenced this issue

2026-09-04 03:04:11 +00:00

Wave C: pull requests (docs/features/03) #9

crueber commented

2026-09-04 03:04:16 +00:00

Author

Owner

Wave C1 ready for review: #9 (branch feat/pulls → main, one commit bdebd02 on top of origin/main 99742ba). Summary: internal/pulls (threads/sidecars/mergeable cache + pulls sink + pull-merge/pull-mergeable/pull-fork/pull-update-branch tasks), refs/pull/N/head reachable-only publish, built-in refs/pull/** push refusal, endpoints/SSE/SolidJS UI/SDK, E4 evidence. 99.5% statements, -race clean, full suite + e2e green, live-verified incl. CDP browser drive. Deviations recorded in docs/features/03 + docs/go/14 + docs/go/06. Not merged — awaiting review.

Wave C1 ready for review: https://git.packden.us/crueber/walhub/pulls/9 (branch feat/pulls → main, one commit bdebd02 on top of origin/main 99742ba). Summary: internal/pulls (threads/sidecars/mergeable cache + pulls sink + pull-merge/pull-mergeable/pull-fork/pull-update-branch tasks), refs/pull/N/head reachable-only publish, built-in refs/pull/** push refusal, endpoints/SSE/SolidJS UI/SDK, E4 evidence. 99.5% statements, -race clean, full suite + e2e green, live-verified incl. CDP browser drive. Deviations recorded in docs/features/03 + docs/go/14 + docs/go/06. Not merged — awaiting review.

crueber commented

2026-09-04 03:17:28 +00:00

Author

Owner

PR #9 review (Wave C1 pulls, bdebd02 + fix c025143). Method: diff origin/main...origin/feat/pulls, scratch worktrees only — main worktree untouched, nothing staged.

VERIFIED CLEAN:

Deps: go.mod unchanged (chi/toml/x-net/x-crypto only); web adds no npm deps (solid-js+router, Tailwind dark: variants present in Pull.jsx/Pulls.jsx). SDK pulls.js dependency-free, both lanes via lane rewrite.
Seams: internal/pulls imports store/identity/policy/server-auth only — never wal/server upward beyond seam interfaces; core never imports pulls. refs/pull/** refusal is a pure predicate (internal/git/managed.go: IsManagedRef) enforced in the shared pushPipeline (internal/server/bind_ssh.go), so HTTP (smart.go:439) and SSH (:181) both refuse — managed-only push gets unpack-ok + per-ref ng without ingest; mixed pushes filter before ingest. Tests TestPushPipelineManagedRefs/TestIsManagedRefBoundary pass.
Open: reachable-only head publish (service.go:335 Reachable → 422 'head commit not reachable'), 409 on duplicate open base+head pair, idempotent CreateRef + named GET repair path.
Merge: task single-flight (repo,pull-merge) + UpdateRef(old=baseLive) CAS, never force (merge.go:215); CAS loss re-plans once then fails. Policy evaluated explicitly (checkProtectedRef: protect + bypass + required-checks pre-scan failing closed only when a rule carries the gate). ApplyClosingReferences called with (sha,title,body) on merge (merge.go:271). Dirty/up_to_date refused narrated.
Force-push: allowed on head, head_force_pushed event + mergeable recompute; base only via non-ff CAS (always fails, never rewrites).
P6: open/write, merge+deleteHead/maintain, fork+updateBranch/write, reads/requireRead. Wire: plain-text errors, []-not-null, RFC3339, ETag<head-sha>+SWR on GET, no-store on task starts, PUT strict (unknown keys 400), both lanes in Handle + forks twin. SSE event 'pull' with opened/closed/reopened/merged/head_force_pushed; sink 'pulls' wired in serve.go:153 with per-sink cursor.
Deps/argv deviation recorded: two-arg merge-tree + --skip/--max-count in 03 Wave C1 notes; fork ForkExecutor=nil + GC enforcement deferred recorded; E4 evidence (3+1 flat, merge 12+5, 16→1 single-flight) plausible — harness counts real Service ops over memory store, git argv proven against stock git 2.53.
Tests: gofmt clean, go vet clean, go test -race ./internal/pulls/... ok, coverage 99.4% (gate 95%), node --test sdk-pulls 3/3 pass. internal/git ok. internal/server full suite shows 9 web-asset failures identical on pristine branch (missing built web/dist in scratch worktree — environmental, unrelated).

FIXED THIS ROUND (pushed c025143 to origin/feat/pulls):

BLOCKER fixed: rebase strategy was dead — git replay with pure-SHA range is a silent no-op (exit 0, empty stdout) on git 2.53, so Replay() always failed validation; gitexec baked the false premise 'stock git has no replay'. Fix: temp branch refs/heads/walhub-tmp-replay-- + --ref-action=print (print mode never moves serving refs; default update mode would — verified live it moved refs/heads/topic), parse update line, delete temp ref always; committer identity threaded explicitly (runner strips env; replay mints commits). Callers: merge.go passes server committer; FakeGit/gitexec/cover scripts updated. Doc §5 argv + rationale updated same change. Verified live: tip lands on base, authorship preserved, committer walhub, temp ref gone, user refs untouched.

MINOR (non-blocking, noted): IsAncestor maps every non-zero exit to false (usage errors conflated with not-ancestor; fail-safe — trial merge errors next); update-branch publishes to head without a policy eval (doc §8 doesn't require one; final base publish is still gated).

RECOMMENDATION: ready to merge.

PR #9 review (Wave C1 pulls, bdebd02 + fix c025143). Method: diff origin/main...origin/feat/pulls, scratch worktrees only — main worktree untouched, nothing staged. VERIFIED CLEAN: - Deps: go.mod unchanged (chi/toml/x-net/x-crypto only); web adds no npm deps (solid-js+router, Tailwind dark: variants present in Pull.jsx/Pulls.jsx). SDK pulls.js dependency-free, both lanes via lane rewrite. - Seams: internal/pulls imports store/identity/policy/server-auth only — never wal/server upward beyond seam interfaces; core never imports pulls. refs/pull/** refusal is a pure predicate (internal/git/managed.go: IsManagedRef) enforced in the shared pushPipeline (internal/server/bind_ssh.go), so HTTP (smart.go:439) and SSH (:181) both refuse — managed-only push gets unpack-ok + per-ref ng without ingest; mixed pushes filter before ingest. Tests TestPushPipelineManagedRefs/TestIsManagedRefBoundary pass. - Open: reachable-only head publish (service.go:335 Reachable → 422 'head commit not reachable'), 409 on duplicate open base+head pair, idempotent CreateRef + named GET repair path. - Merge: task single-flight (repo,pull-merge) + UpdateRef(old=baseLive) CAS, never force (merge.go:215); CAS loss re-plans once then fails. Policy evaluated explicitly (checkProtectedRef: protect + bypass + required-checks pre-scan failing closed only when a rule carries the gate). ApplyClosingReferences called with (sha,title,body) on merge (merge.go:271). Dirty/up_to_date refused narrated. - Force-push: allowed on head, head_force_pushed event + mergeable recompute; base only via non-ff CAS (always fails, never rewrites). - P6: open/write, merge+deleteHead/maintain, fork+updateBranch/write, reads/requireRead. Wire: plain-text errors, []-not-null, RFC3339, ETag<head-sha>+SWR on GET, no-store on task starts, PUT strict (unknown keys 400), both lanes in Handle + forks twin. SSE event 'pull' with opened/closed/reopened/merged/head_force_pushed; sink 'pulls' wired in serve.go:153 with per-sink cursor. - Deps/argv deviation recorded: two-arg merge-tree + --skip/--max-count in 03 Wave C1 notes; fork ForkExecutor=nil + GC enforcement deferred recorded; E4 evidence (3+1 flat, merge 12+5, 16→1 single-flight) plausible — harness counts real Service ops over memory store, git argv proven against stock git 2.53. - Tests: gofmt clean, go vet clean, go test -race ./internal/pulls/... ok, coverage 99.4% (gate 95%), node --test sdk-pulls 3/3 pass. internal/git ok. internal/server full suite shows 9 web-asset failures identical on pristine branch (missing built web/dist in scratch worktree — environmental, unrelated). FIXED THIS ROUND (pushed c025143 to origin/feat/pulls): - BLOCKER fixed: rebase strategy was dead — git replay with pure-SHA range is a silent no-op (exit 0, empty stdout) on git 2.53, so Replay() always failed validation; gitexec baked the false premise 'stock git has no replay'. Fix: temp branch refs/heads/walhub-tmp-replay-<pid>-<nanos> + --ref-action=print (print mode never moves serving refs; default update mode would — verified live it moved refs/heads/topic), parse update line, delete temp ref always; committer identity threaded explicitly (runner strips env; replay mints commits). Callers: merge.go passes server committer; FakeGit/gitexec/cover scripts updated. Doc §5 argv + rationale updated same change. Verified live: tip lands on base, authorship preserved, committer walhub, temp ref gone, user refs untouched. MINOR (non-blocking, noted): IsAncestor maps every non-zero exit to false (usage errors conflated with not-ancestor; fail-safe — trial merge errors next); update-branch publishes to head without a policy eval (doc §8 doesn't require one; final base publish is still gated). RECOMMENDATION: ready to merge.

crueber commented

2026-09-04 03:17:57 +00:00

Author

Owner

Wave C1 complete: PR #9 reviewed (load-bearing rebase/Replay bug fixed in c025143), merged as 536372d. internal/pulls 99.4% coverage, -race clean. Closing.

Wave C1 complete: PR #9 reviewed (load-bearing rebase/Replay bug fixed in c025143), merged as 536372d. internal/pulls 99.4% coverage, -race clean. Closing.

crueber closed this issue

2026-09-04 03:17:57 +00:00

crueber referenced this issue

2026-09-05 06:38:02 +00:00

[feature] [plan] Should be able to paste an image in to the comments and description of issues. #120

crueber added this to the v1 milestone

2026-09-10 22:20:50 +00:00

crueber added the

pull requests

label

2026-09-10 22:22:40 +00:00