Skip to content

feat(workflow): add --watch to workflow approve for multi-gate runs (#26) - #63

Merged
pradyb merged 3 commits into
developfrom
feat/26-approve-watch
Sep 30, 2026
Merged

feat(workflow): add --watch to workflow approve for multi-gate runs (#26)#63
pradyb merged 3 commits into
developfrom
feat/26-approve-watch

Conversation

@pradyb

@pradyb pradyb commented Sep 30, 2026

Copy link
Copy Markdown
Owner

What / why

workflow approve previously handled one round of gates per invocation — a run with several
sequential environment gates needed the command re-run manually after each approval. This adds
--watch, which stays attached to the run and decides each newly-appearing gate (after
confirmation, unless --yes) until the run completes.

  • --watch / -W: enable the loop (requires exactly one -r)
  • --interval <sec>: polling interval, default 10, minimum 5 (enforced at runtime)
  • --timeout <duration>: give up after this long (e.g. 30m); 0 (default) = no timeout
  • --watch --yes additionally requires --environment, so an unattended watch never approves a
    gate that didn't exist when it started
  • Ctrl-C (via Cobra's ExecuteContext/cmd.Context()) stops the watch cleanly without deciding
    anything further
  • --watch --dry-run intentionally falls through to the existing non-looping preview (prints
    currently-pending gates only), rather than looping in dry-run mode
  • Under --json, events stream as compact single-line NDJSON (one JSON object per gate
    decided/skipped/errored, plus a final run_done), not ui.PrintJSON's pretty-printed form

Behaviour notes

  • Core loop logic lives in pkg/workflow/watch.go (WatchApprovals), reusing the existing
    service.GetWorkflowRun / ListPendingDeployments / ReviewPendingDeployments and the
    package's existing latestRunID resolution — no duplication of the non-watch approve path.
  • A gate outside --environment's allowlist is left genuinely pending (not marked "seen"), so a
    later invocation that adds it to the allowlist picks it up automatically; it's reported once,
    not on every poll.
  • A gate you're not a required reviewer of is skipped and reported once.
  • A per-gate submit failure (e.g. someone else already decided it) is reported via a gate_error
    event and does not stop the watch; a 401/403 does stop it (auth is unlikely to recover mid-loop).
  • internal/testutils: added MockGitHubServer.SetResponseSequence so tests can simulate a run's
    state changing across successive polls (falls back to the existing SetResponse when no
    sequence is configured).

Testing

  • pkg/workflow/watch_test.go: 13 tests covering the approve/reject loop, sequential gates,
    decline-then-continue, the --yes + --environment allowlist requirement, non-reviewer/outside-
    allowlist skipping and single-report behaviour, concurrent-decision and auth-error handling,
    Ctrl-C cancellation, timeout, and latest-run resolution.
  • internal/testutils/mock_server_test.go: 4 tests for SetResponseSequence's advance/repeat/
    fallback/priority behaviour.
  • cmd/workflow/workflow_test.go: 8 new tests covering flag validation (single-repo requirement,
    --yes without --environment, minimum --interval, non-TTY without --yes), the
    --yes end-to-end path, the interactive-confirm accept and decline+timeout paths, and NDJSON
    --json output shape.
  • gofmt -l ., go vet ./..., go build ./..., go test -race ./... all clean;
    ./scripts/check-coverage.sh reports 93.7% (floor 85%).
  • Not verified live against a real GitHub Actions run in this PR (unlike workflow approve: approve or reject pending deployment gates for a run #20/feat(workflow): add approve command for environment approval gates #21's original
    workflow approve, which was live-tested) — the session's SGH_TOKEN may lack the
    Deployments-write/reviewer scope needed, per the project's noted token-permission caveat. The
    non-watch path this reuses (GetWorkflowRun, ListPendingDeployments,
    ReviewPendingDeployments) was already live-verified for workflow approve: approve or reject pending deployment gates for a run #20.

Note on base branch

This PR is intentionally based on feat/1-keyring-token-storage (#62), not develop — stacked
per request while #1 is still being manually tested. It will need rebasing/retargeting to
develop once #1 merges.

Closes #26

https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD

@pradyb
feat(config): store per-owner tokens in the OS keyring instead of pla…
897da38 
…intext (#1)

Claude-Session: https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD
@pradyb
feat(workflow): add --watch to workflow approve for multi-gate runs (#26
78d7e9a 
)

Stays attached to a run and decides each newly-appearing environment
approval gate (after confirmation, unless --yes) until the run completes,
instead of requiring a re-run of `workflow approve` after every gate.

- New --watch/-W, --interval (default 10s, min 5), --timeout (0 = none) flags
- --watch --yes requires --environment, so an unattended watch never
  approves a gate that didn't exist when it started
- Ctrl-C stops cleanly without deciding anything further
- --watch --dry-run falls through to the existing non-looping preview
- NDJSON event output under --json (one compact object per event)
- internal/testutils: MockGitHubServer.SetResponseSequence lets tests
  simulate a run's state changing across successive polls

Closes #26

Claude-Session: https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD
@pradyb pradyb mentioned this pull request Sep 30, 2026
Merged
@pradyb
pradyb deleted the branch develop September 30, 2026 03:02
@pradyb pradyb closed this Sep 30, 2026
@pradyb pradyb reopened this Sep 30, 2026
@pradyb
pradyb changed the base branch from feat/1-keyring-token-storage to develop September 30, 2026 03:04
@pradyb
merge: resolve CHANGELOG conflict merging develop into feat/26-approv…
48d8d10 
…e-watch

Retargeting #63 from feat/1-keyring-token-storage to develop after #62 (#1)
merged; both branches added an Unreleased/Added entry, kept both (#26's
entry first, matching this branch's existing order).

Claude-Session: https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD
@pradyb
pradyb merged commit d3b9326 into develop Sep 30, 2026
7 checks passed
@pradyb
pradyb deleted the feat/26-approve-watch branch September 30, 2026 03:08
@pradyb
pradyb restored the feat/26-approve-watch branch September 30, 2026 03:08
@pradyb
pradyb deleted the feat/26-approve-watch branch September 30, 2026 03:08
pradyb added a commit that referenced this pull request Sep 30, 2026
@pradyb
merge: resolve CHANGELOG conflict merging develop into feat/64-view-w…
48abc17 
…atch-json

Retargeting #65 from feat/26-approve-watch to develop after #63 (#26) merged;
both branches added an Unreleased/Added entry, kept both (#64's and #26's
entries first, matching this branch's existing order).

Claude-Session: https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD
pradyb added a commit that referenced this pull request Sep 30, 2026
@pradyb
fix: remove duplicate approve --watch test block from the develop merge
b5ef84a 
The merge of develop (containing #63's squash-merged commit) into this
branch (which already had #63's pre-squash commits as ancestors) silently
duplicated the entire "workflow approve --watch" test section instead of
recognizing it as already present, since the two copies reached the file
via different diff paths (squash vs. original commits) and didn't line up
for git's merge to dedupe automatically. go vet caught it immediately
("approveRunPath redeclared in this block"). Removed the duplicate,
confirmed the file is otherwise identical, gofmt clean, full suite green.

Claude-Session: https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD
pradyb added a commit that referenced this pull request Sep 30, 2026
@pradyb
feat(workflow): support NDJSON output for view --watch --json (#64) (#65
93c75fd 
)

* feat(config): store per-owner tokens in the OS keyring instead of plaintext (#1)

Claude-Session: https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD

* feat(workflow): add --watch to workflow approve for multi-gate runs (#26)

Stays attached to a run and decides each newly-appearing environment
approval gate (after confirmation, unless --yes) until the run completes,
instead of requiring a re-run of `workflow approve` after every gate.

- New --watch/-W, --interval (default 10s, min 5), --timeout (0 = none) flags
- --watch --yes requires --environment, so an unattended watch never
  approves a gate that didn't exist when it started
- Ctrl-C stops cleanly without deciding anything further
- --watch --dry-run falls through to the existing non-looping preview
- NDJSON event output under --json (one compact object per event)
- internal/testutils: MockGitHubServer.SetResponseSequence lets tests
  simulate a run's state changing across successive polls

Closes #26

Claude-Session: https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD

* feat(workflow): support NDJSON output for view --watch --json (#64)

--watch and --json on `workflow view` were previously mutually exclusive
(--watch is an interactive bubbletea view). With --json, --watch now instead
streams each job/step status transition as compact single-line JSON (NDJSON)
until the run completes, for scripting or logging a watched run:

  job_started, job_completed, step_started, step_completed, run_done

- pkg/workflow/watch_view.go: WatchViewRun polls GetWorkflowRunDetail,
  diffing job/step status against what was already reported so a poll with
  no change produces no events; a job/step that jumps straight to completed
  (skipping in_progress) reports only completed, not a synthetic started
- Reuses the existing --interval flag and the DI Sleep pattern from #26's
  approve --watch; stops cleanly on ctx cancellation (Ctrl-C)
- The plain interactive --watch (no --json) is unchanged

Closes #64

Claude-Session: https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD

* fix: remove duplicate approve --watch test block from the develop merge

The merge of develop (containing #63's squash-merged commit) into this
branch (which already had #63's pre-squash commits as ancestors) silently
duplicated the entire "workflow approve --watch" test section instead of
recognizing it as already present, since the two copies reached the file
via different diff paths (squash vs. original commits) and didn't line up
for git's merge to dedupe automatically. go vet caught it immediately
("approveRunPath redeclared in this block"). Removed the duplicate,
confirmed the file is otherwise identical, gofmt clean, full suite green.

Claude-Session: https://claude.ai/code/session_01Wy6Fqe5usEojMuNcAjKvRD
@pradyb pradyb mentioned this pull request Sep 30, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

workflow approve: add --watch to decide each approval gate as it appears

1 participant

@pradyb