Skip to content

feat(cli): show apply provenance as a clickable source in status output - #1086

Merged
aparajon merged 3 commits into
mainfrom
armand/status-source
Aug 19, 2026
Merged

feat(cli): show apply provenance as a clickable source in status output#1086
aparajon merged 3 commits into
mainfrom
armand/status-source

Conversation

@aparajon

Copy link
Copy Markdown
Collaborator

Summary

An apply's caller string already records where it came from — the webhook path stores github:<user>@<owner>/<repo>#<pr> — but the CLI rendered it raw, so the PR behind an apply was visible yet not clickable. This makes apply provenance a first-class, clickable field across the status surfaces, derived entirely client-side with no API change.

  • The status list's CALLER column becomes SOURCE: webhook-driven applies show the full clickable PR URL, and CLI-driven applies keep their short caller such as cli:jdoe, so their origin still reads at a glance. The database history table gets the same treatment.
  • The status detail box separates who from where: a short Caller row (github:octocat) plus a clickable Source row (the PR URL), replacing the single raw caller with its unclickable repo#pr tail. An apply without PR provenance keeps one raw Caller row, trailing location included.
  • pkg/caller gains PullRequest(), extracting the repository and PR number from a webhook-shaped caller. It splits at the last @ like Short, so email-shaped users parse correctly. The progress parser also carries the server's pull_request URL through to the detail box and prefers it over the caller-derived one.

CLI examples

Status list with the SOURCE column:

schemabot status -e staging

3 active schema changes

  APPLY ID      DATABASE   ENV      STATE                STARTED         SOURCE
  apply_abc123  orders-db  staging  Running              15 minutes ago  https://github.com/acme/shop/pull/412
  apply_def456  users-db   staging  Waiting for cutover  45 minutes ago  cli:jdoe

Status detail box with Caller and Source separated:

┌──────────────────────────────────────────────────────────┐
│  Apply ID:     apply-multi-a1b2c3d4                      │
│  Environment:  staging                                   │
│  State:        Running                                   │
│  Caller:       github:octocat                            │
│  Source:       https://github.com/acme/shop/pull/412     │
│  Started:      Jan 15 14:22:00 UTC                       │
└──────────────────────────────────────────────────────────┘

🤖 Generated with Claude Code

The status list's CALLER column becomes SOURCE: webhook-driven applies show
the full clickable PR URL, and CLI-driven applies keep their short caller
such as cli:jdoe. The status detail box separates the two — a short Caller
row (who drove it) plus a clickable Source row (the PR it came from) —
instead of one raw caller string with an unclickable repo#pr tail. The
database history table gets the same SOURCE treatment.

pkg/caller gains PullRequest(), extracting the repo and PR number from a
webhook-shaped caller, and the progress parser now carries the server's
pull_request URL through to the detail box, preferring it over the
caller-derived one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 19, 2026 05:51
@aparajon
aparajon marked this pull request as ready for review August 19, 2026 05:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the CLI’s “apply provenance” UX by rendering webhook-driven applies with a clickable PR URL (and separating “who” from “where” in the detail box), derived client-side from existing caller/progress data.

Changes:

  • Add parsing for webhook-shaped caller strings via pkg/caller.PullRequest() and use it to render a clickable PR URL on CLI status surfaces.
  • Update status list/history columns from CALLERSOURCE, and split the detail box into Caller + Source rows when PR provenance exists.
  • Update preview/template outputs and add focused unit tests for the new attribution rendering helpers.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
TEMPLATES.md Updates documented CLI output to reflect the new SOURCE column and Caller/Source rows.
pkg/cmd/internal/templates/progress.go Renames list column header to SOURCE and renders provenance via applySource() / callerAndSourceBoxRows().
pkg/cmd/internal/templates/progress_parse.go Plumbs server-provided pull_request URL into ProgressData.
pkg/cmd/internal/templates/progress_multi.go Uses the shared Caller/Source row rendering for multi-deployment boxes.
pkg/cmd/internal/templates/preview_status.go Updates preview status-list callers to the structured forms used for source rendering.
pkg/cmd/internal/templates/preview_progress_multi.go Updates preview multi-deployment progress data to use the structured GitHub caller form.
pkg/cmd/internal/templates/applysource.go Introduces helper functions to render SOURCE and split Caller/Source in detail boxes.
pkg/cmd/internal/templates/applysource_test.go Adds unit coverage for SOURCE rendering and detail box row behavior.
pkg/caller/caller.go Adds GitHubPrefix and new PullRequest() extractor for webhook-shaped callers.
pkg/caller/caller_test.go Adds tests covering PullRequest() parsing (including email-shaped users).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/cmd/internal/templates/applysource.go
Comment thread pkg/caller/caller.go Outdated
…rmed repos

The server substitutes a bare "<owner>/<repo>#<pr>" caller when an apply
carries PR provenance but no recorded caller, so PullRequest now accepts
that form too instead of leaving it non-clickable. The repository must be
exactly owner/name — one slash with non-empty segments — so a malformed
location never yields a bogus PR link.

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

