Link the MergeBox merged SHA to the commit page (Fix #595) #596

Merged
crueber merged 1 commit from fix/issue-595 into main 2026-09-15 21:12:39 +00:00
Owner

Fixes #595. The merge commit is a base child so it never appears in the PR commits tab (base…head by documented design); the MERGE box SHA was dead text. Now links to /:o/:r/commit/ (full SHA href, 12-char text, shared .link rule). Reasons/gate/buttons untouched; timeline rows deliberately stay text-only. Verified: new test 4/4 (3 fail pre-fix), full-minus-smoke 1490/1490, vite+esbuild green, go vet clean.

Fixes #595. The merge commit is a base child so it never appears in the PR commits tab (base…head by documented design); the MERGE box SHA was dead text. Now links to /:o/:r/commit/<sha> (full SHA href, 12-char text, shared .link rule). Reasons/gate/buttons untouched; timeline rows deliberately stay text-only. Verified: new test 4/4 (3 fail pre-fix), full-minus-smoke 1490/1490, vite+esbuild green, go vet clean.
Author
Owner

APPROVED — independent review vs #595 acceptance, verified in /tmp/walhub-595 @61eddf4 (diff origin/main..origin/fix/issue-595, 4 files +72/-2).

Acceptance, all met:

  • Merged line renders short SHA as link: <A class="link font-mono" href={/${props.full}/commit/${sha}}> with full SHA in href, .slice(0,12) text and by {merged_by} credit unchanged (MergeBox.jsx).
  • Route exists: /:owner/:name/commit/:sha nested under /:owner/:name (index.jsx:105). ctx.full is ${params.owner}/${params.name} (Repo.jsx:627), threaded as full={ctx.full} (Pull.jsx:1447) — href resolves to the real commit page.
  • Shared .link rule (ui.css:101, #568); no new CSS, no inline styles (font-mono is a Tailwind utility).
  • Reasons line, tooltip, mergeState, enabled() gate, strategy/buttons byte-identical — diff touches only the router import, a doc comment, the merged-line block, and the one call-site prop.
  • Single consumer: only Pull.jsx renders <MergeBox>; no other call site can produce a //commit/… href. Timeline (textFor/pull-state.js, ThreadTimeline) untouched per scope.
  • Law 12: amendment appended to docs/go/12_web_ui.md. Law 1: only an @solidjs/router import (already-pinned runtime dep); package.json untouched.

Tests (run locally):

  • New merge-sha-link-595.test.js: 4/4 pass on branch; 3 fail with origin/main's MergeBox.jsx swapped in (matches the PR's '3 fail pre-fix' claim — the 4th pin is the Pull.jsx call site, kept during the swap).
  • Full-minus-smoke: 1490/1490 green. The one smoke failure ('built SPA shell is served') fails identically on origin/main's web/ — pre-existing, needs a built/served SPA, unrelated to this change.
  • go vet ./... clean; vite build green (chunk-size warning only, pre-existing).

No defects found — no fix commit made. Out-of-scope items (PR commits tab base…head, text-only timeline rows) correctly untouched.

APPROVED — independent review vs #595 acceptance, verified in /tmp/walhub-595 @61eddf4 (diff origin/main..origin/fix/issue-595, 4 files +72/-2). Acceptance, all met: - Merged line renders short SHA as link: `<A class="link font-mono" href={`/${props.full}/commit/${sha}`}>` with full SHA in href, `.slice(0,12)` text and `by {merged_by}` credit unchanged (MergeBox.jsx). - Route exists: `/:owner/:name/commit/:sha` nested under `/:owner/:name` (index.jsx:105). `ctx.full` is `${params.owner}/${params.name}` (Repo.jsx:627), threaded as `full={ctx.full}` (Pull.jsx:1447) — href resolves to the real commit page. - Shared `.link` rule (ui.css:101, #568); no new CSS, no inline styles (`font-mono` is a Tailwind utility). - Reasons line, tooltip, `mergeState`, `enabled()` gate, strategy/buttons byte-identical — diff touches only the router import, a doc comment, the merged-line block, and the one call-site prop. - Single consumer: only Pull.jsx renders `<MergeBox>`; no other call site can produce a `//commit/…` href. Timeline (`textFor`/pull-state.js, ThreadTimeline) untouched per scope. - Law 12: amendment appended to docs/go/12_web_ui.md. Law 1: only an `@solidjs/router` import (already-pinned runtime dep); package.json untouched. Tests (run locally): - New `merge-sha-link-595.test.js`: 4/4 pass on branch; 3 fail with origin/main's MergeBox.jsx swapped in (matches the PR's '3 fail pre-fix' claim — the 4th pin is the Pull.jsx call site, kept during the swap). - Full-minus-smoke: 1490/1490 green. The one smoke failure ('built SPA shell is served') fails identically on origin/main's web/ — pre-existing, needs a built/served SPA, unrelated to this change. - `go vet ./...` clean; `vite build` green (chunk-size warning only, pre-existing). No defects found — no fix commit made. Out-of-scope items (PR commits tab base…head, text-only timeline rows) correctly untouched.
Sign in to join this conversation.
No description provided.