Create a tag at a given commit from the UI (commit view action), server-side via the WAL ref-update path #253

Closed
opened 2026-09-09 20:25:04 +00:00 by crueber · 3 comments
Owner

What's requested

Users should be able to create a tag pointing at a given commit from the UI — most naturally as an action on the commit detail view (/:owner/:name/commit/:sha), with the tag name (and lightweight vs annotated choice) supplied by the user. Today tags can only arrive via git push.

Current state (code evidence)

  • No server-side tag creation exists. Tags enter the system only through push (receive-pack writes refs/tags/* into a WAL PUSH entry). The releases feature requires the tag to already exist: resolveTag (internal/releases/service.go:29-39) errors unknown revision when refs/tags/<tag> is absent — so a release cannot create the tag, and there is no "new tag" UI anywhere (ReleaseNew.jsx picks from existing tags via the tags ref stream, :16-17).
  • The ref-write machinery exists. WAL EntryKindRefUpdate (internal/store/proto/types.go:47) carries a RefTransaction (updates, push options, atomic flag — types.go:148-152), and RefUpdate supports symbolic targets and peeled shas (NewPeeled, types.go:144) — the peeled field is exactly what an annotated tag needs. publish.go:319 already builds RefUpdate entries.
  • Annotated tag objects are the gap. A lightweight tag is a pure ref move (point refs/tags/<name> at the commit sha — expressible as a RefUpdate today). An annotated tag requires a tag object in the object store (type/tagger/message), which the current WAL entry kinds don't carry — PUSH brings objects+refs together, but there's no server-side "write object then move ref" path for tags.
  • UI surface: the commit view (web/src/pages/Commit.jsx) has mode pills and check details but no per-commit action affordances; the tags list rides the same ref stream the picker uses (repoClient.tags({n:100}), ReleaseNew.jsx:16).

Proposed design

  1. API: POST …/api/tags (repo-scoped, following the repo API route twins pattern) with {name, sha, message?, ref_type: "lightweight"|"annotated"}. AuthWrite-gated (same as push). Server-side:
    • Lightweight: append a WAL EntryKindRefUpdate transaction creating refs/tags/<name> at the given sha (CAS old-oid zero = create, matching the RefUpdate contract, types.go:141).
    • Annotated: requires writing the tag object. Options for planner: (a) extend the WAL with a tag-object entry kind, (b) construct the tag object server-side via the git machinery the maintainer already uses (git binary is available — internal/git wraps it) and publish it as a PUSH-shaped entry, or (c) v1 ships lightweight-only and returns 422 for annotated with a clear message. Recommendation: (c) for the first cut unless (b) falls out cheaply — honest scope, no half-supported tag objects.
  2. Validation: tag name must pass the same refname rules pushes enforce (refs/ prefix rules, no ~^:?*[\, no whitespace — the import parser's ref validation in service.go:152-158 is a reusable precedent); uniqueness (CAS create semantics → 409 on existing tag); sha must resolve in the repo (404 otherwise).
  3. UI: "Create tag" action on the commit view. On Commit.jsx, an action affordance (pill/menu item alongside the diff-mode controls) opening a small inline form: tag name, optional message (message present ⇒ annotated request), submit → POST → navigate or toast. Also worth exposing the same action on Commits.jsx rows if it's cheap — but the commit detail view is the required surface per this request.
  4. Downstream integration: a created tag immediately becomes available to the release composer (ReleaseNew picks from the tags stream — no change needed), the ref picker's tags stream, and bundle strategies' tag filters. Tag-created events should ride the existing ref-event publishing (internal/events) so notifications/webhooks see tag creation the same way they see pushed tags — verify the event producer keys on ref kind, not transport.

Acceptance criteria

  • POST …/api/tags creates a lightweight tag at a given sha; it appears in the tags ref stream, refsList(tags), and resolves via …/api/resolve/<tag>.
  • Creating a tag that already exists → 409; unknown sha → 404; invalid refname → 400 with the refname rule named.
  • Annotated tags: either supported end-to-end (object written, NewPeeled recorded, shows tagger/message in git) or explicitly rejected 422 with a documented message — no silent lightweight downgrade.
  • Commit view shows a "Create tag" affordance (write-capable principals only); the form submits and the new tag is visible without a full reload.
  • Tag creation is WAL-recorded (RefUpdate entry) and survives restart/reconcile like pushed tags.
  • Webhook/notification events fire for API-created tags identically to pushed tags (or the divergence is documented).
  • Auth: write-gated; anonymous → 401; non-writer → 403.
  • Tests: refname validation, CAS conflict, WAL round-trip (tag visible after replay), and a UI-level assertion on the commit-view form flow.
## What's requested Users should be able to create a tag pointing at a given commit from the UI — most naturally as an action on the commit detail view (`/:owner/:name/commit/:sha`), with the tag name (and lightweight vs annotated choice) supplied by the user. Today tags can only arrive via git push. ## Current state (code evidence) - **No server-side tag creation exists.** Tags enter the system only through push (receive-pack writes `refs/tags/*` into a WAL PUSH entry). The releases feature *requires* the tag to already exist: `resolveTag` (`internal/releases/service.go:29-39`) errors `unknown revision` when `refs/tags/<tag>` is absent — so a release cannot create the tag, and there is no "new tag" UI anywhere (`ReleaseNew.jsx` picks from *existing* tags via the tags ref stream, :16-17). - **The ref-write machinery exists.** WAL `EntryKindRefUpdate` (`internal/store/proto/types.go:47`) carries a `RefTransaction` (updates, push options, atomic flag — types.go:148-152), and `RefUpdate` supports symbolic targets and peeled shas (`NewPeeled`, types.go:144) — the peeled field is exactly what an annotated tag needs. `publish.go:319` already builds RefUpdate entries. - **Annotated tag objects are the gap.** A lightweight tag is a pure ref move (point `refs/tags/<name>` at the commit sha — expressible as a RefUpdate today). An **annotated** tag requires a tag *object* in the object store (type/tagger/message), which the current WAL entry kinds don't carry — PUSH brings objects+refs together, but there's no server-side "write object then move ref" path for tags. - **UI surface:** the commit view (`web/src/pages/Commit.jsx`) has mode pills and check details but no per-commit action affordances; the tags list rides the same ref stream the picker uses (`repoClient.tags({n:100})`, `ReleaseNew.jsx:16`). ## Proposed design 1. **API: `POST …/api/tags`** (repo-scoped, following the repo API route twins pattern) with `{name, sha, message?, ref_type: "lightweight"|"annotated"}`. AuthWrite-gated (same as push). Server-side: - Lightweight: append a WAL `EntryKindRefUpdate` transaction creating `refs/tags/<name>` at the given sha (CAS old-oid zero = create, matching the RefUpdate contract, types.go:141). - Annotated: requires writing the tag object. Options for planner: (a) extend the WAL with a tag-object entry kind, (b) construct the tag object server-side via the git machinery the maintainer already uses (git binary is available — `internal/git` wraps it) and publish it as a PUSH-shaped entry, or (c) v1 ships lightweight-only and returns 422 for annotated with a clear message. Recommendation: (c) for the first cut unless (b) falls out cheaply — honest scope, no half-supported tag objects. 2. **Validation:** tag name must pass the same refname rules pushes enforce (`refs/` prefix rules, no `~^:?*[\`, no whitespace — the import parser's ref validation in `service.go:152-158` is a reusable precedent); uniqueness (CAS create semantics → 409 on existing tag); sha must resolve in the repo (404 otherwise). 3. **UI: "Create tag" action on the commit view.** On `Commit.jsx`, an action affordance (pill/menu item alongside the diff-mode controls) opening a small inline form: tag name, optional message (message present ⇒ annotated request), submit → POST → navigate or toast. Also worth exposing the same action on `Commits.jsx` rows if it's cheap — but the commit detail view is the required surface per this request. 4. **Downstream integration:** a created tag immediately becomes available to the release composer (`ReleaseNew` picks from the tags stream — no change needed), the ref picker's tags stream, and bundle strategies' tag filters. Tag-created events should ride the existing ref-event publishing (`internal/events`) so notifications/webhooks see tag creation the same way they see pushed tags — verify the event producer keys on ref kind, not transport. ## Acceptance criteria - [ ] POST `…/api/tags` creates a lightweight tag at a given sha; it appears in the tags ref stream, `refsList(tags)`, and resolves via `…/api/resolve/<tag>`. - [ ] Creating a tag that already exists → 409; unknown sha → 404; invalid refname → 400 with the refname rule named. - [ ] Annotated tags: either supported end-to-end (object written, `NewPeeled` recorded, shows tagger/message in git) or explicitly rejected 422 with a documented message — no silent lightweight downgrade. - [ ] Commit view shows a "Create tag" affordance (write-capable principals only); the form submits and the new tag is visible without a full reload. - [ ] Tag creation is WAL-recorded (RefUpdate entry) and survives restart/reconcile like pushed tags. - [ ] Webhook/notification events fire for API-created tags identically to pushed tags (or the divergence is documented). - [ ] Auth: write-gated; anonymous → 401; non-writer → 403. - [ ] Tests: refname validation, CAS conflict, WAL round-trip (tag visible after replay), and a UI-level assertion on the commit-view form flow.
Author
Owner

PR #262 (branch fix/issue-253) implements this: POST /{o}/{r}/api/tags (both lanes) via new internal/tags, lightweight-only v1 (annotated → 422, follow-up), P6 write + policy create-check, 400/404/409 mapping, Create-tag affordance on Commit.jsx + SDK repo.tagsApi.create. internal/tags at 98.4% coverage, -race clean, end-to-end WAL round-trip incl. restart-replay proven in cmd/walhub test. Ready for review — not merging.

PR #262 (branch fix/issue-253) implements this: POST /{o}/{r}/api/tags (both lanes) via new internal/tags, lightweight-only v1 (annotated → 422, follow-up), P6 write + policy create-check, 400/404/409 mapping, Create-tag affordance on Commit.jsx + SDK repo.tagsApi.create. internal/tags at 98.4% coverage, -race clean, end-to-end WAL round-trip incl. restart-replay proven in cmd/walhub test. Ready for review — not merging.
Author
Owner

Review of PR #262 (fix/issue-253) — verified in scratch worktree /tmp/pr262 at a938607 (+1 fix pushed as 7841369). No browser drive per task note (shared daemon blocks loopback); verdict rests on tests + reasoning.

BUG FOUND + FIXED (pushed to origin/fix/issue-253 as 7841369):

  • web/src/pages/Commit.jsx:142 called reportError(err, 'create-tag') but the data.js import (line 7) lacked the name — a ReferenceError at runtime masking the real server error on every failed create. Fixed by adding reportError to the import (same shape as Access.jsx:6, Issue.jsx:13). Verified: web/src/lib/data.js:47 exports it.

FOLLOW-UP FILED: PR claimed annotated support is 'the tracked follow-up' but no issue existed — filed #263 (annotated tag creation: tag-object write path under frozen proto rules, options (a)/(b) from #253).

VERIFIED OK (file:line):

  • Seams: Handler is server.ExtraRoutes (Handle bool, both lanes api/api-browser, internal/tags/http.go:44-59), chained in cmd/walhub/collab.go:219-221 via chainTags; no new WAL kind (single REF_UPDATE txn); core never imports tags (cmd/walhub/tags.go:108-112). Doc entry in docs/go/14_extensibility.md:523-547. 01_overview canonical tree lists core packages only (issues/pulls/releases absent too) — consistent, no doc-tree violation.
  • Lightweight-only + 422: internal/tags/service.go:103-105, test 'annotated 422' (http_test.go:57). Never a silent downgrade.
  • RefUpdate CAS: cmd/walhub/tags.go:75-104, OldOid zero sized for sha1/sha256 (zeroOid :57-70), PerRef conflict/stale -> 409 (:97-99). Service publishes directly with no pre-check (service.go:121) — no check-then-act, race-safe.
  • Auth: RoleWrite gate (service.go:96; 401 anon / 403 reader in http_test.go:51-52) + explicit EvaluateProtect(principal, refs/tags/, create) (service.go:114,170-193), the pull-merge precedent. Corrupt policy.json refused (fail-closed); note it maps to 500 while pulls maps unparseable to 409 — both refuse, nit only.
  • Validation: ValidateRefName on full refs/tags/ (service.go:142, rule named in error); sha peeled via rev-parse --verify --quiet ^{commit} (git.go:159, trees/blobs rejected) -> 404; existing tag -> 409 via publish verify.
  • Events: eventsFromEntry emits PUSH+REF_UPDATE by ref kind (internal/events/event.go:120-131, refType :48-60) — transport-agnostic, API-created tags identical downstream. No new code needed, as claimed.
  • UI: hidden unless roleAtLeast(write) (Commit.jsx:149); success links to /releases/new (Commit.jsx:182-184).
  • SDK: attachTags in repo() (web/sdk/src/core.js:217), POST p('/tags') riding the lane rewrite; sdk-tags.test.js 2/2.
  • .gitignore: !internal/tags/ correctly re-includes vs the global 'tags' ctags rule — git check-ignore confirms tracked.
  • Imports: stdlib only in new/changed Go files; go.mod unchanged (budget intact).

TESTS (scratch worktree):

  • go test -race ./internal/tags/... : ok; coverage 98.4% (>=95% gate holds).
  • gofmt clean; go vet ./internal/tags/... clean. (go vet ./cmd/walhub/... needs web/dist build artifact — pre-existing env condition; worked around with a stub dist dir in scratch only for the e2e run.)
  • node --test web/test/unit/sdk-tags.test.js: 2/2 pass. Full web/test/unit/*.test.js: 453/460 — 7 failures are pre-existing missing node_modules (marked/solid-js), unrelated files, as the PR notes.
  • TestTagsEndToEndRealWAL (real WAL registry): pass — create -> sync-visible -> 409 -> restart-replay from wiped cache.
  • Re-ran tags + sdk-tags after the reportError fix: green.

Minor nits (non-blocking, not fixed): policy check runs after git resolveCommit (one wasted subprocess on policy-denied requests); no DOM-level test for the Commit form (SDK + e2e cover the path; DOM kept thin per law 11).

MERGE RECOMMENDATION: ready to merge (pending CI). Do NOT merge from this review — left unmerged per instructions.

Review of PR #262 (fix/issue-253) — verified in scratch worktree /tmp/pr262 at a938607 (+1 fix pushed as 7841369). No browser drive per task note (shared daemon blocks loopback); verdict rests on tests + reasoning. BUG FOUND + FIXED (pushed to origin/fix/issue-253 as 7841369): - web/src/pages/Commit.jsx:142 called reportError(err, 'create-tag') but the data.js import (line 7) lacked the name — a ReferenceError at runtime masking the real server error on every failed create. Fixed by adding reportError to the import (same shape as Access.jsx:6, Issue.jsx:13). Verified: web/src/lib/data.js:47 exports it. FOLLOW-UP FILED: PR claimed annotated support is 'the tracked follow-up' but no issue existed — filed #263 (annotated tag creation: tag-object write path under frozen proto rules, options (a)/(b) from #253). VERIFIED OK (file:line): - Seams: Handler is server.ExtraRoutes (Handle bool, both lanes api/api-browser, internal/tags/http.go:44-59), chained in cmd/walhub/collab.go:219-221 via chainTags; no new WAL kind (single REF_UPDATE txn); core never imports tags (cmd/walhub/tags.go:108-112). Doc entry in docs/go/14_extensibility.md:523-547. 01_overview canonical tree lists core packages only (issues/pulls/releases absent too) — consistent, no doc-tree violation. - Lightweight-only + 422: internal/tags/service.go:103-105, test 'annotated 422' (http_test.go:57). Never a silent downgrade. - RefUpdate CAS: cmd/walhub/tags.go:75-104, OldOid zero sized for sha1/sha256 (zeroOid :57-70), PerRef conflict/stale -> 409 (:97-99). Service publishes directly with no pre-check (service.go:121) — no check-then-act, race-safe. - Auth: RoleWrite gate (service.go:96; 401 anon / 403 reader in http_test.go:51-52) + explicit EvaluateProtect(principal, refs/tags/<name>, create) (service.go:114,170-193), the pull-merge precedent. Corrupt policy.json refused (fail-closed); note it maps to 500 while pulls maps unparseable to 409 — both refuse, nit only. - Validation: ValidateRefName on full refs/tags/<name> (service.go:142, rule named in error); sha peeled via rev-parse --verify --quiet <sha>^{commit} (git.go:159, trees/blobs rejected) -> 404; existing tag -> 409 via publish verify. - Events: eventsFromEntry emits PUSH+REF_UPDATE by ref kind (internal/events/event.go:120-131, refType :48-60) — transport-agnostic, API-created tags identical downstream. No new code needed, as claimed. - UI: hidden unless roleAtLeast(write) (Commit.jsx:149); success links to /releases/new (Commit.jsx:182-184). - SDK: attachTags in repo() (web/sdk/src/core.js:217), POST p('/tags') riding the lane rewrite; sdk-tags.test.js 2/2. - .gitignore: !internal/tags/ correctly re-includes vs the global 'tags' ctags rule — git check-ignore confirms tracked. - Imports: stdlib only in new/changed Go files; go.mod unchanged (budget intact). TESTS (scratch worktree): - go test -race ./internal/tags/... : ok; coverage 98.4% (>=95% gate holds). - gofmt clean; go vet ./internal/tags/... clean. (go vet ./cmd/walhub/... needs web/dist build artifact — pre-existing env condition; worked around with a stub dist dir in scratch only for the e2e run.) - node --test web/test/unit/sdk-tags.test.js: 2/2 pass. Full web/test/unit/*.test.js: 453/460 — 7 failures are pre-existing missing node_modules (marked/solid-js), unrelated files, as the PR notes. - TestTagsEndToEndRealWAL (real WAL registry): pass — create -> sync-visible -> 409 -> restart-replay from wiped cache. - Re-ran tags + sdk-tags after the reportError fix: green. Minor nits (non-blocking, not fixed): policy check runs after git resolveCommit (one wasted subprocess on policy-denied requests); no DOM-level test for the Commit form (SDK + e2e cover the path; DOM kept thin per law 11). MERGE RECOMMENDATION: ready to merge (pending CI). Do NOT merge from this review — left unmerged per instructions.
Author
Owner

Fixed by PR #262 incl. review reportError fix (seams, CAS create, auth matrix verified; 98.4% coverage), merged. Closing.

Fixed by PR #262 incl. review reportError fix (seams, CAS create, auth matrix verified; 98.4% coverage), merged. Closing.
crueber added this to the v1 milestone 2026-09-10 22:20:51 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
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
crueber/walhub#253
No description provided.