Copy link
Copy Markdown
Collaborator

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1086, 328a351.

Verdict: 8 findings — 1 blocking (the detail box now drops a CLI caller's host, contradicting its own doc comment), 5 non-blocking, 2 suggestions.

Blocking

1. A CLI-driven apply that also carries repo/PR metadata loses its originating host from the detail box. The Caller row is short-formed whenever any source URL exists (applysource.go:30), including the server-provided one — so schemabot plan --repository acme/shop --pull-request 412 followed by a CLI apply renders Caller: cli:jdoe where the base rendered Caller: cli:jdoe@macbook.local unconditionally (832db3df:progress.go:112). That contradicts this function's own doc ("the raw caller… stays a single Caller row keeping its trailing location, such as the CLI host") and resolveCaller's rationale that the hostname tells an operator "from which machine". The branch should key on whether the caller's own trailing location is a repo#pr, not on sourceURL != "".

Non-blocking

2. The one line that delivers the PR's headline behavior has no test. Deleting PullRequestURL: result.PullRequest (progress_parse.go:152) leaves all five pkg/cmd packages green. TestCallerAndSourceBoxRows hand-passes a URL to the helper and never exercises ParseProgressResponse, so the server→CLI plumbing is unpinned end to end.

3. The bare-form rejection guard survives deletion, and the test named for it exercises a different case. Removing if strings.ContainsAny(caller, ":@") (caller.go:82) is the only one of nine mutations that stays green: caller_test.go:88 is named "bare subject with a slash-and-hash tail is rejected" but passes "jdoe@example.com" — no slash, no hash — which splitRepoPR rejects on its own. Without the guard, jdoe@example.com@acme/repo#42 renders a bogus link.

4. The CALLER→SOURCE sweep missed --failures-only, the view most used during an incident. Five call sites moved to applySource; progress.go:1253 still reads actor := caller.Short(a.Caller), so the same apply shows a clickable URL in schemabot status and a bare github:octocat in schemabot status --failures-only. No test covers statusFailureActor with a github: caller.

5. The list and the detail box can render different PR URLs for the same apply. applySource hardcodes serverURL empty (applysource.go:14) because ActiveApplyResponse/ApplyHistoryResponse carry only Caller — the PullRequest field exists solely on ProgressResponse. So the list is always caller-derived while the detail box prefers the server's, and the PR's own test encodes them disagreeing (…/pull/412 vs …/pull/999).

6. splitRepoPR applies no character-class validation to the segment it newly renders. It checks only non-empty and slash-count (caller.go:96), while the same package's ValidHost restricts hostnames precisely because "stored callers are rendered raw in the CLI detail view — no whitespace, control characters, or terminal escapes". Before this PR the list called caller.Short, which cut that segment off; now a caller containing a newline or ANSI escape prints into the SOURCE column, and resolveCaller returns client-supplied req.Caller verbatim under the shipped allow-all authorizer. Defense-in-depth rather than escalation — an actor who can POST arbitrary applies can do worse — but it is the exact threat this package documents itself as guarding.

General suggestions

7. strconv.Atoi widens the contract splitRepoPR advertises. a/b#+5 and a/b#007 parse to PRs 5 and 7, so a location that literally reads #+5 yields a confident link rather than the promised refusal. Everything else in the probe table — a/b#, a/b#1#2, a/b#0, a/b#-1, a/b# 5, overflow, unicode digits — is correctly rejected.

8. Two presentation gaps. The history and per-deployment preview fixtures were never given PR-shaped callers, so the regenerated TEMPLATES.md documents SOURCE: PR 42 and SOURCE: octocat — and never shows the widest row the change introduces (~155 chars in the deployment view). Nothing emits an OSC 8 hyperlink, so "clickable" rests entirely on terminal auto-detection; at 80 columns those rows wrap mid-URL, and under less -S the tail is clipped outright.

The one thing that could have broken, verified

The obvious risk was the hardcoded https://github.com/ host breaking GitHub Enterprise — the mirror image of #1072, which justified not deriving a link from the configured GitHub host on the grounds that that host serves users' schema repos. I checked and it is not a defect here: GitHubConfig exposes only app-id, private-key, webhook secrets, and check names — no base-url/api-url/host key, no helm value, no env var, and no non-test code sets gh.Client.BaseURL. GHES support is latent plumbing, not a configuration surface. More decisively, the server builds the very same string with the identical hardcoded literal at progress_handlers.go:429, so the client-side fallback is consistent with the server rather than diverging from it.

Verified correct

  • Seven of nine mutations were killed, including dropping the empty-owner/name guard, the extra-slash guard, the n <= 0 guard, and LastIndexIndex (the email-shaped-user promise is genuinely pinned).
  • PullRequest correctly rejects cli:jdoe@acme/repo#42 and jdoe@example.com@acme/repo#42 today — the guard is load-bearing and working, just unpinned.
  • TEMPLATES.md is byte-identical to a fresh scripts/update-templates.sh run against a binary built from the PR head; the generated file was not hand-edited.
  • Changing the preview fixtures does not mask a regression: the old empty-caller list fixtures render identically, and the old Caller: "octocat" multi-deploy fixture still renders one raw Caller row.
  • No assertion was weakened — the −63 lines are the old Caller/PR box rows and the column header, each replaced by a stronger equivalent.
  • Both new exported symbols carry doc comments; tests are table-driven with testify and named subtests, per AGENTS.md.
  • All 34 CI checks pass.

This review was generated by Claude Code (claude-opus-5).

The detail box now splits Caller/Source only when the caller's own
trailing location is the repo#pr; any other caller keeps its full
attribution even when the server supplies a PR URL, and a
server-substituted bare repo#pr renders only the Source row. PR
locations are validated segment-by-segment (repository-shaped
characters, plain positive decimal PR number) so a malformed caller
never yields a bogus link, failure listings render the clickable
source, and status previews use wire-format callers.

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

Copy link
Copy Markdown
Collaborator Author

🤖 Thanks for the thorough review — all fixable findings are addressed in f221e1f.

1 (blocking): Fixed. The detail box now keys the Caller short-form on whether the caller's own trailing location is a repo#pr (caller.PullRequest), not on the presence of a source URL. A CLI caller keeps its host even alongside a server-provided Source row, and a server-substituted bare repo#pr renders only the Source row. Both behaviors are pinned by new subtests.

2: Fixed — the ParseProgressResponse test now asserts Caller and PullRequest map through to ProgressData, so the plumbing line no longer survives deletion.

3: Fixed — the guard test now passes jdoe@example.com@acme/repo#42, which actually exercises the :@ rejection.

4: Fixed — statusFailureActor renders via applySource, and the failures-only test asserts a github: caller shows the clickable URL.

5: Acknowledged, leaving as-is for now. The list endpoints (ActiveApplyResponse/ApplyHistoryResponse) carry only Caller; the pull_request field exists solely on ProgressResponse. Server-side, both are built from the same apply.Repository/PullRequest fields, so the two URLs agree in practice — the 412-vs-999 test values are artificial, pinning precedence rather than an expected real divergence. Plumbing pull_request into the list responses is a reasonable follow-up if we want the invariant structural.

6: Fixed — repo segments are validated against a GitHub-safe character class (validRepoSegment, mirroring ValidHost's rationale), so newlines, spaces, and terminal escapes are rejected before anything renders.

7: Fixed — PR numbers must be plain positive decimals: #+5, #-1, and #007 are all refused, with tests.

8: Fixtures fixed — history and per-deployment previews now use wire-format callers, and the regenerated TEMPLATES.md shows the widest deployment SOURCE row. OSC 8 hyperlink emission is deferred: it's a CLI-wide presentation decision (every URL we print, plus terminal-capability detection), better taken as its own change than folded in here.

This reply was generated by Claude Code (Fable 5).

@aparajon
aparajon merged commit 82899b0 into main Aug 19, 2026
34 checks passed
@aparajon
aparajon deleted the armand/status-source branch August 19, 2026 09:15
Kiran01bm added a commit that referenced this pull request Aug 23, 2026
…ew-drift-rollup

* origin/main: (357 commits)
  fix(github): render each lint violation as its own bullet in unsafe-change comments (#1105)
  feat(engine): disclose at plan time whether an apply continues or discards a copy (#1087)
  fix(operator): choose the drive mode from the generation manifest, not the attached row count (#1101)
  feat(tern): one deployment correlates to exactly one remote apply (#1060)
  fix(github): record the passing check when an apply plan finds no changes (#1099)
  feat(spirit): detect an unfinished row copy and log what the apply will do to it (#1048)
  docs: reserve metrics for signals worth alerting on (#1089)
  feat(cli): browse stored plan history with the list-plans command (#1083)
  feat(cli): render status sources as OSC 8 hyperlinks on interactive terminals (#1097)
  feat(github): show VSchema changes in sharded apply comments (#1096)
  test(webhook): PostgreSQL failure-matrix row — declined stop is terminal, apply completes (#1098)
  feat(observability): log the delivery GUID when a goroutine panics (#1092)
  test(webhook): pin apply-confirm lock-path dispositions (#1091)
  fix(api): type terminal rollback validation errors (#1090)
  build(deps): pin pg-sprite to released v0.1.0 (#1093)
  feat(cli): show apply provenance as a clickable source in status output (#1086)
  fix(github): give sharded applies a real terminal summary comment (#1085)
  fix(vitess): gate stored-plan applies on recorded VSchema deletions and mutations (#1084)
  webhook: PostgreSQL failure-matrix rows — restart survival and permanent privilege refusal (#1079)
  fix(tern): complete a deployment-keyed apply only when its generation manifest is satisfied (#1076)
  ...

# Conflicts:
#	pkg/webhook/plan.go
#	pkg/webhook/templates/plan.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants