Phase B: Issues (internal/issues) #4

New issue

Closed

opened 2026-09-03 23:44:26 +00:00 by crueber · 5 comments

crueber commented

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

Owner

Phase B: Issues — `internal/issues`

Spec: docs/features/02_issues.md (normative) + P1–P9 in docs/features/README.md.
Rollout: Wave B in docs/features/09_rollout.md §3 (parallel with checks + notify core trays). Depends on 01 (roles, access.json). Proves the thread pattern end to end.

Recommended implementation (verified against the doc)

New package internal/issues: one api.RouteProvider (Seam 1, via api.Lanes — every route on both /{o}/{r}/api/… and /api-browser/…) + one task kind issue-index-compact (Seam 5). No other task kinds — mutations are synchronous (P8).
Objects (repos/<o>/<r>/ prefix; <num:06x> hex keys, decimal on the wire):
- meta/next_num — P2 shared counter with PRs (CAS loop; human-rate, contention a non-issue).
- issues/<num>/thread.json (CAS'd P3 header: num/kind/title/state/state_reason/labels/assignees/milestone/participants/next_event_seq/comment_count/reaction_summary/version) + issues/<num>/events/<seq:012x>.json (immutable, Create only).
- issues/index.json (P4 CAS'd card index for BOTH kinds; 02 owns schema; joins frozen overwritable list) — header-then-index order, repair-on-next-mutation, LIST fallback.
- meta/labels.json (CAS'd; names immutable — rename = delete + create with compensating events) + meta/milestones/<id:06x>.json + allocator meta/milestones/index.json.
Write discipline: P3 two-step everywhere (CAS header to reserve seq → Create event; gaps allowed). No cross-feature/cross-thread locks — CAS loops only (13 §3/§5).
References: #N parsed at WRITE time in the commenting handler (fence/code-span skipper, cap 100, dedup by (source seq, target)); best-effort P3 two-step on target thread; missing target silently skipped.
Closing keywords (fixes #N): parsed at PR MERGE time only — merge task calls 02-owned seam ApplyClosingReferences(repo, pr_num, merged_sha, texts…); never on the push path.
Reactions: events + denormalized reaction_summary in header (same CAS); emoji allowlist; unique per (principal, target, content).
Notifications contract (02 §10): synchronous fan-out post-CAS — assigned, mentioned (@-parse), subscribed (from participants[]); 06 must not scan the event log.

Acceptance criteria

Endpoints in 02 §7 with P6 auth gates (create/comment at read; labels/milestones/moderation at triage); plain-text errors; paginated LIST routes state page size (P5).
Milestone counters denormalized, progress derived on read; DELETE blocked at 409 while open issues reference it.
Compaction task triggers at ~256 KiB (sampled + maintainer pass), (repo, kind) single-flight, SSE-attachable.
make cover ≥ 95% on internal/issues; -race clean; table-driven httptest per handler.
UI/SDK per 02 §11 (or deferred to Wave D with a note — 08 owns the full inventory).

# Phase B: Issues — `internal/issues` **Spec:** `docs/features/02_issues.md` (normative) + P1–P9 in `docs/features/README.md`. **Rollout:** Wave B in `docs/features/09_rollout.md` §3 (parallel with checks + notify core trays). Depends on 01 (roles, access.json). Proves the thread pattern end to end. ## Recommended implementation (verified against the doc) - **New package `internal/issues`**: one `api.RouteProvider` (Seam 1, via `api.Lanes` — every route on both `/{o}/{r}/api/…` and `/api-browser/…`) + one task kind `issue-index-compact` (Seam 5). No other task kinds — mutations are synchronous (P8). - **Objects** (`repos/<o>/<r>/` prefix; `<num:06x>` hex keys, decimal on the wire): - `meta/next_num` — P2 shared counter with PRs (CAS loop; human-rate, contention a non-issue). - `issues/<num>/thread.json` (CAS'd P3 header: num/kind/title/state/state_reason/labels/assignees/milestone/participants/next_event_seq/comment_count/reaction_summary/version) + `issues/<num>/events/<seq:012x>.json` (immutable, `Create` only). - `issues/index.json` (P4 CAS'd card index for BOTH kinds; 02 owns schema; joins frozen overwritable list) — header-then-index order, repair-on-next-mutation, LIST fallback. - `meta/labels.json` (CAS'd; names immutable — rename = delete + create with compensating events) + `meta/milestones/<id:06x>.json` + allocator `meta/milestones/index.json`. - **Write discipline:** P3 two-step everywhere (CAS header to reserve seq → `Create` event; gaps allowed). No cross-feature/cross-thread locks — CAS loops only (13 §3/§5). - **References:** `#N` parsed at WRITE time in the commenting handler (fence/code-span skipper, cap 100, dedup by (source seq, target)); best-effort P3 two-step on target thread; missing target silently skipped. - **Closing keywords** (`fixes #N`): parsed at PR MERGE time only — merge task calls 02-owned seam `ApplyClosingReferences(repo, pr_num, merged_sha, texts…)`; never on the push path. - **Reactions:** events + denormalized `reaction_summary` in header (same CAS); emoji allowlist; unique per (principal, target, content). - **Notifications contract (02 §10):** synchronous fan-out post-CAS — `assigned`, `mentioned` (@-parse), `subscribed` (from `participants[]`); 06 must not scan the event log. ## Acceptance criteria - [ ] Endpoints in 02 §7 with P6 auth gates (create/comment at read; labels/milestones/moderation at triage); plain-text errors; paginated LIST routes state page size (P5). - [ ] Milestone counters denormalized, progress derived on read; DELETE blocked at 409 while open issues reference it. - [ ] Compaction task triggers at ~256 KiB (sampled + maintainer pass), `(repo, kind)` single-flight, SSE-attachable. - [ ] `make cover` ≥ 95% on `internal/issues`; `-race` clean; table-driven httptest per handler. - [ ] UI/SDK per 02 §11 (or deferred to Wave D with a note — 08 owns the full inventory).

crueber commented

2026-09-04 01:16:42 +00:00

Author

Owner

Wave B started: new internal/issues package (P2 numbering, P3 threads/events, labels, milestones, refs, reactions, index+compact, notify seam) on both API lanes, plus SDK/UI/EVIDENCE.

crueber commented

2026-09-04 01:41:39 +00:00

Author

Owner

Wave B ready for review: PR #8 (#8) — internal/issues end to end per docs/features/02. Race-clean, 96.3% cover, live-stack verified. Not merging.

Wave B ready for review: PR #8 (https://git.packden.us/crueber/walhub/pulls/8) — internal/issues end to end per docs/features/02. Race-clean, 96.3% cover, live-stack verified. Not merging.

crueber commented

2026-09-04 01:45:32 +00:00

Author

Owner

PR #8 review (feat/issues, Wave B) — round 1 findings, all fixed in review commit:

Must-fix, fixed:

internal/issues/service.go writeRefs — cross-repo ref from an opened body recorded kind:comment + event_seq:-1. Per 02 Wave B notes only comment bodies source kind:comment; opened bodies source kind:thread. Fixed (gate on srcSeq>=0).
internal/issues/service.go ApplyClosingReferences — merge-close wrote no subscribed notification (§10: state changes fan out to participants). Added emitSubscribed (nil-safe no-op until 06).
internal/issues/service.go AddReaction + http.go addReaction — 201 body re-read the newest event instead of returning the committed one (concurrent interleave misattributes). AddReaction now returns (*Thread,*Event,bool,error); handler uses the committed event; test call sites updated + new assertion on the 201 body.
web/src/pages/Issue.jsx — reaction counts never rendered: summary keys are %06x hex ('000003'), UI looked up String(seq) ('3'). Fixed with seqKey helper.
web/src/pages/Issue.jsx loadOlder — fetched the older page then discarded it (void page + invalidate reloads newest-50); Older button was a no-op. Fixed: accumulate windows in a local signal, reset on navigation.

Notes (no code change):

Reaction dup-check is outside the CAS (best-effort; sequential double-submit handled, true-concurrent duplicate double-add could drift the summary; log stays truth). Recorded in 02 Wave B notes.
Event windows are newest-first arrays; §2/P3 'newest-last' names the pagination direction, not array order. Clarified in 02 Wave B notes; code+tests+UI agree.
scanHeaders LISTs the full issues/ prefix (event keys included) then filters — bounded by scanCap=2000, P5-allowed, E3-measured. No change.
ApplyClosingReferences takes mergedSHA but stores no SHA (no §1.2 field carries it) — cosmetic, seam signature kept.
Dependency budget clean (no go.mod/npm changes); seam direction fine (issues→store/git/identity/server-auth, no upward imports into core); CAS discipline clean (bounded loops, no lock objects); P5 page sizes stated (list n≤100/400-on-abuse, events ≤200, scanCap 2000); auth gates per P6; wire shape clean (plain-text errors, []-not-null, RFC3339, ETag v/SWR on GET-by-num, no-store lists); both lanes via Handle segs[2]; dark: classes on all new pages; E3 numbers rechecked against code (2 GET list, 62 GET/1 LIST thread, 302 fallback at 300) — plausible.

Tests: gofmt clean, go vet clean, go test -race ./internal/issues/... ok, coverage 96.3% (≥95% gate), go test -race ./internal/server/... ok, node --test sdk-issues 2/2 pass, Issue.jsx esbuild-compiles.

PR #8 review (feat/issues, Wave B) — round 1 findings, all fixed in review commit: Must-fix, fixed: 1. internal/issues/service.go writeRefs — cross-repo ref from an opened body recorded kind:comment + event_seq:-1. Per 02 Wave B notes only comment bodies source kind:comment; opened bodies source kind:thread. Fixed (gate on srcSeq>=0). 2. internal/issues/service.go ApplyClosingReferences — merge-close wrote no subscribed notification (§10: state changes fan out to participants). Added emitSubscribed (nil-safe no-op until 06). 3. internal/issues/service.go AddReaction + http.go addReaction — 201 body re-read the newest event instead of returning the committed one (concurrent interleave misattributes). AddReaction now returns (*Thread,*Event,bool,error); handler uses the committed event; test call sites updated + new assertion on the 201 body. 4. web/src/pages/Issue.jsx — reaction counts never rendered: summary keys are %06x hex ('000003'), UI looked up String(seq) ('3'). Fixed with seqKey helper. 5. web/src/pages/Issue.jsx loadOlder — fetched the older page then discarded it (void page + invalidate reloads newest-50); Older button was a no-op. Fixed: accumulate windows in a local signal, reset on navigation. Notes (no code change): - Reaction dup-check is outside the CAS (best-effort; sequential double-submit handled, true-concurrent duplicate double-add could drift the summary; log stays truth). Recorded in 02 Wave B notes. - Event windows are newest-first arrays; §2/P3 'newest-last' names the pagination direction, not array order. Clarified in 02 Wave B notes; code+tests+UI agree. - scanHeaders LISTs the full issues/ prefix (event keys included) then filters — bounded by scanCap=2000, P5-allowed, E3-measured. No change. - ApplyClosingReferences takes mergedSHA but stores no SHA (no §1.2 field carries it) — cosmetic, seam signature kept. - Dependency budget clean (no go.mod/npm changes); seam direction fine (issues→store/git/identity/server-auth, no upward imports into core); CAS discipline clean (bounded loops, no lock objects); P5 page sizes stated (list n≤100/400-on-abuse, events ≤200, scanCap 2000); auth gates per P6; wire shape clean (plain-text errors, []-not-null, RFC3339, ETag v<version>/SWR on GET-by-num, no-store lists); both lanes via Handle segs[2]; dark: classes on all new pages; E3 numbers rechecked against code (2 GET list, 62 GET/1 LIST thread, 302 fallback at 300) — plausible. Tests: gofmt clean, go vet clean, go test -race ./internal/issues/... ok, coverage 96.3% (≥95% gate), go test -race ./internal/server/... ok, node --test sdk-issues 2/2 pass, Issue.jsx esbuild-compiles.

crueber commented

2026-09-04 01:46:07 +00:00

Author

Owner

PR #8 review round 2 (post-fix re-review): all 5 findings verified fixed; no new findings. Labels page uses the 200 {threads_affected} delete report correctly. Final: gofmt clean, vet clean, issues -race ok at 96.3% coverage, server -race ok, sdk js 2/2, Issue.jsx esbuild-compiles. Recommendation: ready to merge (merge itself left to you per instructions).