From a0f1c31a52c2fe8aca055d4095c735eb032c80d5 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 28 May 2026 22:40:14 -0400 Subject: [PATCH 01/24] docs(abctl): Add design spec for in-place pipeline editor Design for the next major abctl feature: pressing "e" on the Pipeline pane opens the agent's runtime pipeline subtree in the user's $EDITOR. On save, abctl shows a diff, applies the edit to the per-agent ConfigMap via kubectl apply --server-side, polls /reload/status until the framework reloads, and refreshes the Pipeline pane. The single edit-pipeline-subtree flow naturally covers all four sub-features (edit a plugin's config, reorder plugins, remove a plugin, add a plugin) since each is just lines the user changes inside the subtree. Key foundations: A1 (write via kubectl), B1 (free-form YAML; framework validates on reload), tea.ExecProcess for the editor suspend/resume, two-port port-forward (:9094 + :9093) so abctl can reach /reload/status without a second forward. Spec only - no code. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../2026-05-28-abctl-pipeline-edit-design.md | 406 ++++++++++++++++++ 1 file changed, 406 insertions(+) create mode 100644 authbridge/docs/superpowers/specs/2026-05-28-abctl-pipeline-edit-design.md diff --git a/authbridge/docs/superpowers/specs/2026-05-28-abctl-pipeline-edit-design.md b/authbridge/docs/superpowers/specs/2026-05-28-abctl-pipeline-edit-design.md new file mode 100644 index 000000000..787a5c99d --- /dev/null +++ b/authbridge/docs/superpowers/specs/2026-05-28-abctl-pipeline-edit-design.md @@ -0,0 +1,406 @@ +# abctl pipeline editor — in-place ConfigMap editing with hot-reload + +**Status:** Design — pending user review +**Date:** 2026-05-28 + +## Goal + +Make abctl's Pipeline pane editable. Pressing `e` opens the agent's +runtime `pipeline:` subtree in `$EDITOR`; on save, abctl shows a diff, +applies the edit to the per-agent ConfigMap via `kubectl apply +--server-side`, polls `/reload/status` until the framework reloads, and +returns the user to the Pipeline pane now showing the new state. + +This single flow covers four operations the user can perform inside the +editor — **edit a plugin's config, reorder plugins, remove a plugin, +add a plugin** — because each is just lines the user changes inside the +pipeline subtree. + +## Non-goals + +- Editing config OUTSIDE the `pipeline:` subtree (`mode`, `listener`, + `session`, `mtls`, `spiffe`). Typos there can make the pod + unschedulable or break the listener; locking the editor to the + pipeline subtree limits the blast radius. +- Schema-aware form editor. The user types raw YAML and the framework + validates on reload. A schema endpoint + structured form is a + follow-up if it's wanted. +- Multi-pod fanout. The picker already constrains the user to one + agent; the edit flow operates on that one ConfigMap. +- Undo / audit trail beyond the per-edit diff preview. The cluster's + audit log + git are the right places for that. +- Inline secret redaction in the diff. The convention (mirrored from + `:9093/config` and the new `:9094/v1/pipeline`) is `*_file` paths, + never inline secrets. The diff shows what the user typed. + +## User-facing behavior + +``` +Pipeline pane press "e" + │ │ + ▼ ▼ +[fetching ConfigMap...] (overlay, ~1s) + │ + ▼ +$EDITOR opens on /tmp/abctl-pipeline-XXXXXX.yaml +(content: only the pipeline: subtree of data.config.yaml, + indented at the subtree's natural indent) + │ user edits, saves, exits + ▼ +[validating YAML...] (overlay, instant) + │ + │ if invalid: error + "r" to re-edit + ▼ +┌────────── DIFF OVERLAY ──────────┐ +│ pipeline: │ +│ inbound: │ +│ - name: jwt-validation │ +│ - config: {issuer: a} │ +│ + config: {issuer: b} │ +│ outbound: │ +│ ... │ +│ │ +│ apply this change? (y/N) │ +└──────────────────────────────────┘ + │ y + ▼ +[applying to ConfigMap...] (overlay, ~1-2s) + │ + ▼ +[waiting for hot-reload...] (overlay, up to 120s) + spinner; updates as `last_success` advances on /reload/status + │ + ▼ +Pipeline pane (refreshed; new state) +``` + +Cancel paths: `Esc` from any overlay returns to the Pipeline pane. +`N` at the diff prompt is the same as Esc. `r` from a recoverable +error (validation, apply conflict) re-opens the editor with the same +tempfile. The pipeline state is read-only until the user explicitly +confirms apply. + +## Foundation choices (resolved during brainstorming) + +These are constraints the design takes as given: + +- **A1 — write via `kubectl`.** abctl shells out to kubectl (matching + the picker's pattern). The user's kubeconfig + RBAC controls writes. + No new write API on the sidecar. +- **B1 — free-form YAML.** No schema discovery; the framework's + reload-time validation is the source of truth. Bad YAML keeps the + active pipeline serving (per `authbridge/CLAUDE.md` §"Config + Hot-Reload") and reports the error on `/reload/status`. +- **Editor model — `$EDITOR` via `tea.ExecProcess`.** Bubbletea + suspends, the user's chosen editor takes the terminal, abctl + resumes. Mirrors `kubectl edit`, `git commit`, `K9s edit`. +- **Edit scope — pipeline subtree.** Listener / session / mtls etc. + are out of scope; the editor only sees and only mutates the + `pipeline:` block. +- **Apply mechanism — `kubectl apply --server-side`.** Uses + `resourceVersion` for optimistic concurrency; concurrent edits + surface as a 409 the user sees verbatim. +- **Reload feedback — poll `/reload/status` on `:9093`.** Watches + `last_success_unix` advancing past the apply timestamp (success) or + `reloads_failed_total` incrementing (failure with the framework's + error message). 120s timeout. +- **Port-forward both `:9094` and `:9093`.** The picker's + `kubectl port-forward` is extended to map two ephemeral local + ports. abctl uses `:9094` for sessions/pipeline and `:9093` for + reload-status. + +## Architecture + +```text +abctl picker mode + │ + ▼ user picks pod; PortForward forwards both :9094 and :9093 + │ +panePipeline + │ press "e" + ▼ +edit state machine (cmd/abctl/edit/ + cmd/abctl/tui/edit_overlay.go) + + ① Fetch kubectl get cm authbridge-config- -n -o yaml + → parse → extract data.config.yaml string + → locate "pipeline:" byte range inside + → write subtree bytes to /tmp/abctl-pipeline-XXXX.yaml + + ② Edit tea.ExecProcess($EDITOR tempPath) + — bubbletea suspends; resumes on editor exit + + ③ Validate parse the edited file as YAML + — on error, surface line/col, offer "r" to re-edit + + ④ Diff line-diff old vs new pipeline subtree, overlay, + "apply? (y/N)" prompt + + ⑤ Apply splice edited subtree back into original config.yaml + (text byte-range surgery — preserves comments outside + the pipeline subtree); rebuild ConfigMap manifest; + kubectl apply --server-side --force-conflicts=false + + ⑥ Wait poll http://127.0.0.1:/reload/status: + last_success_unix > applyTime → success + reloads_failed_total incremented → failure (body has the error) + 120s elapsed → timeout + + ⑦ Refresh re-fetch /v1/pipeline; rebuild pipeline table; close overlay +``` + +The state machine lives entirely on the bubbletea model. Each phase +emits one `tea.Msg`; the corresponding `Update` handler advances state +and dispatches the next `Cmd`. Cancel from any phase returns the user +to `panePipeline` cleanly. + +## Components + +### New package `cmd/abctl/edit/` + +Pure logic, kubectl shell-out behind an injection seam (same pattern +as `cmd/abctl/cluster/`). + +#### `edit/configmap.go` + +```go +package edit + +// Runner abstracts a `kubectl ` invocation. Production uses +// os/exec; tests inject their own. +type Runner func(ctx context.Context, args ...string) ([]byte, error) + +// FetchedPipeline holds the result of a successful fetch + extract: +// the original ConfigMap bytes (untouched, for splice-back) and the +// pipeline-subtree byte range within data.config.yaml (so the editor +// only sees + mutates that subtree). +type FetchedPipeline struct { + ConfigMapYAML []byte // raw `kubectl get cm -o yaml` output + InnerYAML []byte // value of data.config.yaml (the runtime config) + PipelineStart int // byte offset in InnerYAML where "pipeline:" begins + PipelineEnd int // byte offset where the subtree ends +} + +func Fetch(ctx context.Context, run Runner, namespace, agent string) (*FetchedPipeline, error) + +// Splice replaces the pipeline subtree in fp.InnerYAML with newSubtree +// and returns a new ConfigMap YAML manifest ready for `kubectl apply`. +func Splice(fp *FetchedPipeline, newSubtree []byte) ([]byte, error) + +// Apply writes manifest to a tempfile and runs kubectl apply --server-side. +// Returns the apply's wall-clock time (used to compare against +// /reload/status's last_success). +func Apply(ctx context.Context, run Runner, manifest []byte) (applyTime time.Time, err error) +``` + +The pipeline-subtree byte range is found by parsing `InnerYAML` with +`gopkg.in/yaml.v3` to a Node tree, finding the `pipeline:` key node, +and reading its `Line`/`Column` info to locate end-of-subtree +(start-of-next-top-level-key, or end-of-document). The splice is then +a `bytes.Buffer` rebuild: bytes before `PipelineStart`, then +`newSubtree`, then bytes after `PipelineEnd`. **No round-trip through +yaml.v3 emit** — that would lose comments and re-format whitespace. + +#### `edit/diff.go` + +```go +// Diff returns a colorized line-diff of old vs new (lipgloss-styled +// "+", "-", and dim context lines). Implementation is hand-rolled +// LCS-on-lines; the typical pipeline subtree is < 50 lines so even +// quadratic LCS is trivially fast. +func Diff(old, new []byte) string +``` + +No external diff library. yaml.v3's diff helpers are tree-aware and +hide line-by-line changes; users want to see the lines they typed. + +#### `edit/status.go` + +```go +type ReloadStatus struct { + LastSuccessUnix int64 + ReloadsOK uint64 + ReloadsFailed uint64 + LastError string // body when ReloadsFailed advanced +} + +// PollUntilReloaded watches /reload/status until either: +// - LastSuccessUnix > applyTime.Unix(), OR +// - ReloadsFailed exceeds the value at apply time (failure: error in LastError), +// - 120s timeout. +// Returns sum-typed result. +func PollUntilReloaded(ctx context.Context, statusURL string, applyTime time.Time) PollResult +``` + +Polls every 1s. The 1s cadence is a balance between user-visible +spinner progress and avoiding pointless cluster chatter. + +#### `edit/edit.go` + +Top-level orchestrator: + +```go +type Result struct { + Status ResultStatus // applied / aborted / yamlInvalid / + // applyError / reloadError / timeout + Diff string // populated for any post-validate state + Error error // present for any *Error status + NewYAML []byte // populated for applied +} + +type ResultStatus int + +const ( + StatusUnknown ResultStatus = iota + StatusApplied + StatusAborted + StatusYAMLInvalid + StatusApplyError + StatusReloadError + StatusTimeout + StatusNoChanges +) +``` + +Each phase fires one tea.Msg; a state machine on the bubbletea model +tracks where we are. The orchestrator is split between +`edit/edit.go` (pure phase functions, returning results) and +`tui/edit_overlay.go` (UI state, drives the phases via tea.Cmds). + +### Modified `cluster/portforward.go` + +The `kubectlPortForwarder.Start` method is extended to forward two +ports. The `PortForward` interface gains: + +```go +type PortForward interface { + Endpoint() string // existing — http://127.0.0.1: + StatusEndpoint() string // NEW — http://127.0.0.1: + Close() error // existing +} +``` + +`kubectl port-forward` accepts multiple `host:remote` pairs, e.g. +`9094:9094 9093:9093`, so this is one subprocess. Both ports use +`waitForAccept` to confirm readiness. + +### TUI integration + +`tui/edit_overlay.go` (new) — owns the overlay UI: + +```go +type editPhase int + +const ( + editPhaseFetching editPhase = iota + editPhaseEditing // $EDITOR is running; abctl is suspended + editPhaseValidating + editPhaseDiff + editPhaseApplying + editPhaseWaiting + editPhaseError + editPhaseDone +) + +// editState lives on *model. +type editState struct { + phase editPhase + fetched *edit.FetchedPipeline + tempPath string + diff string // colorized + err string // single-line message in editPhaseError + applyTime time.Time +} + +func (m *model) showEditOverlay() string // rendered +``` + +`tui/keys.go` — add `case "e":` in the `panePipeline` arm; sets +`m.editState.phase = editPhaseFetching` and dispatches `editFetchCmd`. + +`tui/app.go` — five new `tea.Msg` types (one per phase transition): +`editFetchedMsg`, `editorExitedMsg`, `editValidatedMsg`, +`editAppliedMsg`, `editReloadedMsg`. Each handler in `Update` advances +`m.editState.phase` and dispatches the next Cmd. + +The overlay renders on top of the existing Pipeline pane View when +`m.editState.phase != editPhaseDone`; otherwise the Pipeline pane +renders normally. + +## Error handling + +Every error / cancel path returns the user to `panePipeline` cleanly. +No stuck states. + +| Failure | Surface | Recovery | +|---|---|---| +| `kubectl get cm` fails (RBAC, missing CM) | `"fetch failed: "` | `Esc` | +| `$EDITOR` exits non-zero | `"editor exited "` | `Esc`; tempfile is left on disk | +| Edited file == original | `"no changes; nothing to apply"` | auto-dismiss after 1s | +| YAML parse error on edited file | `"invalid YAML at line N: "` | `r` re-opens editor; `Esc` aborts | +| User says `N` at diff confirm | drop overlay | n/a | +| `kubectl apply` fails (RBAC denied / 409 conflict / validation) | stderr verbatim | `r` re-fetch + re-edit; `Esc` aborts | +| `/reload/status` reports `reloads_failed_total++` | `"reload failed: "` | `Esc`. Pod is still serving the OLD pipeline (framework keeps active on reload failure — `authbridge/CLAUDE.md` §"Config Hot-Reload"). User fixes their YAML and re-edits | +| 120s timeout polling /reload/status | `"reload not observed in 120s; check kubectl logs deploy/"` | `Esc`. Apply succeeded; kubelet just hasn't synced. Next /v1/pipeline fetch eventually shows the new state | +| User quits abctl mid-edit (q / Ctrl+C / SIGTERM) | program exits | `m.cancel()` propagates ctx, kubectl subprocesses die, port-forward closes, tempfile is left for recovery | + +## Concurrent edits + +`kubectl apply --server-side --force-conflicts=false` rejects +conflicting field changes with a clear 409 message naming the +conflicting field manager. We surface verbatim. `r` triggers a +re-fetch (which gets the updated state) and re-edit. + +This is sufficient for the demo / single-operator case. Multi-operator +contention with diverging concurrent edits is rare in practice; if it +becomes a problem, a follow-up could read-after-apply to confirm. + +## Trust & permissions + +- abctl shells out to kubectl, which uses the user's kubeconfig. + RBAC enforcement is the cluster's job. +- Caller needs `get` and `update` on `configmaps` in the agent's + namespace. `get` was already needed for the picker; `update` is + new for this PR. +- The edit endpoint is the cluster's API server, not the sidecar. + No change to the sidecar's existing trust model (`:9094` and + `:9093` are still in-cluster, no auth). +- Edits go to a ConfigMap that may already contain references to + secrets via `*_file` paths. abctl doesn't introspect or redact — + what the user types is what gets applied. Same model as + `kubectl edit configmap`. + +## Testing + +| File | What | +|---|---| +| `edit/configmap_test.go` | Find pipeline byte range in fixture inner YAML; splice modified subtree; verify byte-for-byte that comments outside the pipeline subtree survive. Table-driven. | +| `edit/configmap_test.go` | End-to-end fetch → splice → apply with a stub kubectl Runner that captures the manifest written. Stub records args + final manifest bytes. | +| `edit/diff_test.go` | Diff renderer: equal inputs → empty diff; one-line edit; multi-line; reorder. Assert specific `+`/`-`/context line markers. | +| `edit/status_test.go` | `PollUntilReloaded` against an `httptest.Server` that returns successive ReloadStatus snapshots; verify success, failure, and timeout paths. | +| `edit/edit_test.go` | State machine end-to-end with fakes: stub kubectl, stub `/reload/status`, fake "editor" (writes pre-canned bytes to the tempfile). Drive the bubbletea model via `Update` directly. | +| `cluster/portforward_test.go` | Two-port mode of `kubectlPortForwarder` — wait for both local ports to accept. Same `freeLocalPort` + Go-listener pattern as existing tests. | +| `tui/edit_overlay_test.go` | Render the overlay in each phase (fetching / editing / validating / diff / applying / waiting / error). Drive `m.Update` with each new msg, assert overlay text. | +| `cmd/abctl/cluster/e2e_test.go` (existing, build-tag `e2e`) | New subtest drives the real edit flow against the live IBAC cluster: pick a Configurable plugin, append a no-op key to its config, apply, wait for reload, assert /v1/pipeline reflects the new value. | + +## Acceptance criteria + +1. Pressing `e` on the Pipeline pane opens `$EDITOR` with the agent's + `pipeline:` subtree. +2. On editor exit with no changes, abctl shows "no changes" and returns. +3. On editor exit with changes, abctl shows a colorized diff and + prompts `apply this change? (y/N)`. +4. `y` writes the edit to the per-agent ConfigMap via `kubectl apply + --server-side`. `N` discards. +5. After apply, abctl polls `/reload/status` until the framework + reports the reload succeeded (or failed); both outcomes are + surfaced clearly. +6. After successful reload, the Pipeline pane refreshes from + `/v1/pipeline` and shows the new state. +7. Add / remove / reorder a plugin all work because they're just + line-edits inside the pipeline subtree. +8. RBAC denial on apply surfaces verbatim and the user can `Esc` back + to the Pipeline pane without abctl losing state. +9. Bad YAML on save surfaces a clear error with line/col and offers + to re-open the editor. +10. `go test ./...` passes including the new tests; the e2e build-tag + test passes against a live IBAC cluster. From 2eae87c2a501879f60e21c11ef03216094758655 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:20:11 -0400 Subject: [PATCH 02/24] docs(abctl): Add implementation plan for in-place pipeline editor Ten-task TDD plan covering: two-port port-forward, edit/configmap.go (FindPipelineRange, Splice, BuildManifest, Fetch, Apply with kubectl Runner injection seam), edit/diff.go (line-based LCS with lipgloss), edit/status.go (poll /reload/status with success/failure/timeout), tui/edit_overlay.go (state + render), edit/edit.go (tea.Cmd factories), TUI Update wiring + e keybind + end-to-end test, README. Each task is TDD-shaped with full code in every step. No placeholders. Spec at docs/superpowers/specs/2026-05-28-abctl-pipeline-edit-design.md. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../plans/2026-05-28-abctl-pipeline-edit.md | 2522 +++++++++++++++++ 1 file changed, 2522 insertions(+) create mode 100644 authbridge/docs/superpowers/plans/2026-05-28-abctl-pipeline-edit.md diff --git a/authbridge/docs/superpowers/plans/2026-05-28-abctl-pipeline-edit.md b/authbridge/docs/superpowers/plans/2026-05-28-abctl-pipeline-edit.md new file mode 100644 index 000000000..cf210e628 --- /dev/null +++ b/authbridge/docs/superpowers/plans/2026-05-28-abctl-pipeline-edit.md @@ -0,0 +1,2522 @@ +# abctl Pipeline Editor Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make abctl's Pipeline pane editable: pressing `e` opens the agent's runtime `pipeline:` subtree in `$EDITOR`, then on save shows a diff, applies via `kubectl apply --server-side`, polls `/reload/status` until the framework reloads, and refreshes the pane. + +**Architecture:** New `cmd/abctl/edit/` package owns the kubectl wrappers, byte-range YAML splice, line-diff renderer, and reload-status poller — each behind a `Runner` injection seam (mirrors `cmd/abctl/cluster/`). The `cluster.PortForwarder` is extended to forward both `:9094` and `:9093` so abctl can reach `/reload/status` without a second port-forward. A new TUI overlay state machine drives the user through fetch → edit → validate → diff → apply → wait phases, using `tea.ExecProcess` to suspend bubbletea while `$EDITOR` runs. + +**Tech Stack:** Go 1.24, `gopkg.in/yaml.v3` (new dependency, ~200KB), `os/exec`, bubbletea (`tea.ExecProcess`), existing lipgloss + `cluster.Runner` patterns. + +**Spec:** [`docs/superpowers/specs/2026-05-28-abctl-pipeline-edit-design.md`](../specs/2026-05-28-abctl-pipeline-edit-design.md) + +--- + +## File Structure + +**New files:** +- `authbridge/cmd/abctl/edit/configmap.go` — `Runner` type, `FetchedPipeline` struct, `FindPipelineRange`, `Splice`, `Fetch`, `Apply`. +- `authbridge/cmd/abctl/edit/configmap_test.go` — unit tests with golden-YAML fixtures + stub `Runner`. +- `authbridge/cmd/abctl/edit/diff.go` — `Diff(old, new []byte) string` line-based, lipgloss-colored. +- `authbridge/cmd/abctl/edit/diff_test.go` — equal / one-line / multi-line / reorder cases. +- `authbridge/cmd/abctl/edit/status.go` — `ReloadStatus` struct, `PollUntilReloaded`. +- `authbridge/cmd/abctl/edit/status_test.go` — `httptest.Server` driving success / failure / timeout paths. +- `authbridge/cmd/abctl/edit/edit.go` — `tea.Cmd` factories that wrap each phase (used by the TUI Update handlers). +- `authbridge/cmd/abctl/edit/edit_test.go` — Cmd factory unit tests (each Cmd produces the expected msg with stubbed dependencies). +- `authbridge/cmd/abctl/tui/edit_overlay.go` — `editPhase` enum, `editState` struct, render functions. +- `authbridge/cmd/abctl/tui/edit_overlay_test.go` — render assertions per phase. + +**Modified files:** +- `authbridge/cmd/abctl/cluster/portforward.go` — extend `kubectlPortForwarder.Start` to forward two ports. `PortForward` interface gains `StatusEndpoint() string`. +- `authbridge/cmd/abctl/cluster/portforward_test.go` — add `TestPortForwarderTwoPorts`. +- `authbridge/cmd/abctl/tui/picker_test.go` — extend `fakePortForward` with `StatusEndpoint() string`. +- `authbridge/cmd/abctl/tui/app.go` — new tea.Msg types; new `*model` fields (`editState`, `statusURL`); Update handlers per phase; `View` overlay branch when `editState.phase != editPhaseDone`. +- `authbridge/cmd/abctl/tui/keys.go` — `"e"` keybind on `panePipeline`. +- `authbridge/cmd/abctl/go.mod` — add `gopkg.in/yaml.v3` dependency. +- `authbridge/cmd/abctl/README.md` — document the `e` keybind, RBAC requirements, tempfile lifecycle. + +**Convention notes:** +- The `cmd/abctl/` directory is its own Go module. All `go test` / `go build` invocations cd there: `cd authbridge/cmd/abctl`. +- Repo policy: DCO sign-off (`-s`), `Assisted-By` (NOT `Co-Authored-By`), conventional commits. +- Branch already exists (`feat/abctl-pipeline-edit-spec`) with the design doc committed. All implementation tasks add commits to that branch. + +--- + +## Task 1: Two-port port-forward + +Extend the picker's `kubectl port-forward` to forward both `:9094` (session API) and `:9093` (stat server). Add a `StatusEndpoint() string` accessor on `PortForward` so callers can reach `/reload/status`. + +**Files:** +- Modify: `authbridge/cmd/abctl/cluster/portforward.go` +- Modify: `authbridge/cmd/abctl/cluster/portforward_test.go` +- Modify: `authbridge/cmd/abctl/tui/picker_test.go` (the `fakePortForward` now needs `StatusEndpoint`) + +### Step 1: Write the failing test + +Append to `authbridge/cmd/abctl/cluster/portforward_test.go`: + +```go +func TestKubectlPortForward_StatusEndpoint(t *testing.T) { + // Build-only check that the kubectlPortForward struct exposes a + // StatusEndpoint() method returning a 127.0.0.1 URL with a non-zero + // port. Doesn't spawn kubectl. + pf := &kubectlPortForward{port: 12345, statusPort: 12346} + want := "http://127.0.0.1:12346" + if got := pf.StatusEndpoint(); got != want { + t.Fatalf("StatusEndpoint: got %q want %q", got, want) + } + if got := pf.Endpoint(); got != "http://127.0.0.1:12345" { + t.Fatalf("Endpoint: got %q", got) + } +} + +func TestPortForwarderInterfaceHasStatusEndpoint(t *testing.T) { + // Compile-time assertion that the interface exposes StatusEndpoint. + var _ interface { + Endpoint() string + StatusEndpoint() string + Close() error + } = (*kubectlPortForward)(nil) +} +``` + +### Step 2: Run test to verify it fails + +```bash +cd authbridge/cmd/abctl +go test ./cluster/ -run "TestKubectlPortForward_StatusEndpoint|TestPortForwarderInterfaceHasStatusEndpoint" -v +``` +Expected: build failure — `kubectlPortForward.statusPort` and `StatusEndpoint` undefined. + +### Step 3: Write minimal implementation + +In `authbridge/cmd/abctl/cluster/portforward.go`, modify `kubectlPortForward`: + +```go +type kubectlPortForward struct { + cmd *exec.Cmd + port int // local 127.0.0.1 port forwarding to pod :9094 + statusPort int // local 127.0.0.1 port forwarding to pod :9093 + stderr io.ReadCloser + + mu sync.Mutex + stderrLines []string + stderrDone chan struct{} +} +``` + +Add `StatusEndpoint`: + +```go +func (p *kubectlPortForward) Endpoint() string { + return "http://127.0.0.1:" + strconv.Itoa(p.port) +} + +// StatusEndpoint is the URL of the agent's stat server (:9093) reached +// via the picker's port-forward. abctl's edit flow polls /reload/status +// here. +func (p *kubectlPortForward) StatusEndpoint() string { + return "http://127.0.0.1:" + strconv.Itoa(p.statusPort) +} +``` + +Update the `PortForward` interface (it lives in the same file): + +```go +type PortForward interface { + Endpoint() string + StatusEndpoint() string + Close() error +} +``` + +Now extend `kubectlPortForwarder.Start` to allocate two free local ports and forward both. Replace the existing body with: + +```go +func (k *kubectlPortForwarder) Start(ctx context.Context, namespace, pod string) (PortForward, error) { + port, err := freeLocalPort() + if err != nil { + return nil, err + } + statusPort, err := freeLocalPort() + if err != nil { + return nil, err + } + cmd := exec.Command("kubectl", "port-forward", + "-n", namespace, + "pod/"+pod, + strconv.Itoa(port)+":9094", + strconv.Itoa(statusPort)+":9093", + ) + stderr, err := cmd.StderrPipe() + if err != nil { + return nil, fmt.Errorf("stderr pipe: %w", err) + } + if err := cmd.Start(); err != nil { + return nil, fmt.Errorf("start kubectl port-forward: %w", err) + } + pf := &kubectlPortForward{ + cmd: cmd, port: port, statusPort: statusPort, stderr: stderr, + } + pf.startStderrDrain() + + readyCtx, cancel := context.WithTimeout(ctx, pfReadyTimeout) + defer cancel() + if err := waitForAccept(readyCtx, port); err != nil { + _ = pf.Close() + stderrTail := pf.stderrTail() + if stderrTail != "" { + return nil, fmt.Errorf("port-forward not ready: %w (stderr: %s)", err, stderrTail) + } + return nil, fmt.Errorf("port-forward not ready: %w", err) + } + if err := waitForAccept(readyCtx, statusPort); err != nil { + _ = pf.Close() + stderrTail := pf.stderrTail() + if stderrTail != "" { + return nil, fmt.Errorf("port-forward (:9093) not ready: %w (stderr: %s)", err, stderrTail) + } + return nil, fmt.Errorf("port-forward (:9093) not ready: %w", err) + } + return pf, nil +} +``` + +### Step 4: Update fakePortForward in picker_test.go + +In `authbridge/cmd/abctl/tui/picker_test.go`, find the `fakePortForward` type. Add a `StatusEndpoint()` method: + +```go +func (p *fakePortForward) Endpoint() string { return p.endpoint } +func (p *fakePortForward) StatusEndpoint() string { return "" } // not used by current picker tests +func (p *fakePortForward) Close() error { p.parent.closeCount++; return nil } +``` + +### Step 5: Run tests; expect PASS + +```bash +cd authbridge/cmd/abctl +go test ./cluster/ -v +go test ./tui/ -v +go vet ./cluster/ ./tui/ +``` +Expected: all tests pass, vet clean. The new `TestKubectlPortForward_StatusEndpoint` and the build-only interface check pass. + +### Step 6: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/cluster/portforward.go \ + authbridge/cmd/abctl/cluster/portforward_test.go \ + authbridge/cmd/abctl/tui/picker_test.go +git commit -s -m "feat(abctl): Forward :9093 alongside :9094 in the picker + +The pipeline editor (incoming) needs to reach /reload/status on the +stat server (:9093) to know when the framework finished reloading. +Extend kubectlPortForwarder to forward both ports through one kubectl +subprocess. PortForward interface gains StatusEndpoint() returning the +:9093 URL. + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Task 2: edit/configmap.go — FindPipelineRange + +Pure-Go function that locates the `pipeline:` subtree's byte range inside a runtime config YAML. Uses `gopkg.in/yaml.v3` Node tree only to find the boundaries; the actual splice happens against raw bytes (preserves comments). + +**Files:** +- Create: `authbridge/cmd/abctl/edit/configmap.go` +- Create: `authbridge/cmd/abctl/edit/configmap_test.go` +- Modify: `authbridge/cmd/abctl/go.mod` (will be auto-updated by `go mod tidy` after first import) + +### Step 1: Write the failing test + +Create `authbridge/cmd/abctl/edit/configmap_test.go`: + +```go +package edit + +import ( + "strings" + "testing" +) + +const fixtureMidYAML = `mode: proxy-sidecar + +listener: + forward_proxy_addr: ":8081" + +pipeline: + inbound: + - name: jwt-validation + config: + issuer: http://idp + outbound: + - name: token-exchange + +session: + enabled: true +` + +const fixtureLastYAML = `mode: proxy-sidecar + +pipeline: + inbound: + - name: jwt-validation +` + +const fixtureFirstYAML = `pipeline: + inbound: + - name: jwt-validation + +mode: proxy-sidecar +` + +const fixtureMissingYAML = `mode: proxy-sidecar + +listener: + forward_proxy_addr: ":8081" +` + +func TestFindPipelineRange_Middle(t *testing.T) { + start, end, err := FindPipelineRange([]byte(fixtureMidYAML)) + if err != nil { + t.Fatalf("FindPipelineRange: %v", err) + } + got := fixtureMidYAML[start:end] + // Range must include the full pipeline subtree. + if !strings.Contains(got, "pipeline:") { + t.Fatalf("range missing pipeline header: %q", got) + } + if !strings.Contains(got, "token-exchange") { + t.Fatalf("range missing pipeline body: %q", got) + } + // Range must NOT include the next top-level key. + if strings.Contains(got, "session:") { + t.Fatalf("range includes next key: %q", got) + } + if strings.Contains(got, "listener:") { + t.Fatalf("range includes prior key: %q", got) + } +} + +func TestFindPipelineRange_LastKey(t *testing.T) { + start, end, err := FindPipelineRange([]byte(fixtureLastYAML)) + if err != nil { + t.Fatalf("FindPipelineRange: %v", err) + } + if end != len(fixtureLastYAML) { + t.Fatalf("end = %d, want len(yaml) = %d", end, len(fixtureLastYAML)) + } + got := fixtureLastYAML[start:end] + if !strings.Contains(got, "jwt-validation") { + t.Fatalf("range missing pipeline body: %q", got) + } +} + +func TestFindPipelineRange_FirstKey(t *testing.T) { + start, _, err := FindPipelineRange([]byte(fixtureFirstYAML)) + if err != nil { + t.Fatalf("FindPipelineRange: %v", err) + } + if start != 0 { + t.Fatalf("start = %d, want 0", start) + } +} + +func TestFindPipelineRange_Missing(t *testing.T) { + _, _, err := FindPipelineRange([]byte(fixtureMissingYAML)) + if err == nil { + t.Fatal("want error when pipeline key is absent") + } + if !strings.Contains(err.Error(), "pipeline") { + t.Fatalf("error should mention pipeline: %v", err) + } +} +``` + +### Step 2: Run test to verify it fails + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -run TestFindPipelineRange -v +``` +Expected: build failure — `edit` package + `FindPipelineRange` undefined. + +### Step 3: Write minimal implementation + +Create `authbridge/cmd/abctl/edit/configmap.go`: + +```go +// Package edit implements abctl's in-place pipeline editor. The flow is: +// fetch the agent's ConfigMap via kubectl, locate the pipeline: subtree, +// open just that subtree in the user's $EDITOR, splice the edit back into +// the original ConfigMap manifest, kubectl apply --server-side, then poll +// /reload/status until the framework reloads. +// +// All kubectl interaction goes through the Runner injection seam so tests +// can stub it out. +package edit + +import ( + "bytes" + "fmt" + + "gopkg.in/yaml.v3" +) + +// FindPipelineRange returns the byte offsets [start, end) in innerYAML +// that span the "pipeline:" subtree, including the "pipeline:" key line +// itself but not any following top-level keys. Used by the editor to +// extract just the pipeline subtree for the user, and by Splice to +// replace it with the user's edit. +// +// Returns an error if innerYAML is not valid YAML or if no top-level +// "pipeline" key exists. +func FindPipelineRange(innerYAML []byte) (start, end int, err error) { + var root yaml.Node + if err := yaml.Unmarshal(innerYAML, &root); err != nil { + return 0, 0, fmt.Errorf("parse runtime YAML: %w", err) + } + if root.Kind != yaml.DocumentNode || len(root.Content) == 0 { + return 0, 0, fmt.Errorf("runtime YAML is not a document") + } + doc := root.Content[0] + if doc.Kind != yaml.MappingNode { + return 0, 0, fmt.Errorf("runtime YAML root is not a mapping") + } + + // Children of a MappingNode alternate key, value, key, value, ... + // Find the index of the "pipeline" key, capture its line, and find + // the next sibling's line (or end-of-document if it's the last key). + pipelineKeyIdx := -1 + for i := 0; i < len(doc.Content); i += 2 { + k := doc.Content[i] + if k.Value == "pipeline" { + pipelineKeyIdx = i + break + } + } + if pipelineKeyIdx == -1 { + return 0, 0, fmt.Errorf("no top-level pipeline key in runtime YAML") + } + + pipelineKeyLine := doc.Content[pipelineKeyIdx].Line // 1-indexed + var nextKeyLine int // 1-indexed; 0 if pipeline is last + if pipelineKeyIdx+2 < len(doc.Content) { + nextKeyLine = doc.Content[pipelineKeyIdx+2].Line + } + + // Map line numbers to byte offsets. yaml.v3 Line is 1-indexed. + lineStarts := []int{0} // lineStarts[i] = byte offset where line i+1 starts + for i, b := range innerYAML { + if b == '\n' { + lineStarts = append(lineStarts, i+1) + } + } + + if pipelineKeyLine < 1 || pipelineKeyLine > len(lineStarts) { + return 0, 0, fmt.Errorf("pipeline key line %d out of range", pipelineKeyLine) + } + start = lineStarts[pipelineKeyLine-1] + + if nextKeyLine == 0 { + end = len(innerYAML) + } else { + if nextKeyLine < 1 || nextKeyLine > len(lineStarts) { + return 0, 0, fmt.Errorf("next-key line %d out of range", nextKeyLine) + } + end = lineStarts[nextKeyLine-1] + } + return start, end, nil +} + +// preserve unused-import safety while we bootstrap the file +var _ = bytes.Buffer{} +``` + +(The `var _ = bytes.Buffer{}` line keeps the `bytes` import alive; it'll be used by `Splice` in Task 3 and removed then.) + +Run `go mod tidy` to add yaml.v3: + +```bash +cd authbridge/cmd/abctl +go mod tidy +``` + +### Step 4: Run tests; expect PASS + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -v +go vet ./edit/ +``` +Expected: all 4 tests PASS, vet clean. + +### Step 5: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/edit/configmap.go \ + authbridge/cmd/abctl/edit/configmap_test.go \ + authbridge/cmd/abctl/go.mod \ + authbridge/cmd/abctl/go.sum +git commit -s -m "feat(abctl): Add edit package with FindPipelineRange + +Pure-Go function that locates the pipeline: subtree's byte range +inside a runtime config YAML. Uses yaml.v3 only to find boundaries; +the actual splice (Task 3) happens against raw bytes to preserve +comments outside the subtree. + +Adds gopkg.in/yaml.v3 dependency. + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Task 3: edit/configmap.go — Splice + manifest rebuild + +Add `Splice` (byte-range surgery on the inner YAML) and `BuildManifest` (parse the outer ConfigMap YAML, replace `data["config.yaml"]`, re-emit). The two functions together produce the manifest passed to `kubectl apply`. + +**Files:** +- Modify: `authbridge/cmd/abctl/edit/configmap.go` +- Modify: `authbridge/cmd/abctl/edit/configmap_test.go` + +### Step 1: Write the failing tests + +Append to `authbridge/cmd/abctl/edit/configmap_test.go`: + +```go +const fixtureCMYAML = `apiVersion: v1 +kind: ConfigMap +metadata: + name: authbridge-config-email-agent + namespace: team1 +data: + config.yaml: | + mode: proxy-sidecar + pipeline: + inbound: + - name: jwt-validation + config: + issuer: old + session: + enabled: true +` + +func TestSplice_PreservesOutsideRange(t *testing.T) { + const orig = `mode: proxy-sidecar +# this comment must survive +listener: + forward_proxy_addr: ":8081" + +pipeline: + inbound: + - name: jwt-validation + +session: + enabled: true +` + start, end, err := FindPipelineRange([]byte(orig)) + if err != nil { + t.Fatal(err) + } + const newSubtree = `pipeline: + inbound: + - name: jwt-validation + config: + issuer: new + +` + got := Splice([]byte(orig), start, end, []byte(newSubtree)) + gotS := string(got) + if !strings.Contains(gotS, "# this comment must survive") { + t.Fatalf("comment outside pipeline subtree was dropped:\n%s", gotS) + } + if !strings.Contains(gotS, "listener:") { + t.Fatalf("listener section was dropped:\n%s", gotS) + } + if !strings.Contains(gotS, "session:") { + t.Fatalf("session section was dropped:\n%s", gotS) + } + if !strings.Contains(gotS, "issuer: new") { + t.Fatalf("new pipeline content not present:\n%s", gotS) + } + if strings.Contains(gotS, "issuer: old") { + t.Fatalf("old pipeline content still present:\n%s", gotS) + } +} + +func TestBuildManifest_UpdatesDataField(t *testing.T) { + const newInner = `mode: proxy-sidecar +pipeline: + inbound: + - name: jwt-validation + config: + issuer: new +session: + enabled: true +` + out, err := BuildManifest([]byte(fixtureCMYAML), []byte(newInner)) + if err != nil { + t.Fatalf("BuildManifest: %v", err) + } + outS := string(out) + // metadata is preserved. + if !strings.Contains(outS, "name: authbridge-config-email-agent") { + t.Fatalf("metadata.name lost:\n%s", outS) + } + if !strings.Contains(outS, "namespace: team1") { + t.Fatalf("metadata.namespace lost:\n%s", outS) + } + // data.config.yaml carries the new content. + if !strings.Contains(outS, "issuer: new") { + t.Fatalf("new content not in manifest:\n%s", outS) + } + if strings.Contains(outS, "issuer: old") { + t.Fatalf("old content still in manifest:\n%s", outS) + } +} +``` + +### Step 2: Run tests; expect failure + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -run "TestSplice|TestBuildManifest" -v +``` +Expected: build failure — `Splice` and `BuildManifest` undefined. + +### Step 3: Write minimal implementation + +In `authbridge/cmd/abctl/edit/configmap.go`, replace `var _ = bytes.Buffer{}` with the real functions: + +```go +// Splice replaces the byte range [start, end) of innerYAML with newSubtree +// and returns the result. Used to apply the user's edit to just the pipeline +// subtree, leaving everything outside it byte-for-byte unchanged. Comments, +// blank lines, and field ordering outside the pipeline subtree all survive. +func Splice(innerYAML []byte, start, end int, newSubtree []byte) []byte { + var b bytes.Buffer + b.Grow(len(innerYAML) - (end - start) + len(newSubtree)) + b.Write(innerYAML[:start]) + b.Write(newSubtree) + b.Write(innerYAML[end:]) + return b.Bytes() +} + +// BuildManifest takes the original ConfigMap YAML manifest (as returned by +// kubectl get cm -o yaml) and a new inner runtime YAML (the contents that +// belong in data.config.yaml). Returns a manifest ready for kubectl apply. +// +// The manifest passes through yaml.v3 so the outer structure (apiVersion, +// kind, metadata, etc.) is preserved. Only data.config.yaml is replaced. +// Comments inside the inner runtime YAML survive because we set the +// data.config.yaml value to a literal block (|) string carrying newInner +// verbatim. +func BuildManifest(origCMYAML, newInner []byte) ([]byte, error) { + var root yaml.Node + if err := yaml.Unmarshal(origCMYAML, &root); err != nil { + return nil, fmt.Errorf("parse ConfigMap manifest: %w", err) + } + if root.Kind != yaml.DocumentNode || len(root.Content) == 0 { + return nil, fmt.Errorf("ConfigMap manifest is not a document") + } + doc := root.Content[0] + if doc.Kind != yaml.MappingNode { + return nil, fmt.Errorf("ConfigMap manifest root is not a mapping") + } + + // Find data → config.yaml. + var dataNode *yaml.Node + for i := 0; i < len(doc.Content); i += 2 { + if doc.Content[i].Value == "data" { + dataNode = doc.Content[i+1] + break + } + } + if dataNode == nil || dataNode.Kind != yaml.MappingNode { + return nil, fmt.Errorf("ConfigMap has no data: mapping") + } + var configValueNode *yaml.Node + for i := 0; i < len(dataNode.Content); i += 2 { + if dataNode.Content[i].Value == "config.yaml" { + configValueNode = dataNode.Content[i+1] + break + } + } + if configValueNode == nil { + return nil, fmt.Errorf("ConfigMap data has no config.yaml key") + } + + // Set the value to a literal-block scalar carrying newInner. + configValueNode.Kind = yaml.ScalarNode + configValueNode.Tag = "!!str" + configValueNode.Style = yaml.LiteralStyle + configValueNode.Value = string(newInner) + + out, err := yaml.Marshal(&root) + if err != nil { + return nil, fmt.Errorf("emit ConfigMap manifest: %w", err) + } + return out, nil +} +``` + +### Step 4: Run tests; expect PASS + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -v +go vet ./edit/ +``` +Expected: all 6 tests PASS, vet clean. + +### Step 5: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/edit/configmap.go \ + authbridge/cmd/abctl/edit/configmap_test.go +git commit -s -m "feat(abctl): Add Splice and BuildManifest helpers + +Splice does byte-range surgery to replace the pipeline subtree without +touching anything outside it (preserves comments, blank lines, field +ordering). BuildManifest parses the outer ConfigMap YAML, sets +data.config.yaml to a literal-block scalar carrying the new inner +content, and re-emits the manifest ready for kubectl apply. + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Task 4: edit/configmap.go — Fetch + Apply (kubectl integration) + +Add the `Runner` injection seam plus `Fetch` (wraps `kubectl get cm`) and `Apply` (wraps `kubectl apply --server-side`). + +**Files:** +- Modify: `authbridge/cmd/abctl/edit/configmap.go` +- Modify: `authbridge/cmd/abctl/edit/configmap_test.go` + +### Step 1: Write the failing tests + +Append to `authbridge/cmd/abctl/edit/configmap_test.go`: + +```go +import ( + "context" + "strings" + "testing" + "time" +) + +func TestFetch_HappyPath(t *testing.T) { + wantArgs := []string{ + "get", "cm", "authbridge-config-email-agent", + "-n", "team1", "-o", "yaml", + } + stub := func(ctx context.Context, args ...string) ([]byte, error) { + if !equalArgs(args, wantArgs) { + t.Fatalf("kubectl args: got %v want %v", args, wantArgs) + } + return []byte(fixtureCMYAML), nil + } + fp, err := Fetch(context.Background(), stub, "team1", "email-agent") + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if len(fp.ConfigMapYAML) == 0 { + t.Fatal("ConfigMapYAML empty") + } + if len(fp.InnerYAML) == 0 { + t.Fatal("InnerYAML empty") + } + if fp.PipelineEnd <= fp.PipelineStart { + t.Fatalf("pipeline range invalid: [%d, %d)", fp.PipelineStart, fp.PipelineEnd) + } + subtree := fp.InnerYAML[fp.PipelineStart:fp.PipelineEnd] + if !strings.Contains(string(subtree), "pipeline:") { + t.Fatalf("subtree missing header: %q", subtree) + } + if !strings.Contains(string(subtree), "jwt-validation") { + t.Fatalf("subtree missing body: %q", subtree) + } +} + +func TestFetch_KubectlError(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + return nil, fmt.Errorf("forbidden") + } + _, err := Fetch(context.Background(), stub, "team1", "email-agent") + if err == nil { + t.Fatal("want error from kubectl") + } + if !strings.Contains(err.Error(), "forbidden") { + t.Fatalf("error should surface kubectl message: %v", err) + } +} + +func TestApply_PassesManifest(t *testing.T) { + captured := make([]byte, 0) + stub := func(ctx context.Context, args ...string) ([]byte, error) { + // Args should be: apply --server-side --force-conflicts=false -f + if len(args) < 4 || args[0] != "apply" || args[1] != "--server-side" { + t.Fatalf("kubectl args: %v", args) + } + // Read the manifest the orchestrator wrote to a tempfile. + path := args[len(args)-1] + b, err := readFileForTest(path) + if err != nil { + t.Fatalf("read manifest: %v", err) + } + captured = b + return []byte("configmap/foo applied\n"), nil + } + manifest := []byte("apiVersion: v1\nkind: ConfigMap\n") + at, err := Apply(context.Background(), stub, manifest) + if err != nil { + t.Fatalf("Apply: %v", err) + } + if at.IsZero() { + t.Fatal("apply time should be set") + } + if time.Since(at) > 5*time.Second { + t.Fatalf("apply time too far in past: %v", at) + } + if string(captured) != string(manifest) { + t.Fatalf("manifest captured by stub differs from input") + } +} + +// equalArgs checks two []string for equality. +func equalArgs(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +// readFileForTest is a tiny helper so the test doesn't import os just +// to exercise Apply's tempfile path. +func readFileForTest(path string) ([]byte, error) { + return os.ReadFile(path) +} +``` + +Add `"fmt"` and `"os"` to the test file's imports if not already present. + +### Step 2: Run tests; expect failure + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -run "TestFetch|TestApply" -v +``` +Expected: build failure — `Runner`, `FetchedPipeline`, `Fetch`, `Apply` undefined. + +### Step 3: Write minimal implementation + +Append to `authbridge/cmd/abctl/edit/configmap.go`. Update its import block to include `"context"`, `"os"`, `"os/exec"`, `"strings"`, `"time"`: + +```go +// Runner abstracts a `kubectl ` invocation. Production uses os/exec; +// tests inject their own. Mirrors the Runner pattern in cmd/abctl/cluster. +type Runner func(ctx context.Context, args ...string) ([]byte, error) + +// DefaultRunner shells out to the system `kubectl`. +func DefaultRunner(ctx context.Context, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "kubectl", args...) + out, err := cmd.Output() + if err != nil { + if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 { + return nil, fmt.Errorf("kubectl: %s", strings.TrimSpace(string(ee.Stderr))) + } + return nil, fmt.Errorf("kubectl: %w", err) + } + return out, nil +} + +// FetchedPipeline is what Fetch returns: the full ConfigMap manifest, the +// inner runtime YAML extracted from data.config.yaml, and the byte range +// of the pipeline subtree within the inner YAML. +type FetchedPipeline struct { + ConfigMapYAML []byte // raw kubectl get cm -o yaml output + InnerYAML []byte // value of data.config.yaml + PipelineStart int // byte offset in InnerYAML where pipeline: begins + PipelineEnd int // byte offset where the subtree ends +} + +// Fetch reads the per-agent ConfigMap (authbridge-config-), extracts +// the inner runtime YAML from data.config.yaml, and locates the pipeline +// subtree's byte range. Returns an error if the ConfigMap doesn't exist, +// has no data.config.yaml, or has no top-level pipeline: key. +func Fetch(ctx context.Context, run Runner, namespace, agent string) (*FetchedPipeline, error) { + cmName := "authbridge-config-" + agent + cmBytes, err := run(ctx, "get", "cm", cmName, "-n", namespace, "-o", "yaml") + if err != nil { + return nil, err + } + inner, err := extractInnerYAML(cmBytes) + if err != nil { + return nil, err + } + start, end, err := FindPipelineRange(inner) + if err != nil { + return nil, err + } + return &FetchedPipeline{ + ConfigMapYAML: cmBytes, + InnerYAML: inner, + PipelineStart: start, + PipelineEnd: end, + }, nil +} + +// extractInnerYAML pulls data.config.yaml out of an outer ConfigMap manifest. +func extractInnerYAML(cmYAML []byte) ([]byte, error) { + var root yaml.Node + if err := yaml.Unmarshal(cmYAML, &root); err != nil { + return nil, fmt.Errorf("parse ConfigMap manifest: %w", err) + } + if root.Kind != yaml.DocumentNode || len(root.Content) == 0 { + return nil, fmt.Errorf("ConfigMap manifest is not a document") + } + doc := root.Content[0] + for i := 0; i+1 < len(doc.Content); i += 2 { + if doc.Content[i].Value == "data" { + dataNode := doc.Content[i+1] + for j := 0; j+1 < len(dataNode.Content); j += 2 { + if dataNode.Content[j].Value == "config.yaml" { + return []byte(dataNode.Content[j+1].Value), nil + } + } + } + } + return nil, fmt.Errorf("ConfigMap data has no config.yaml key") +} + +// Apply writes manifest to a tempfile and runs kubectl apply --server-side. +// Returns the wall-clock time at which the apply call started; the caller +// uses this to compare against /reload/status's last_success_unix to know +// whether the framework has picked up the change yet. +func Apply(ctx context.Context, run Runner, manifest []byte) (time.Time, error) { + tmp, err := os.CreateTemp("", "abctl-cm-*.yaml") + if err != nil { + return time.Time{}, fmt.Errorf("create temp manifest: %w", err) + } + defer os.Remove(tmp.Name()) + if _, err := tmp.Write(manifest); err != nil { + _ = tmp.Close() + return time.Time{}, fmt.Errorf("write temp manifest: %w", err) + } + if err := tmp.Close(); err != nil { + return time.Time{}, fmt.Errorf("close temp manifest: %w", err) + } + applyTime := time.Now() + if _, err := run(ctx, "apply", "--server-side", "--force-conflicts=false", "-f", tmp.Name()); err != nil { + return time.Time{}, err + } + return applyTime, nil +} +``` + +### Step 4: Run tests; expect PASS + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -v +go vet ./edit/ +``` +Expected: all 9 tests pass, vet clean. + +### Step 5: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/edit/configmap.go \ + authbridge/cmd/abctl/edit/configmap_test.go +git commit -s -m "feat(abctl): Add Fetch and Apply for the edit flow + +Fetch reads authbridge-config- via kubectl, extracts the inner +runtime YAML from data.config.yaml, and locates the pipeline subtree's +byte range. Apply writes a manifest to a tempfile and runs kubectl +apply --server-side --force-conflicts=false; returns the apply +timestamp for downstream comparison against /reload/status. + +All kubectl interaction goes through a Runner injection seam mirroring +cmd/abctl/cluster. + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Task 5: edit/diff.go — Line-diff renderer + +Pure-Go line-based LCS diff with lipgloss styling (green `+`, red `-`, dim context). + +**Files:** +- Create: `authbridge/cmd/abctl/edit/diff.go` +- Create: `authbridge/cmd/abctl/edit/diff_test.go` + +### Step 1: Write the failing tests + +Create `authbridge/cmd/abctl/edit/diff_test.go`: + +```go +package edit + +import ( + "strings" + "testing" +) + +func TestDiff_Equal(t *testing.T) { + got := Diff([]byte("a\nb\nc\n"), []byte("a\nb\nc\n")) + if got != "" { + t.Fatalf("equal inputs should produce empty diff, got %q", got) + } +} + +func TestDiff_OneLineChange(t *testing.T) { + old := []byte("a\nb\nc\n") + new := []byte("a\nB\nc\n") + got := Diff(old, new) + // We don't assert exact ANSI bytes (style-dependent); just check + // the line markers + content are present. + if !strings.Contains(got, "-b") { + t.Fatalf("missing -b line:\n%s", got) + } + if !strings.Contains(got, "+B") { + t.Fatalf("missing +B line:\n%s", got) + } +} + +func TestDiff_AddAndRemove(t *testing.T) { + old := []byte("a\nb\nc\n") + new := []byte("a\nc\nd\n") + got := Diff(old, new) + if !strings.Contains(got, "-b") { + t.Fatalf("missing -b:\n%s", got) + } + if !strings.Contains(got, "+d") { + t.Fatalf("missing +d:\n%s", got) + } +} + +func TestDiff_PreservesContext(t *testing.T) { + old := []byte("line1\nline2\nline3\nline4\n") + new := []byte("line1\nline2\nLINE3\nline4\n") + got := Diff(old, new) + // Both unchanged lines should appear (as context). + if !strings.Contains(got, "line1") || !strings.Contains(got, "line4") { + t.Fatalf("missing context lines:\n%s", got) + } + if !strings.Contains(got, "-line3") || !strings.Contains(got, "+LINE3") { + t.Fatalf("missing change markers:\n%s", got) + } +} +``` + +### Step 2: Run tests; expect failure + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -run TestDiff -v +``` +Expected: build failure — `Diff` undefined. + +### Step 3: Write minimal implementation + +Create `authbridge/cmd/abctl/edit/diff.go`: + +```go +package edit + +import ( + "strings" + + "github.com/charmbracelet/lipgloss" +) + +var ( + diffStyleAdd = lipgloss.NewStyle().Foreground(lipgloss.Color("2")) // green + diffStyleRemove = lipgloss.NewStyle().Foreground(lipgloss.Color("1")) // red + diffStyleContext = lipgloss.NewStyle().Faint(true) +) + +// Diff renders a line-based diff of old vs new with lipgloss styling. +// Returns the empty string when inputs are byte-equal. The algorithm is +// LCS-on-lines; output is one rendered line per diff entry, terminated +// by newlines. +// +// LCS is O(N*M) in the line count; pipeline subtrees are typically <50 +// lines so the cost is negligible. The diff is intended for the abctl +// edit confirmation overlay — it shows the user every line that +// changed, in original order, with two lines of context emit suppressed +// for compactness (every line is either ±something or context). +func Diff(old, new []byte) string { + if string(old) == string(new) { + return "" + } + oldLines := splitLinesKeepNewline(old) + newLines := splitLinesKeepNewline(new) + ops := lcsDiff(oldLines, newLines) + var b strings.Builder + for _, op := range ops { + switch op.kind { + case opEqual: + b.WriteString(diffStyleContext.Render(" " + strings.TrimRight(op.line, "\n"))) + b.WriteByte('\n') + case opRemove: + b.WriteString(diffStyleRemove.Render("-" + strings.TrimRight(op.line, "\n"))) + b.WriteByte('\n') + case opAdd: + b.WriteString(diffStyleAdd.Render("+" + strings.TrimRight(op.line, "\n"))) + b.WriteByte('\n') + } + } + return b.String() +} + +func splitLinesKeepNewline(b []byte) []string { + if len(b) == 0 { + return nil + } + s := string(b) + out := strings.SplitAfter(s, "\n") + // strings.SplitAfter on "a\n" returns ["a\n", ""] — drop trailing empty. + if len(out) > 0 && out[len(out)-1] == "" { + out = out[:len(out)-1] + } + return out +} + +type opKind int + +const ( + opEqual opKind = iota + opRemove + opAdd +) + +type diffOp struct { + kind opKind + line string +} + +// lcsDiff is a textbook LCS-then-backtrack implementation. Returns the +// edit script as a sequence of equal/remove/add ops in order. +func lcsDiff(old, new []string) []diffOp { + m, n := len(old), len(new) + if m == 0 { + out := make([]diffOp, n) + for i, l := range new { + out[i] = diffOp{opAdd, l} + } + return out + } + if n == 0 { + out := make([]diffOp, m) + for i, l := range old { + out[i] = diffOp{opRemove, l} + } + return out + } + // dp[i][j] = LCS length of old[:i] vs new[:j] + dp := make([][]int, m+1) + for i := range dp { + dp[i] = make([]int, n+1) + } + for i := 1; i <= m; i++ { + for j := 1; j <= n; j++ { + if old[i-1] == new[j-1] { + dp[i][j] = dp[i-1][j-1] + 1 + } else if dp[i-1][j] >= dp[i][j-1] { + dp[i][j] = dp[i-1][j] + } else { + dp[i][j] = dp[i][j-1] + } + } + } + var rev []diffOp + i, j := m, n + for i > 0 && j > 0 { + switch { + case old[i-1] == new[j-1]: + rev = append(rev, diffOp{opEqual, old[i-1]}) + i-- + j-- + case dp[i-1][j] >= dp[i][j-1]: + rev = append(rev, diffOp{opRemove, old[i-1]}) + i-- + default: + rev = append(rev, diffOp{opAdd, new[j-1]}) + j-- + } + } + for i > 0 { + rev = append(rev, diffOp{opRemove, old[i-1]}) + i-- + } + for j > 0 { + rev = append(rev, diffOp{opAdd, new[j-1]}) + j-- + } + // Reverse. + out := make([]diffOp, len(rev)) + for k, op := range rev { + out[len(rev)-1-k] = op + } + return out +} +``` + +### Step 4: Run tests; expect PASS + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -v +go vet ./edit/ +``` +Expected: all 13 tests pass. + +### Step 5: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/edit/diff.go \ + authbridge/cmd/abctl/edit/diff_test.go +git commit -s -m "feat(abctl): Add line-diff renderer for edit confirmation + +LCS-on-lines with lipgloss styling (green +, red -, dim context). +Returns empty string for byte-equal inputs. Used by the edit overlay's +confirm prompt to show the user what they're about to apply. + +Hand-rolled rather than pulling a diff library; pipeline subtrees are +typically <50 lines so even quadratic LCS is trivially fast. + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Task 6: edit/status.go — Poll /reload/status + +Reload-status poller. Watches `last_success_unix` and `reloads_failed_total` to detect success or failure of the framework's reload after the user's apply. 1s cadence, 120s timeout. + +**Files:** +- Create: `authbridge/cmd/abctl/edit/status.go` +- Create: `authbridge/cmd/abctl/edit/status_test.go` + +### Step 1: Write the failing tests + +Create `authbridge/cmd/abctl/edit/status_test.go`: + +```go +package edit + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" +) + +// makeStatusServer returns a httptest.Server whose /reload/status +// endpoint reads its body from the supplied function — letting the +// test advance state between polls. +func makeStatusServer(t *testing.T, body func() ReloadStatus) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/reload/status" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(body()) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestPollUntilReloaded_Success(t *testing.T) { + applyTime := time.Now() + var calls atomic.Int32 + srv := makeStatusServer(t, func() ReloadStatus { + c := calls.Add(1) + // First call: no successful reload yet. Second call: success. + if c == 1 { + return ReloadStatus{LastSuccessUnix: applyTime.Add(-1 * time.Hour).Unix()} + } + return ReloadStatus{LastSuccessUnix: time.Now().Unix()} + }) + res := PollUntilReloaded(context.Background(), srv.URL, applyTime) + if res.Status != PollSuccess { + t.Fatalf("status = %v, want PollSuccess", res.Status) + } +} + +func TestPollUntilReloaded_Failure(t *testing.T) { + applyTime := time.Now() + var calls atomic.Int32 + srv := makeStatusServer(t, func() ReloadStatus { + c := calls.Add(1) + // Pre-apply baseline on first call (ReloadsFailed = 5). + // Increment on second call. + if c == 1 { + return ReloadStatus{ReloadsFailed: 5} + } + return ReloadStatus{ReloadsFailed: 6, LastError: "invalid YAML at line 3"} + }) + res := PollUntilReloaded(context.Background(), srv.URL, applyTime) + if res.Status != PollFailure { + t.Fatalf("status = %v, want PollFailure", res.Status) + } + if res.LastError != "invalid YAML at line 3" { + t.Fatalf("LastError: %q", res.LastError) + } +} + +func TestPollUntilReloaded_Timeout(t *testing.T) { + applyTime := time.Now() + srv := makeStatusServer(t, func() ReloadStatus { + // Never advances; timeout path. + return ReloadStatus{LastSuccessUnix: applyTime.Add(-1 * time.Hour).Unix()} + }) + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + // Override the package's poll deadline by using ctx — see implementation. + res := PollUntilReloaded(ctx, srv.URL, applyTime) + if res.Status != PollTimeout { + t.Fatalf("status = %v, want PollTimeout", res.Status) + } +} + +func TestPollUntilReloaded_HTTPError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "internal", http.StatusInternalServerError) + })) + defer srv.Close() + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + res := PollUntilReloaded(ctx, srv.URL, time.Now()) + // On persistent errors we time out. (Transient errors are retried.) + if res.Status != PollTimeout { + t.Fatalf("status = %v, want PollTimeout", res.Status) + } +} + +// no-op: keep the imports list honest if this file ever drops fmt +var _ = fmt.Sprintf +``` + +### Step 2: Run tests; expect failure + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -run TestPollUntilReloaded -v +``` +Expected: build failure — `ReloadStatus`, `PollUntilReloaded`, `PollSuccess`/`PollFailure`/`PollTimeout` undefined. + +### Step 3: Write minimal implementation + +Create `authbridge/cmd/abctl/edit/status.go`: + +```go +package edit + +import ( + "context" + "encoding/json" + "net/http" + "time" +) + +// ReloadStatus is the wire shape of the framework's /reload/status endpoint. +// Only the fields abctl uses are decoded. +type ReloadStatus struct { + LastSuccessUnix int64 `json:"last_success_unix"` + ReloadsOK uint64 `json:"reloads_ok"` + ReloadsFailed uint64 `json:"reloads_failed"` + LastError string `json:"last_error"` +} + +// PollResultStatus is a sum type for PollUntilReloaded outcomes. +type PollResultStatus int + +const ( + PollUnknown PollResultStatus = iota + PollSuccess + PollFailure + PollTimeout +) + +// PollResult is what PollUntilReloaded returns. +type PollResult struct { + Status PollResultStatus + LastError string // populated when Status == PollFailure +} + +// pollInterval is the cadence between /reload/status fetches. 1s balances +// user-visible spinner progress with not hammering the cluster on slow +// reloads. +const pollInterval = 1 * time.Second + +// PollUntilReloaded watches statusURL/reload/status until either: +// - LastSuccessUnix > applyTime.Unix() → PollSuccess. +// - ReloadsFailed exceeds the value at first successful poll → PollFailure +// with LastError populated. +// - ctx is done → PollTimeout. (Caller is expected to set a 120s timeout +// via context.WithTimeout.) +// +// HTTP errors (network, non-200) are retried until ctx expires; we treat +// them as "framework not yet reachable, keep waiting." +func PollUntilReloaded(ctx context.Context, statusURL string, applyTime time.Time) PollResult { + url := statusURL + "/reload/status" + client := &http.Client{Timeout: 2 * time.Second} + + var baselineFailed uint64 + first := true + + for { + select { + case <-ctx.Done(): + return PollResult{Status: PollTimeout} + default: + } + + req, err := http.NewRequestWithContext(ctx, "GET", url, nil) + if err != nil { + return PollResult{Status: PollTimeout} // ctx-canceled mid-build + } + resp, err := client.Do(req) + if err == nil && resp.StatusCode == 200 { + var rs ReloadStatus + decodeErr := json.NewDecoder(resp.Body).Decode(&rs) + resp.Body.Close() + if decodeErr == nil { + if first { + baselineFailed = rs.ReloadsFailed + first = false + } + if rs.LastSuccessUnix > applyTime.Unix() { + return PollResult{Status: PollSuccess} + } + if rs.ReloadsFailed > baselineFailed { + return PollResult{Status: PollFailure, LastError: rs.LastError} + } + } + } else if resp != nil { + resp.Body.Close() + } + + select { + case <-ctx.Done(): + return PollResult{Status: PollTimeout} + case <-time.After(pollInterval): + } + } +} +``` + +### Step 4: Run tests; expect PASS + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -v +go vet ./edit/ +``` +Expected: all 17 tests pass, vet clean. + +### Step 5: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/edit/status.go \ + authbridge/cmd/abctl/edit/status_test.go +git commit -s -m "feat(abctl): Poll /reload/status to detect framework reload + +Watches last_success_unix advancing past applyTime (success) and +reloads_failed incrementing past the pre-apply baseline (failure with +the framework's error message). 1s cadence. Caller sets the 120s +timeout via context.WithTimeout. + +Transient HTTP errors retry until ctx expires. + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Task 7: tui/edit_overlay.go — State + render + +Overlay state machine + per-phase render functions. No Cmd factories yet (Task 8); no Update wiring yet (Task 9). This task purely defines the shape. + +**Files:** +- Create: `authbridge/cmd/abctl/tui/edit_overlay.go` +- Create: `authbridge/cmd/abctl/tui/edit_overlay_test.go` + +### Step 1: Write the failing tests + +Create `authbridge/cmd/abctl/tui/edit_overlay_test.go`: + +```go +package tui + +import ( + "strings" + "testing" +) + +func TestEditOverlayRender_Fetching(t *testing.T) { + s := editState{phase: editPhaseFetching} + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "Fetching ConfigMap") { + t.Fatalf("fetching phase missing message:\n%s", out) + } +} + +func TestEditOverlayRender_Diff(t *testing.T) { + s := editState{ + phase: editPhaseDiff, + diff: "-old line\n+new line\n", + } + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "old line") || !strings.Contains(out, "new line") { + t.Fatalf("diff content missing:\n%s", out) + } + if !strings.Contains(out, "(y/N)") { + t.Fatalf("confirm prompt missing:\n%s", out) + } +} + +func TestEditOverlayRender_Applying(t *testing.T) { + s := editState{phase: editPhaseApplying} + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "Applying") { + t.Fatalf("applying phase missing message:\n%s", out) + } +} + +func TestEditOverlayRender_Waiting(t *testing.T) { + s := editState{phase: editPhaseWaiting} + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "reload") { + t.Fatalf("waiting phase should mention reload:\n%s", out) + } +} + +func TestEditOverlayRender_Error(t *testing.T) { + s := editState{phase: editPhaseError, err: "kubectl: forbidden"} + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "forbidden") { + t.Fatalf("error message not surfaced:\n%s", out) + } +} +``` + +### Step 2: Run tests; expect failure + +```bash +cd authbridge/cmd/abctl +go test ./tui/ -run TestEditOverlayRender -v +``` +Expected: build failure — `editState`, `editPhase*`, `renderEditOverlay` undefined. + +### Step 3: Write minimal implementation + +Create `authbridge/cmd/abctl/tui/edit_overlay.go`: + +```go +package tui + +import ( + "fmt" + "strings" + "time" + + "github.com/charmbracelet/lipgloss" + + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" +) + +// editPhase tracks where the edit state machine currently sits. +type editPhase int + +const ( + editPhaseDone editPhase = iota // not editing + editPhaseFetching + editPhaseEditing // $EDITOR is running; bubbletea is suspended + editPhaseValidating + editPhaseDiff + editPhaseApplying + editPhaseWaiting + editPhaseError +) + +// editState lives on *model when an edit is in flight. +type editState struct { + phase editPhase + fetched *edit.FetchedPipeline + tempPath string + editedRaw []byte // bytes the user wrote in $EDITOR + diff string // colorized output from edit.Diff + err string // single-line message in editPhaseError + applyTime time.Time +} + +// renderEditOverlay returns the overlay content (rendered into a +// styled box) for the current edit phase. width/height are the +// terminal's full dimensions; the overlay sizes itself to fit +// comfortably inside. +func renderEditOverlay(s editState, width, height int) string { + box := lipgloss.NewStyle(). + Border(lipgloss.RoundedBorder()). + Padding(1, 2). + Width(min(width-4, 100)) + + var b strings.Builder + switch s.phase { + case editPhaseFetching: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString("Fetching ConfigMap…") + case editPhaseEditing: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString(fmt.Sprintf("Editor open at %s", s.tempPath)) + case editPhaseValidating: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString("Validating YAML…") + case editPhaseDiff: + b.WriteString(styleTitle.Render("Edit pipeline — review diff")) + b.WriteString("\n\n") + b.WriteString(s.diff) + b.WriteString("\n") + b.WriteString(styleHint.Render("apply this change? (y/N)")) + case editPhaseApplying: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString("Applying to ConfigMap…") + case editPhaseWaiting: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString("Waiting for hot-reload…") + b.WriteString("\n") + b.WriteString(styleHint.Render("(this can take up to 120s while kubelet syncs the ConfigMap)")) + case editPhaseError: + b.WriteString(styleTitle.Render("Edit pipeline — error")) + b.WriteString("\n\n") + b.WriteString(s.err) + b.WriteString("\n\n") + b.WriteString(styleHint.Render("[r] re-edit [Esc] back to Pipeline")) + } + return box.Render(b.String()) +} + +func min(a, b int) int { + if a < b { + return a + } + return b +} +``` + +### Step 4: Run tests; expect PASS + +```bash +cd authbridge/cmd/abctl +go test ./tui/ -v +go vet ./tui/ +``` +Expected: 5 new tests pass, all existing TUI tests still pass. + +### Step 5: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/tui/edit_overlay.go \ + authbridge/cmd/abctl/tui/edit_overlay_test.go +git commit -s -m "feat(abctl): Add edit overlay state + render + +editPhase enum, editState struct, renderEditOverlay function for each +phase (fetching/editing/validating/diff/applying/waiting/error). No +state-machine wiring yet (Task 8 adds Cmd factories, Task 9 wires +Update handlers + the e keybind). + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Task 8: edit/edit.go — tea.Cmd factories + +Wrap each phase's work in a `tea.Cmd` factory that produces the corresponding `tea.Msg`. The factories live in the `edit` package alongside their implementation; the TUI Update handlers (Task 9) just dispatch them. + +**Files:** +- Create: `authbridge/cmd/abctl/edit/edit.go` +- Create: `authbridge/cmd/abctl/edit/edit_test.go` + +### Step 1: Write the failing tests + +Create `authbridge/cmd/abctl/edit/edit_test.go`: + +```go +package edit + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" +) + +func TestFetchCmd_Success(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + return []byte(fixtureCMYAML), nil + } + cmd := FetchCmd(context.Background(), stub, "team1", "email-agent") + msg := cmd().(FetchedMsg) + if msg.Err != nil { + t.Fatalf("FetchedMsg.Err = %v", msg.Err) + } + if msg.Fetched == nil { + t.Fatal("FetchedMsg.Fetched is nil") + } + if msg.TempPath == "" { + t.Fatal("FetchedMsg.TempPath empty (should be the path to the subtree-only tempfile)") + } +} + +func TestFetchCmd_Error(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + return nil, fmt.Errorf("forbidden") + } + cmd := FetchCmd(context.Background(), stub, "team1", "email-agent") + msg := cmd().(FetchedMsg) + if msg.Err == nil { + t.Fatal("expected error") + } + if !strings.Contains(msg.Err.Error(), "forbidden") { + t.Fatalf("error: %v", msg.Err) + } +} + +func TestApplyCmd_Success(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + return []byte("applied"), nil + } + cmd := ApplyCmd(context.Background(), stub, []byte("manifest")) + msg := cmd().(AppliedMsg) + if msg.Err != nil { + t.Fatalf("err = %v", msg.Err) + } + if msg.ApplyTime.IsZero() { + t.Fatal("ApplyTime not set") + } +} + +func TestPollCmd_Success(t *testing.T) { + applyTime := time.Now() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + // Always report last_success after applyTime. + _, _ = w.Write([]byte(`{"last_success_unix":99999999999}`)) + })) + defer srv.Close() + cmd := PollCmd(context.Background(), srv.URL, applyTime) + msg := cmd().(PolledMsg) + if msg.Result.Status != PollSuccess { + t.Fatalf("status: %v", msg.Result.Status) + } +} + +// Compile-time guard: each factory returns a tea.Cmd. +var ( + _ tea.Cmd = FetchCmd(context.Background(), nil, "", "") + _ tea.Cmd = ApplyCmd(context.Background(), nil, nil) + _ tea.Cmd = PollCmd(context.Background(), "", time.Time{}) +) +``` + +Add `"fmt"` to the test file's imports if not already present. + +### Step 2: Run tests; expect failure + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -run "TestFetchCmd|TestApplyCmd|TestPollCmd" -v +``` +Expected: build failure — `FetchCmd`, `FetchedMsg`, `ApplyCmd`, `AppliedMsg`, `PollCmd`, `PolledMsg` undefined. + +### Step 3: Write minimal implementation + +Create `authbridge/cmd/abctl/edit/edit.go`: + +```go +package edit + +import ( + "context" + "os" + "time" + + tea "github.com/charmbracelet/bubbletea" +) + +// FetchedMsg is the result of FetchCmd. On success: Fetched and TempPath +// are both set, Err is nil. On failure: Err is populated, others are zero. +type FetchedMsg struct { + Fetched *FetchedPipeline + TempPath string // path to the tempfile holding just the pipeline subtree + Err error +} + +// FetchCmd returns a tea.Cmd that fetches the agent's ConfigMap, locates +// the pipeline subtree, writes the subtree to a tempfile (ready for +// $EDITOR), and emits FetchedMsg. The tempfile lives in $TMPDIR; abctl +// leaves it in place on every exit path (success, error, abort) so users +// can recover an in-progress edit. +func FetchCmd(ctx context.Context, run Runner, namespace, agent string) tea.Cmd { + return func() tea.Msg { + fp, err := Fetch(ctx, run, namespace, agent) + if err != nil { + return FetchedMsg{Err: err} + } + tmp, err := os.CreateTemp("", "abctl-pipeline-*.yaml") + if err != nil { + return FetchedMsg{Err: err} + } + subtree := fp.InnerYAML[fp.PipelineStart:fp.PipelineEnd] + if _, err := tmp.Write(subtree); err != nil { + tmp.Close() + return FetchedMsg{Err: err} + } + path := tmp.Name() + if err := tmp.Close(); err != nil { + return FetchedMsg{Err: err} + } + return FetchedMsg{Fetched: fp, TempPath: path} + } +} + +// AppliedMsg is the result of ApplyCmd. +type AppliedMsg struct { + ApplyTime time.Time + Err error +} + +// ApplyCmd returns a tea.Cmd that runs kubectl apply --server-side on +// the supplied manifest and emits AppliedMsg with the apply timestamp. +func ApplyCmd(ctx context.Context, run Runner, manifest []byte) tea.Cmd { + return func() tea.Msg { + at, err := Apply(ctx, run, manifest) + return AppliedMsg{ApplyTime: at, Err: err} + } +} + +// PolledMsg is the result of PollCmd. +type PolledMsg struct { + Result PollResult +} + +// PollCmd returns a tea.Cmd that polls /reload/status until the framework +// reload completes (success or failure) or ctx expires. Emits PolledMsg. +// +// Caller should construct ctx with a 120s WithTimeout so the poll +// terminates if kubelet doesn't sync within a reasonable window. +func PollCmd(ctx context.Context, statusURL string, applyTime time.Time) tea.Cmd { + return func() tea.Msg { + return PolledMsg{Result: PollUntilReloaded(ctx, statusURL, applyTime)} + } +} +``` + +### Step 4: Run tests; expect PASS + +```bash +cd authbridge/cmd/abctl +go test ./edit/ -v +go vet ./edit/ +``` +Expected: all tests pass, vet clean. + +### Step 5: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/edit/edit.go \ + authbridge/cmd/abctl/edit/edit_test.go +git commit -s -m "feat(abctl): Add tea.Cmd factories for the edit phases + +FetchCmd / ApplyCmd / PollCmd wrap the existing Fetch/Apply/PollUntilReloaded +implementations in tea.Cmd shapes; each emits its corresponding tea.Msg +(FetchedMsg/AppliedMsg/PolledMsg). Tested with stub Runners and a fake +status server. + +The validate + diff + edit phases don't need separate Cmd factories — +they're synchronous, small, and live inside the TUI Update handlers +(Task 9). + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Task 9: TUI integration — Update wiring + "e" keybind + end-to-end + +This is the biggest task: wire the state machine into `tui/app.go`, add the `e` keybind on `panePipeline`, render the overlay when `m.editState.phase != editPhaseDone`, and add an end-to-end test that exercises the full flow with stubs. + +**Files:** +- Modify: `authbridge/cmd/abctl/tui/app.go` — add `editState` field, `View` overlay branch, Update handlers. +- Modify: `authbridge/cmd/abctl/tui/keys.go` — `e` keybind on `panePipeline`. +- Modify: `authbridge/cmd/abctl/tui/picker_test.go` (extend existing tests if any reference the model shape). +- Create: `authbridge/cmd/abctl/tui/edit_e2e_test.go` — full state-machine drive with stubs. + +### Step 1: Write the failing end-to-end test + +Create `authbridge/cmd/abctl/tui/edit_e2e_test.go`: + +```go +package tui + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" +) + +const fixtureCMYAML = `apiVersion: v1 +kind: ConfigMap +metadata: + name: authbridge-config-email-agent + namespace: team1 +data: + config.yaml: | + mode: proxy-sidecar + pipeline: + inbound: + - name: jwt-validation + session: + enabled: true +` + +// fakeRunner records args + returns canned responses. +type fakeRunner struct { + getResponse []byte + applyError error + captured []string // args of each call, joined by " " + applyManifest []byte +} + +func (f *fakeRunner) run(ctx context.Context, args ...string) ([]byte, error) { + f.captured = append(f.captured, strings.Join(args, " ")) + if len(args) > 0 && args[0] == "get" { + return f.getResponse, nil + } + if len(args) > 0 && args[0] == "apply" { + // Read the manifest the orchestrator wrote. + path := args[len(args)-1] + b, err := os.ReadFile(path) + if err != nil { + return nil, err + } + f.applyManifest = b + if f.applyError != nil { + return nil, f.applyError + } + return []byte("applied"), nil + } + return nil, nil +} + +// TestEditFlow_HappyPath drives the full state machine: e → fetch → (skip +// editor) → validate → diff → apply → poll → done. +// +// We bypass the editor by writing the "edited" content to the tempfile +// directly, then injecting the editorExitedMsg with err=nil. +func TestEditFlow_HappyPath(t *testing.T) { + runner := &fakeRunner{getResponse: []byte(fixtureCMYAML)} + + // Stub /reload/status to report success on every poll. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "last_success_unix": 99999999999, + }) + })) + defer srv.Close() + + m := newPickerModel(context.Background(), nil, nil) + m.statusURL = srv.URL + m.editRunner = runner.run + m.selectedNamespace = "team1" + m.selectedPod = "email-agent" + m.pane = panePipeline + + // Press "e". + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) + mm := updated.(*model) + if mm.editState.phase != editPhaseFetching { + t.Fatalf("phase = %v, want editPhaseFetching", mm.editState.phase) + } + + // Run FetchCmd. + fetchedMsg := cmd().(edit.FetchedMsg) + if fetchedMsg.Err != nil { + t.Fatalf("Fetch failed: %v", fetchedMsg.Err) + } + + // Bypass the editor: write a modified subtree to the tempfile + // directly, then deliver editorExitedMsg. + editedSubtree := []byte(`pipeline: + inbound: + - name: jwt-validation + config: + new_key: new_value +`) + if err := os.WriteFile(fetchedMsg.TempPath, editedSubtree, 0o600); err != nil { + t.Fatal(err) + } + defer os.Remove(fetchedMsg.TempPath) + + updated, _ = mm.Update(fetchedMsg) + mm = updated.(*model) + if mm.editState.phase != editPhaseEditing { + t.Fatalf("phase = %v, want editPhaseEditing", mm.editState.phase) + } + + // Skip past the ExecProcess (we'd normally suspend bubbletea here). + updated, _ = mm.Update(editorExitedMsg{err: nil}) + mm = updated.(*model) + if mm.editState.phase != editPhaseDiff { + t.Fatalf("phase = %v, want editPhaseDiff (validate should pass)", mm.editState.phase) + } + if mm.editState.diff == "" { + t.Fatal("diff should be populated") + } + + // Press "y" to confirm. + updated, cmd = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'y'}}) + mm = updated.(*model) + if mm.editState.phase != editPhaseApplying { + t.Fatalf("phase = %v, want editPhaseApplying", mm.editState.phase) + } + + // Run ApplyCmd. + appliedMsg := cmd().(edit.AppliedMsg) + if appliedMsg.Err != nil { + t.Fatalf("apply failed: %v", appliedMsg.Err) + } + + updated, cmd = mm.Update(appliedMsg) + mm = updated.(*model) + if mm.editState.phase != editPhaseWaiting { + t.Fatalf("phase = %v, want editPhaseWaiting", mm.editState.phase) + } + + // Run PollCmd. + polledMsg := cmd().(edit.PolledMsg) + if polledMsg.Result.Status != edit.PollSuccess { + t.Fatalf("poll status = %v, want PollSuccess", polledMsg.Result.Status) + } + + updated, _ = mm.Update(polledMsg) + mm = updated.(*model) + if mm.editState.phase != editPhaseDone { + t.Fatalf("phase = %v, want editPhaseDone", mm.editState.phase) + } + + // The applied manifest should contain the new content. + if !strings.Contains(string(runner.applyManifest), "new_key: new_value") { + t.Fatalf("manifest missing new content:\n%s", runner.applyManifest) + } +} + +// TestEditFlow_NCancelsAtDiff verifies "N" at the confirm prompt +// returns to panePipeline without applying. +func TestEditFlow_NCancelsAtDiff(t *testing.T) { + runner := &fakeRunner{getResponse: []byte(fixtureCMYAML)} + m := newPickerModel(context.Background(), nil, nil) + m.editRunner = runner.run + m.selectedNamespace = "team1" + m.selectedPod = "email-agent" + m.pane = panePipeline + + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) + fetchedMsg := cmd().(edit.FetchedMsg) + mm := updated.(*model).Update(fetchedMsg) + mm2 := mm.(*model) + + // Pretend the user edited. + editedSubtree := []byte("pipeline:\n inbound:\n - name: jwt-validation\n config: {x: 1}\n") + _ = os.WriteFile(fetchedMsg.TempPath, editedSubtree, 0o600) + defer os.Remove(fetchedMsg.TempPath) + + updated, _ = mm2.Update(editorExitedMsg{err: nil}) + mm2 = updated.(*model) + + // Press "N". + updated, _ = mm2.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'N'}}) + mm2 = updated.(*model) + if mm2.editState.phase != editPhaseDone { + t.Fatalf("phase = %v, want editPhaseDone (N should cancel)", mm2.editState.phase) + } + // No apply should have run. + for _, c := range runner.captured { + if strings.HasPrefix(c, "apply") { + t.Fatalf("apply ran despite N: %q", c) + } + } + _ = filepath.Base(fetchedMsg.TempPath) // silence unused +} +``` + +### Step 2: Run tests; expect failure + +```bash +cd authbridge/cmd/abctl +go test ./tui/ -run TestEditFlow -v +``` +Expected: build failure — `m.statusURL`, `m.editRunner`, `editorExitedMsg`, the e-keybind handler, etc. all undefined. + +### Step 3: Wire the model + Update + keybind + +In `authbridge/cmd/abctl/tui/app.go`, add fields to `*model`: + +```go +// Edit state. editState.phase == editPhaseDone means "no edit in flight" +// and the rest of the fields are zero. statusURL is set by the picker +// when port-forward succeeds (PortForward.StatusEndpoint()). editRunner +// is the kubectl Runner for the edit package (set in main.go to +// edit.DefaultRunner; tests inject a stub). +editState editState +statusURL string +editRunner edit.Runner +``` + +Add to the imports: +```go +"github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" +``` + +Add new `tea.Msg` types near the others: + +```go +type editorExitedMsg struct{ err error } +``` + +(`edit.FetchedMsg`, `edit.AppliedMsg`, `edit.PolledMsg` are imported types.) + +Wire up `Update` cases. Find the type switch on `msg` in `Update()` and add: + +```go +case edit.FetchedMsg: + if msg.Err != nil { + m.editState.phase = editPhaseError + m.editState.err = msg.Err.Error() + return m, nil + } + m.editState.fetched = msg.Fetched + m.editState.tempPath = msg.TempPath + m.editState.phase = editPhaseEditing + return m, openEditorCmd(msg.TempPath) + +case editorExitedMsg: + if msg.err != nil { + m.editState.phase = editPhaseError + m.editState.err = "editor exited: " + msg.err.Error() + return m, nil + } + // Read the edited bytes; validate as YAML; compute diff. + edited, err := os.ReadFile(m.editState.tempPath) + if err != nil { + m.editState.phase = editPhaseError + m.editState.err = "read edited file: " + err.Error() + return m, nil + } + m.editState.editedRaw = edited + originalSubtree := m.editState.fetched.InnerYAML[m.editState.fetched.PipelineStart:m.editState.fetched.PipelineEnd] + if string(edited) == string(originalSubtree) { + // No changes — return to panePipeline. + m.editState = editState{phase: editPhaseDone} + return m, nil + } + if !validYAML(edited) { + m.editState.phase = editPhaseError + m.editState.err = "edited file is not valid YAML" + return m, nil + } + m.editState.diff = edit.Diff(originalSubtree, edited) + m.editState.phase = editPhaseDiff + return m, nil + +case edit.AppliedMsg: + if msg.Err != nil { + m.editState.phase = editPhaseError + m.editState.err = "apply failed: " + msg.Err.Error() + return m, nil + } + m.editState.applyTime = msg.ApplyTime + m.editState.phase = editPhaseWaiting + return m, edit.PollCmd(m.ctx, m.statusURL, msg.ApplyTime) + +case edit.PolledMsg: + switch msg.Result.Status { + case edit.PollSuccess: + m.editState = editState{phase: editPhaseDone} + // Refresh /v1/pipeline so the pane shows the new state. + return m, m.loadPipelineCmd() + case edit.PollFailure: + m.editState.phase = editPhaseError + m.editState.err = "reload failed: " + msg.Result.LastError + return m, nil + case edit.PollTimeout: + m.editState.phase = editPhaseError + m.editState.err = "reload not observed in 120s; check kubectl logs" + return m, nil + } +``` + +Add helpers near the bottom of the file: + +```go +// openEditorCmd returns a tea.Cmd that suspends bubbletea, runs $EDITOR +// (vi if unset) on path, and emits editorExitedMsg when the editor exits. +func openEditorCmd(path string) tea.Cmd { + editor := os.Getenv("EDITOR") + if editor == "" { + editor = "vi" + } + c := exec.Command("sh", "-c", editor+" "+path) + return tea.ExecProcess(c, func(err error) tea.Msg { + return editorExitedMsg{err: err} + }) +} + +// validYAML returns true iff b parses as YAML. +func validYAML(b []byte) bool { + var v any + return yaml.Unmarshal(b, &v) == nil +} +``` + +Add to imports: `"os"`, `"os/exec"`, `"gopkg.in/yaml.v3"`. + +Update `View()` to render the overlay when an edit is in flight. Find the existing view function and add this near the start: + +```go +// Edit overlay takes over the screen while an edit is in flight. +if m.editState.phase != editPhaseDone { + return renderEditOverlay(m.editState, m.width, m.height) +} +``` + +In `authbridge/cmd/abctl/tui/keys.go`, add the `e` keybind to the `panePipeline` arm of the key switch. Also add the `y`/`N`/`r`/`Esc` handlers for the edit overlay phases: + +```go +// Edit overlay takes over key input while editing is in flight. +if m.editState.phase != editPhaseDone { + return m.handleEditKey(msg) +} +``` + +Then define `handleEditKey` in `keys.go`: + +```go +// handleEditKey is the keymap that takes over while an edit is in flight. +func (m *model) handleEditKey(msg tea.KeyMsg) tea.Cmd { + switch m.editState.phase { + case editPhaseDiff: + switch msg.String() { + case "y", "Y": + m.editState.phase = editPhaseApplying + newSubtree := m.editState.editedRaw + newInner := edit.Splice( + m.editState.fetched.InnerYAML, + m.editState.fetched.PipelineStart, + m.editState.fetched.PipelineEnd, + newSubtree, + ) + manifest, err := edit.BuildManifest(m.editState.fetched.ConfigMapYAML, newInner) + if err != nil { + m.editState.phase = editPhaseError + m.editState.err = "build manifest: " + err.Error() + return nil + } + return edit.ApplyCmd(m.ctx, m.editRunner, manifest) + case "n", "N", "esc": + m.editState = editState{phase: editPhaseDone} + return nil + } + return nil + case editPhaseError: + switch msg.String() { + case "r": + // Re-open editor with the same tempfile. + m.editState.phase = editPhaseEditing + return openEditorCmd(m.editState.tempPath) + case "esc": + m.editState = editState{phase: editPhaseDone} + return nil + } + return nil + } + // Other phases: only Esc cancels (best-effort; in-flight Cmds keep running but their msgs are dropped by the editPhaseDone gate). + if msg.String() == "esc" { + m.editState = editState{phase: editPhaseDone} + return nil + } + return nil +} +``` + +In `keys.go`, in the `panePipeline` arm of the regular key switch, add: + +```go +case "e": + if m.editState.phase != editPhaseDone { + return nil // already editing + } + m.editState = editState{phase: editPhaseFetching} + return edit.FetchCmd(m.ctx, m.editRunner, m.selectedNamespace, m.selectedPod) +``` + +In `main.go`, wire the production runner + statusURL after the picker spawns the port-forward. Add to the model setup (or where the apiclient gets set): + +```go +m.editRunner = edit.DefaultRunner +m.statusURL = portForward.StatusEndpoint() +``` + +(The exact splice depends on existing main.go shape. Read the file before patching.) + +### Step 4: Run tests; expect PASS + +```bash +cd authbridge/cmd/abctl +go test ./tui/ -v +go vet ./tui/ +go build ./... +``` +Expected: all tests pass, vet clean, binary builds. + +### Step 5: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/tui/app.go \ + authbridge/cmd/abctl/tui/keys.go \ + authbridge/cmd/abctl/tui/edit_e2e_test.go \ + authbridge/cmd/abctl/main.go +git commit -s -m "feat(abctl): Wire edit state machine + e keybind + +Adds editState/statusURL/editRunner fields on *model. Update handlers +for the four msgs (FetchedMsg, editorExitedMsg, AppliedMsg, PolledMsg) +advance the state machine. View() overlays the edit UI when an edit +is in flight; keys.go's handleEditKey takes over key input during +that time (y/N/r/Esc). + +The e keybind on panePipeline kicks off the flow. main.go wires +edit.DefaultRunner + the picker's StatusEndpoint into the model +post-port-forward. + +End-to-end test exercises the full flow with a stub kubectl Runner +and a fake /reload/status server. + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Task 10: README + e2e + +Document the keybind, RBAC requirement, and tempfile lifecycle. Add an opt-in e2e test if a live cluster is handy. + +**Files:** +- Modify: `authbridge/cmd/abctl/README.md` +- Optional: `authbridge/cmd/abctl/edit/e2e_test.go` (build-tag `e2e`) + +### Step 1: Update README + +Add to the keybinds table: + +``` +| `e` | pipeline | edit pipeline subtree in $EDITOR | +| `y` | edit/diff | apply the edit | +| `N` | edit/diff | abort the edit | +| `r` | edit/error | re-open the editor with the same tempfile | +| `Esc` | edit/* | abort the edit, return to Pipeline pane | +``` + +Add a new section: + +```markdown +## Editing the pipeline + +Press `e` on the Pipeline pane to edit the agent's runtime `pipeline:` +subtree in `$EDITOR` (or `vi` if unset). On save, abctl shows a diff +and asks `apply this change? (y/N)`. Confirming runs +`kubectl apply --server-side` against the per-agent ConfigMap, then +polls the framework's `/reload/status` until the reload completes +(success or failure). + +The single edit flow covers four operations: +- **Edit a value** — change a config field of an existing plugin +- **Reorder** — move a plugin's lines up or down +- **Remove** — delete a plugin's entry from `inbound:` or `outbound:` +- **Add** — append a new plugin entry + +All four work because they're all just lines you change inside the +pipeline subtree. + +### Permissions + +abctl shells out to `kubectl`; kubectl uses your kubeconfig. Editing +requires `update` on `configmaps` in the agent's namespace (in +addition to `get pods` which the picker already needs). RBAC denial +surfaces verbatim in the overlay. + +### Tempfile lifecycle + +abctl writes the editable pipeline subtree to `$TMPDIR/abctl-pipeline-*.yaml` +on every edit. The tempfile is **left in place on every exit path** +(success, error, abort) so an interrupted edit is recoverable. Clean +up `/tmp/abctl-pipeline-*` periodically. + +### Hot-reload window + +The framework reloads via a config-file watcher; kubelet syncs +ConfigMap edits into the pod's mount within ~60s, then the framework +debounces and reloads. Total wall-clock from `apply` to reload is +typically under 90s. abctl shows a spinner during the wait. If +`/reload/status` doesn't observe a successful reload within 120s, +abctl gives up watching and tells you to check `kubectl logs deploy/`. +``` + +### Step 2: Verify markdown rendering + +```bash +head -100 authbridge/cmd/abctl/README.md +``` +Confirm the new section reads cleanly. + +### Step 3: Commit + +```bash +cd /Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions +git add authbridge/cmd/abctl/README.md +git commit -s -m "docs(abctl): Document the e keybind for pipeline editing + +Adds a new section explaining the edit flow, the four operations it +covers (edit/reorder/remove/add), the RBAC requirement (update +configmaps), the tempfile lifecycle, and the hot-reload wait +window. Keybinds table updated. + +Assisted-By: Claude (Anthropic AI) " +``` + +### Step 4 (optional): Live e2e test + +If you have an IBAC cluster running and want belt-and-braces coverage: + +Create `authbridge/cmd/abctl/edit/e2e_test.go`: + +```go +//go:build e2e + +package edit + +import ( + "context" + "strings" + "testing" + "time" +) + +// TestE2E_FetchExistingAgent verifies Fetch works against a real cluster. +// Requires `make demo-ibac` to have run. +func TestE2E_FetchExistingAgent(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + fp, err := Fetch(ctx, DefaultRunner, "team1", "email-agent") + if err != nil { + t.Fatalf("Fetch: %v", err) + } + subtree := fp.InnerYAML[fp.PipelineStart:fp.PipelineEnd] + if !strings.Contains(string(subtree), "pipeline:") { + t.Fatalf("subtree missing header: %s", subtree) + } +} +``` + +Run: `go test -tags=e2e ./edit/ -run TestE2E_FetchExistingAgent -v` + +### Step 5 (optional): Commit e2e test + +```bash +git add authbridge/cmd/abctl/edit/e2e_test.go +git commit -s -m "test(abctl): Add opt-in e2e test for edit Fetch path + +Build-tag-gated; requires the IBAC demo cluster running. Verifies the +real kubectl path against an actual ConfigMap. + +Assisted-By: Claude (Anthropic AI) " +``` + +--- + +## Self-Review Checklist + +**1. Spec coverage:** +- ✅ Two-port port-forward → Task 1 +- ✅ Fetch ConfigMap, locate pipeline range → Tasks 2, 4 +- ✅ Splice + manifest rebuild → Task 3 +- ✅ Apply via kubectl --server-side → Task 4 +- ✅ Diff renderer → Task 5 +- ✅ Poll /reload/status → Task 6 +- ✅ Overlay state + render → Task 7 +- ✅ Cmd factories → Task 8 +- ✅ Update wiring + e keybind + e2e test → Task 9 +- ✅ README + RBAC docs + tempfile lifecycle → Task 10 +- ✅ Acceptance criteria 1-9 covered by Task 9's e2e test +- ✅ Acceptance 10 (`go test ./...` + e2e) — Task 10 + +**2. Type consistency:** +- `Runner` is `func(ctx, args ...string) ([]byte, error)` everywhere (defined Task 4, used Tasks 4/8/9). +- `FetchedPipeline` fields: `ConfigMapYAML`, `InnerYAML`, `PipelineStart`, `PipelineEnd` — same name in Task 4 def + Task 8 (FetchCmd) + Task 9 (state machine). +- Cmd factory msg types: `FetchedMsg`, `AppliedMsg`, `PolledMsg` — referenced consistently. `editorExitedMsg` is TUI-local (not in `edit` package). +- `editPhase` enum values `editPhaseDone/Fetching/Editing/Validating/Diff/Applying/Waiting/Error` — same names in render (Task 7), state machine (Task 9), and tests. +- `PollResult.Status` is `PollSuccess/PollFailure/PollTimeout` — same names in Task 6, Task 8, Task 9. + +**3. Placeholder scan:** +- Task 9 references `m.loadPipelineCmd()` for the post-edit refresh. This is the existing helper from the picker work — no new code needed. +- Task 9 says "the exact splice depends on existing main.go shape. Read the file before patching." This is necessary; the implementer needs to look at main.go's current model construction. Marked as such. +- No TBDs / TODOs. From c71cc393981931a7c41f349ff362289174bd69b3 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:24:40 -0400 Subject: [PATCH 03/24] feat(abctl): Forward :9093 alongside :9094 in the picker The pipeline editor (incoming) needs to reach /reload/status on the stat server (:9093) to know when the framework finished reloading. Extend kubectlPortForwarder to forward both ports through one kubectl subprocess. PortForward interface gains StatusEndpoint() returning the :9093 URL. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/cluster/portforward.go | 33 ++++++++++++++++--- .../cmd/abctl/cluster/portforward_test.go | 19 +++++++++++ authbridge/cmd/abctl/tui/picker_test.go | 5 +-- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/authbridge/cmd/abctl/cluster/portforward.go b/authbridge/cmd/abctl/cluster/portforward.go index aa43e0a67..0b24c682b 100644 --- a/authbridge/cmd/abctl/cluster/portforward.go +++ b/authbridge/cmd/abctl/cluster/portforward.go @@ -59,8 +59,10 @@ type PortForwarder interface { // PortForward is a live tunnel to a pod. The caller MUST Close it // exactly once. type PortForward interface { - // Endpoint is the URL abctl points its apiclient at. + // Endpoint is the URL abctl points its apiclient at (:9094 session API). Endpoint() string + // StatusEndpoint is the URL of the agent's stat server (:9093 /reload/status). + StatusEndpoint() string // Close terminates the tunnel and waits for it to exit. Close() error } @@ -81,6 +83,10 @@ func (k *kubectlPortForwarder) Start(ctx context.Context, namespace, pod string) if err != nil { return nil, err } + statusPort, err := freeLocalPort() + if err != nil { + return nil, err + } // We do NOT bind ctx to the subprocess — kubectl port-forward should // outlive the per-call context (which is just for the readiness // check). The subprocess is terminated explicitly via Close. @@ -88,6 +94,7 @@ func (k *kubectlPortForwarder) Start(ctx context.Context, namespace, pod string) "-n", namespace, "pod/"+pod, strconv.Itoa(port)+":9094", + strconv.Itoa(statusPort)+":9093", ) stderr, err := cmd.StderrPipe() if err != nil { @@ -96,7 +103,7 @@ func (k *kubectlPortForwarder) Start(ctx context.Context, namespace, pod string) if err := cmd.Start(); err != nil { return nil, fmt.Errorf("start kubectl port-forward: %w", err) } - pf := &kubectlPortForward{cmd: cmd, port: port, stderr: stderr} + pf := &kubectlPortForward{cmd: cmd, port: port, statusPort: statusPort, stderr: stderr} pf.startStderrDrain() readyCtx, cancel := context.WithTimeout(ctx, pfReadyTimeout) @@ -110,13 +117,22 @@ func (k *kubectlPortForwarder) Start(ctx context.Context, namespace, pod string) } return nil, fmt.Errorf("port-forward not ready: %w", err) } + if err := waitForAccept(readyCtx, statusPort); err != nil { + _ = pf.Close() + stderrTail := pf.stderrTail() + if stderrTail != "" { + return nil, fmt.Errorf("port-forward (:9093) not ready: %w (stderr: %s)", err, stderrTail) + } + return nil, fmt.Errorf("port-forward (:9093) not ready: %w", err) + } return pf, nil } type kubectlPortForward struct { - cmd *exec.Cmd - port int - stderr io.ReadCloser + cmd *exec.Cmd + port int // local 127.0.0.1 port forwarding to pod :9094 + statusPort int // local 127.0.0.1 port forwarding to pod :9093 + stderr io.ReadCloser mu sync.Mutex stderrLines []string @@ -127,6 +143,13 @@ func (p *kubectlPortForward) Endpoint() string { return "http://127.0.0.1:" + strconv.Itoa(p.port) } +// StatusEndpoint is the URL of the agent's stat server (:9093) reached +// via the picker's port-forward. abctl's edit flow polls /reload/status +// here. +func (p *kubectlPortForward) StatusEndpoint() string { + return "http://127.0.0.1:" + strconv.Itoa(p.statusPort) +} + func (p *kubectlPortForward) Close() error { if p.cmd.Process == nil { return nil diff --git a/authbridge/cmd/abctl/cluster/portforward_test.go b/authbridge/cmd/abctl/cluster/portforward_test.go index 90aefdaf8..194146780 100644 --- a/authbridge/cmd/abctl/cluster/portforward_test.go +++ b/authbridge/cmd/abctl/cluster/portforward_test.go @@ -72,3 +72,22 @@ func TestPortForwarderBuildOnly(t *testing.T) { // kubectl. var _ PortForwarder = NewPortForwarder() } + +func TestKubectlPortForward_StatusEndpoint(t *testing.T) { + pf := &kubectlPortForward{port: 12345, statusPort: 12346} + want := "http://127.0.0.1:12346" + if got := pf.StatusEndpoint(); got != want { + t.Fatalf("StatusEndpoint: got %q want %q", got, want) + } + if got := pf.Endpoint(); got != "http://127.0.0.1:12345" { + t.Fatalf("Endpoint: got %q", got) + } +} + +func TestPortForwarderInterfaceHasStatusEndpoint(t *testing.T) { + var _ interface { + Endpoint() string + StatusEndpoint() string + Close() error + } = (*kubectlPortForward)(nil) +} diff --git a/authbridge/cmd/abctl/tui/picker_test.go b/authbridge/cmd/abctl/tui/picker_test.go index 0e5b1aefb..ee28dd2b4 100644 --- a/authbridge/cmd/abctl/tui/picker_test.go +++ b/authbridge/cmd/abctl/tui/picker_test.go @@ -261,8 +261,9 @@ type fakePortForward struct { parent *fakePortForwarder } -func (p *fakePortForward) Endpoint() string { return p.endpoint } -func (p *fakePortForward) Close() error { p.parent.closeCount++; return nil } +func (p *fakePortForward) Endpoint() string { return p.endpoint } +func (p *fakePortForward) StatusEndpoint() string { return "" } // unused by current picker tests +func (p *fakePortForward) Close() error { p.parent.closeCount++; return nil } func TestPodEnterStartsPortForwardAndTransitions(t *testing.T) { pf := &fakePortForwarder{endpoint: "http://127.0.0.1:60000"} From e9e1172162456a2e4ecafe0d60dc11d7cfb49c20 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:26:50 -0400 Subject: [PATCH 04/24] feat(abctl): Add edit package with FindPipelineRange Pure-Go function that locates the pipeline: subtree's byte range inside a runtime config YAML. Uses yaml.v3 only to find boundaries; the actual splice (Task 3) happens against raw bytes to preserve comments outside the subtree. Adds gopkg.in/yaml.v3 dependency. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/configmap.go | 81 +++++++++++++++++ authbridge/cmd/abctl/edit/configmap_test.go | 97 +++++++++++++++++++++ authbridge/cmd/abctl/go.mod | 1 + authbridge/cmd/abctl/go.sum | 4 + 4 files changed, 183 insertions(+) create mode 100644 authbridge/cmd/abctl/edit/configmap.go create mode 100644 authbridge/cmd/abctl/edit/configmap_test.go diff --git a/authbridge/cmd/abctl/edit/configmap.go b/authbridge/cmd/abctl/edit/configmap.go new file mode 100644 index 000000000..a94436558 --- /dev/null +++ b/authbridge/cmd/abctl/edit/configmap.go @@ -0,0 +1,81 @@ +// Package edit implements abctl's in-place pipeline editor. The flow is: +// fetch the agent's ConfigMap via kubectl, locate the pipeline: subtree, +// open just that subtree in the user's $EDITOR, splice the edit back into +// the original ConfigMap manifest, kubectl apply --server-side, then poll +// /reload/status until the framework reloads. +// +// All kubectl interaction goes through the Runner injection seam so tests +// can stub it out. +package edit + +import ( + "fmt" + + "gopkg.in/yaml.v3" +) + +// FindPipelineRange returns the byte offsets [start, end) in innerYAML +// that span the "pipeline:" subtree, including the "pipeline:" key line +// itself but not any following top-level keys. Used by the editor to +// extract just the pipeline subtree for the user, and by Splice to +// replace it with the user's edit. +// +// Returns an error if innerYAML is not valid YAML or if no top-level +// "pipeline" key exists. +func FindPipelineRange(innerYAML []byte) (start, end int, err error) { + var root yaml.Node + if err := yaml.Unmarshal(innerYAML, &root); err != nil { + return 0, 0, fmt.Errorf("parse runtime YAML: %w", err) + } + if root.Kind != yaml.DocumentNode || len(root.Content) == 0 { + return 0, 0, fmt.Errorf("runtime YAML is not a document") + } + doc := root.Content[0] + if doc.Kind != yaml.MappingNode { + return 0, 0, fmt.Errorf("runtime YAML root is not a mapping") + } + + // Children of a MappingNode alternate key, value, key, value, ... + // Find the index of the "pipeline" key, capture its line, and find + // the next sibling's line (or end-of-document if it's the last key). + pipelineKeyIdx := -1 + for i := 0; i < len(doc.Content); i += 2 { + k := doc.Content[i] + if k.Value == "pipeline" { + pipelineKeyIdx = i + break + } + } + if pipelineKeyIdx == -1 { + return 0, 0, fmt.Errorf("no top-level pipeline key in runtime YAML") + } + + pipelineKeyLine := doc.Content[pipelineKeyIdx].Line // 1-indexed + var nextKeyLine int // 1-indexed; 0 if pipeline is last + if pipelineKeyIdx+2 < len(doc.Content) { + nextKeyLine = doc.Content[pipelineKeyIdx+2].Line + } + + // Map line numbers to byte offsets. yaml.v3 Line is 1-indexed. + lineStarts := []int{0} // lineStarts[i] = byte offset where line i+1 starts + for i, b := range innerYAML { + if b == '\n' { + lineStarts = append(lineStarts, i+1) + } + } + + if pipelineKeyLine < 1 || pipelineKeyLine > len(lineStarts) { + return 0, 0, fmt.Errorf("pipeline key line %d out of range", pipelineKeyLine) + } + start = lineStarts[pipelineKeyLine-1] + + if nextKeyLine == 0 { + end = len(innerYAML) + } else { + if nextKeyLine < 1 || nextKeyLine > len(lineStarts) { + return 0, 0, fmt.Errorf("next-key line %d out of range", nextKeyLine) + } + end = lineStarts[nextKeyLine-1] + } + return start, end, nil +} diff --git a/authbridge/cmd/abctl/edit/configmap_test.go b/authbridge/cmd/abctl/edit/configmap_test.go new file mode 100644 index 000000000..0942e6344 --- /dev/null +++ b/authbridge/cmd/abctl/edit/configmap_test.go @@ -0,0 +1,97 @@ +package edit + +import ( + "strings" + "testing" +) + +const fixtureMidYAML = `mode: proxy-sidecar + +listener: + forward_proxy_addr: ":8081" + +pipeline: + inbound: + - name: jwt-validation + config: + issuer: http://idp + outbound: + - name: token-exchange + +session: + enabled: true +` + +const fixtureLastYAML = `mode: proxy-sidecar + +pipeline: + inbound: + - name: jwt-validation +` + +const fixtureFirstYAML = `pipeline: + inbound: + - name: jwt-validation + +mode: proxy-sidecar +` + +const fixtureMissingYAML = `mode: proxy-sidecar + +listener: + forward_proxy_addr: ":8081" +` + +func TestFindPipelineRange_Middle(t *testing.T) { + start, end, err := FindPipelineRange([]byte(fixtureMidYAML)) + if err != nil { + t.Fatalf("FindPipelineRange: %v", err) + } + got := fixtureMidYAML[start:end] + if !strings.Contains(got, "pipeline:") { + t.Fatalf("range missing pipeline header: %q", got) + } + if !strings.Contains(got, "token-exchange") { + t.Fatalf("range missing pipeline body: %q", got) + } + if strings.Contains(got, "session:") { + t.Fatalf("range includes next key: %q", got) + } + if strings.Contains(got, "listener:") { + t.Fatalf("range includes prior key: %q", got) + } +} + +func TestFindPipelineRange_LastKey(t *testing.T) { + start, end, err := FindPipelineRange([]byte(fixtureLastYAML)) + if err != nil { + t.Fatalf("FindPipelineRange: %v", err) + } + if end != len(fixtureLastYAML) { + t.Fatalf("end = %d, want len(yaml) = %d", end, len(fixtureLastYAML)) + } + got := fixtureLastYAML[start:end] + if !strings.Contains(got, "jwt-validation") { + t.Fatalf("range missing pipeline body: %q", got) + } +} + +func TestFindPipelineRange_FirstKey(t *testing.T) { + start, _, err := FindPipelineRange([]byte(fixtureFirstYAML)) + if err != nil { + t.Fatalf("FindPipelineRange: %v", err) + } + if start != 0 { + t.Fatalf("start = %d, want 0", start) + } +} + +func TestFindPipelineRange_Missing(t *testing.T) { + _, _, err := FindPipelineRange([]byte(fixtureMissingYAML)) + if err == nil { + t.Fatal("want error when pipeline key is absent") + } + if !strings.Contains(err.Error(), "pipeline") { + t.Fatalf("error should mention pipeline: %v", err) + } +} diff --git a/authbridge/cmd/abctl/go.mod b/authbridge/cmd/abctl/go.mod index 5b07473bd..5a97f938c 100644 --- a/authbridge/cmd/abctl/go.mod +++ b/authbridge/cmd/abctl/go.mod @@ -15,6 +15,7 @@ require ( github.com/charmbracelet/lipgloss v1.1.0 github.com/charmbracelet/x/ansi v0.11.6 github.com/kagenti/kagenti-extensions/authbridge/authlib v0.0.0-00010101000000-000000000000 + gopkg.in/yaml.v3 v3.0.1 ) require ( diff --git a/authbridge/cmd/abctl/go.sum b/authbridge/cmd/abctl/go.sum index 59a5512f1..b26a4e232 100644 --- a/authbridge/cmd/abctl/go.sum +++ b/authbridge/cmd/abctl/go.sum @@ -54,3 +54,7 @@ golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k= golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/text v0.34.0 h1:oL/Qq0Kdaqxa1KbNeMKwQq0reLCCaFtqu2eNuSeNHbk= golang.org/x/text v0.34.0/go.mod h1:homfLqTYRFyVYemLBFl5GgL/DWEiH5wcsQ5gSh1yziA= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 624c52f4e792c0abd9c54c6ec3da62bf2f55dafc Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:29:48 -0400 Subject: [PATCH 05/24] feat(abctl): Add Splice and BuildManifest helpers Splice does byte-range surgery to replace the pipeline subtree without touching anything outside it (preserves comments, blank lines, field ordering). BuildManifest parses the outer ConfigMap YAML, sets data.config.yaml to a literal-block scalar carrying the new inner content, and re-emits the manifest ready for kubectl apply. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/configmap.go | 71 ++++++++++++++++ authbridge/cmd/abctl/edit/configmap_test.go | 89 +++++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/authbridge/cmd/abctl/edit/configmap.go b/authbridge/cmd/abctl/edit/configmap.go index a94436558..ecb4227ed 100644 --- a/authbridge/cmd/abctl/edit/configmap.go +++ b/authbridge/cmd/abctl/edit/configmap.go @@ -9,6 +9,7 @@ package edit import ( + "bytes" "fmt" "gopkg.in/yaml.v3" @@ -79,3 +80,73 @@ func FindPipelineRange(innerYAML []byte) (start, end int, err error) { } return start, end, nil } + +// Splice replaces the byte range [start, end) of innerYAML with newSubtree +// and returns the result. Used to apply the user's edit to just the pipeline +// subtree, leaving everything outside it byte-for-byte unchanged. Comments, +// blank lines, and field ordering outside the pipeline subtree all survive. +func Splice(innerYAML []byte, start, end int, newSubtree []byte) []byte { + var b bytes.Buffer + b.Grow(len(innerYAML) - (end - start) + len(newSubtree)) + b.Write(innerYAML[:start]) + b.Write(newSubtree) + b.Write(innerYAML[end:]) + return b.Bytes() +} + +// BuildManifest takes the original ConfigMap YAML manifest (as returned by +// kubectl get cm -o yaml) and a new inner runtime YAML (the contents that +// belong in data.config.yaml). Returns a manifest ready for kubectl apply. +// +// The manifest passes through yaml.v3 so the outer structure (apiVersion, +// kind, metadata, etc.) is preserved. Only data.config.yaml is replaced. +// Comments inside the inner runtime YAML survive because we set the +// data.config.yaml value to a literal block (|) string carrying newInner +// verbatim. +func BuildManifest(origCMYAML, newInner []byte) ([]byte, error) { + var root yaml.Node + if err := yaml.Unmarshal(origCMYAML, &root); err != nil { + return nil, fmt.Errorf("parse ConfigMap manifest: %w", err) + } + if root.Kind != yaml.DocumentNode || len(root.Content) == 0 { + return nil, fmt.Errorf("ConfigMap manifest is not a document") + } + doc := root.Content[0] + if doc.Kind != yaml.MappingNode { + return nil, fmt.Errorf("ConfigMap manifest root is not a mapping") + } + + // Find data → config.yaml. + var dataNode *yaml.Node + for i := 0; i < len(doc.Content); i += 2 { + if doc.Content[i].Value == "data" { + dataNode = doc.Content[i+1] + break + } + } + if dataNode == nil || dataNode.Kind != yaml.MappingNode { + return nil, fmt.Errorf("ConfigMap has no data: mapping") + } + var configValueNode *yaml.Node + for i := 0; i < len(dataNode.Content); i += 2 { + if dataNode.Content[i].Value == "config.yaml" { + configValueNode = dataNode.Content[i+1] + break + } + } + if configValueNode == nil { + return nil, fmt.Errorf("ConfigMap data has no config.yaml key") + } + + // Set the value to a literal-block scalar carrying newInner. + configValueNode.Kind = yaml.ScalarNode + configValueNode.Tag = "!!str" + configValueNode.Style = yaml.LiteralStyle + configValueNode.Value = string(newInner) + + out, err := yaml.Marshal(&root) + if err != nil { + return nil, fmt.Errorf("emit ConfigMap manifest: %w", err) + } + return out, nil +} diff --git a/authbridge/cmd/abctl/edit/configmap_test.go b/authbridge/cmd/abctl/edit/configmap_test.go index 0942e6344..5790c17f1 100644 --- a/authbridge/cmd/abctl/edit/configmap_test.go +++ b/authbridge/cmd/abctl/edit/configmap_test.go @@ -42,6 +42,23 @@ listener: forward_proxy_addr: ":8081" ` +const fixtureCMYAML = `apiVersion: v1 +kind: ConfigMap +metadata: + name: authbridge-config-email-agent + namespace: team1 +data: + config.yaml: | + mode: proxy-sidecar + pipeline: + inbound: + - name: jwt-validation + config: + issuer: old + session: + enabled: true +` + func TestFindPipelineRange_Middle(t *testing.T) { start, end, err := FindPipelineRange([]byte(fixtureMidYAML)) if err != nil { @@ -95,3 +112,75 @@ func TestFindPipelineRange_Missing(t *testing.T) { t.Fatalf("error should mention pipeline: %v", err) } } + +func TestSplice_PreservesOutsideRange(t *testing.T) { + const orig = `mode: proxy-sidecar +# this comment must survive +listener: + forward_proxy_addr: ":8081" + +pipeline: + inbound: + - name: jwt-validation + +session: + enabled: true +` + start, end, err := FindPipelineRange([]byte(orig)) + if err != nil { + t.Fatal(err) + } + const newSubtree = `pipeline: + inbound: + - name: jwt-validation + config: + issuer: new + +` + got := Splice([]byte(orig), start, end, []byte(newSubtree)) + gotS := string(got) + if !strings.Contains(gotS, "# this comment must survive") { + t.Fatalf("comment outside pipeline subtree was dropped:\n%s", gotS) + } + if !strings.Contains(gotS, "listener:") { + t.Fatalf("listener section was dropped:\n%s", gotS) + } + if !strings.Contains(gotS, "session:") { + t.Fatalf("session section was dropped:\n%s", gotS) + } + if !strings.Contains(gotS, "issuer: new") { + t.Fatalf("new pipeline content not present:\n%s", gotS) + } + if strings.Contains(gotS, "issuer: old") { + t.Fatalf("old pipeline content still present:\n%s", gotS) + } +} + +func TestBuildManifest_UpdatesDataField(t *testing.T) { + const newInner = `mode: proxy-sidecar +pipeline: + inbound: + - name: jwt-validation + config: + issuer: new +session: + enabled: true +` + out, err := BuildManifest([]byte(fixtureCMYAML), []byte(newInner)) + if err != nil { + t.Fatalf("BuildManifest: %v", err) + } + outS := string(out) + if !strings.Contains(outS, "name: authbridge-config-email-agent") { + t.Fatalf("metadata.name lost:\n%s", outS) + } + if !strings.Contains(outS, "namespace: team1") { + t.Fatalf("metadata.namespace lost:\n%s", outS) + } + if !strings.Contains(outS, "issuer: new") { + t.Fatalf("new content not in manifest:\n%s", outS) + } + if strings.Contains(outS, "issuer: old") { + t.Fatalf("old content still in manifest:\n%s", outS) + } +} From a2a476358e04b2aee982347a7af9a2a00e613f71 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:32:25 -0400 Subject: [PATCH 06/24] feat(abctl): Add Fetch and Apply for the edit flow Fetch reads authbridge-config- via kubectl, extracts the inner runtime YAML from data.config.yaml, and locates the pipeline subtree's byte range. Apply writes a manifest to a tempfile and runs kubectl apply --server-side --force-conflicts=false; returns the apply timestamp for downstream comparison against /reload/status. All kubectl interaction goes through a Runner injection seam mirroring cmd/abctl/cluster. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/configmap.go | 105 ++++++++++++++++++++ authbridge/cmd/abctl/edit/configmap_test.go | 94 ++++++++++++++++++ 2 files changed, 199 insertions(+) diff --git a/authbridge/cmd/abctl/edit/configmap.go b/authbridge/cmd/abctl/edit/configmap.go index ecb4227ed..d43ba43e7 100644 --- a/authbridge/cmd/abctl/edit/configmap.go +++ b/authbridge/cmd/abctl/edit/configmap.go @@ -10,7 +10,12 @@ package edit import ( "bytes" + "context" "fmt" + "os" + "os/exec" + "strings" + "time" "gopkg.in/yaml.v3" ) @@ -150,3 +155,103 @@ func BuildManifest(origCMYAML, newInner []byte) ([]byte, error) { } return out, nil } + +// Runner abstracts a `kubectl ` invocation. Production uses os/exec; +// tests inject their own. Mirrors the Runner pattern in cmd/abctl/cluster. +type Runner func(ctx context.Context, args ...string) ([]byte, error) + +// DefaultRunner shells out to the system `kubectl`. +func DefaultRunner(ctx context.Context, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "kubectl", args...) + out, err := cmd.Output() + if err != nil { + if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 { + return nil, fmt.Errorf("kubectl: %s", strings.TrimSpace(string(ee.Stderr))) + } + return nil, fmt.Errorf("kubectl: %w", err) + } + return out, nil +} + +// FetchedPipeline is what Fetch returns: the full ConfigMap manifest, the +// inner runtime YAML extracted from data.config.yaml, and the byte range +// of the pipeline subtree within the inner YAML. +type FetchedPipeline struct { + ConfigMapYAML []byte // raw kubectl get cm -o yaml output + InnerYAML []byte // value of data.config.yaml + PipelineStart int // byte offset in InnerYAML where pipeline: begins + PipelineEnd int // byte offset where the subtree ends +} + +// Fetch reads the per-agent ConfigMap (authbridge-config-), extracts +// the inner runtime YAML from data.config.yaml, and locates the pipeline +// subtree's byte range. Returns an error if the ConfigMap doesn't exist, +// has no data.config.yaml, or has no top-level pipeline: key. +func Fetch(ctx context.Context, run Runner, namespace, agent string) (*FetchedPipeline, error) { + cmName := "authbridge-config-" + agent + cmBytes, err := run(ctx, "get", "cm", cmName, "-n", namespace, "-o", "yaml") + if err != nil { + return nil, err + } + inner, err := extractInnerYAML(cmBytes) + if err != nil { + return nil, err + } + start, end, err := FindPipelineRange(inner) + if err != nil { + return nil, err + } + return &FetchedPipeline{ + ConfigMapYAML: cmBytes, + InnerYAML: inner, + PipelineStart: start, + PipelineEnd: end, + }, nil +} + +// extractInnerYAML pulls data.config.yaml out of an outer ConfigMap manifest. +func extractInnerYAML(cmYAML []byte) ([]byte, error) { + var root yaml.Node + if err := yaml.Unmarshal(cmYAML, &root); err != nil { + return nil, fmt.Errorf("parse ConfigMap manifest: %w", err) + } + if root.Kind != yaml.DocumentNode || len(root.Content) == 0 { + return nil, fmt.Errorf("ConfigMap manifest is not a document") + } + doc := root.Content[0] + for i := 0; i+1 < len(doc.Content); i += 2 { + if doc.Content[i].Value == "data" { + dataNode := doc.Content[i+1] + for j := 0; j+1 < len(dataNode.Content); j += 2 { + if dataNode.Content[j].Value == "config.yaml" { + return []byte(dataNode.Content[j+1].Value), nil + } + } + } + } + return nil, fmt.Errorf("ConfigMap data has no config.yaml key") +} + +// Apply writes manifest to a tempfile and runs kubectl apply --server-side. +// Returns the wall-clock time at which the apply call started; the caller +// uses this to compare against /reload/status's last_success_unix to know +// whether the framework has picked up the change yet. +func Apply(ctx context.Context, run Runner, manifest []byte) (time.Time, error) { + tmp, err := os.CreateTemp("", "abctl-cm-*.yaml") + if err != nil { + return time.Time{}, fmt.Errorf("create temp manifest: %w", err) + } + defer os.Remove(tmp.Name()) + if _, err := tmp.Write(manifest); err != nil { + _ = tmp.Close() + return time.Time{}, fmt.Errorf("write temp manifest: %w", err) + } + if err := tmp.Close(); err != nil { + return time.Time{}, fmt.Errorf("close temp manifest: %w", err) + } + applyTime := time.Now() + if _, err := run(ctx, "apply", "--server-side", "--force-conflicts=false", "-f", tmp.Name()); err != nil { + return time.Time{}, err + } + return applyTime, nil +} diff --git a/authbridge/cmd/abctl/edit/configmap_test.go b/authbridge/cmd/abctl/edit/configmap_test.go index 5790c17f1..d1fa0485e 100644 --- a/authbridge/cmd/abctl/edit/configmap_test.go +++ b/authbridge/cmd/abctl/edit/configmap_test.go @@ -1,8 +1,12 @@ package edit import ( + "context" + "fmt" + "os" "strings" "testing" + "time" ) const fixtureMidYAML = `mode: proxy-sidecar @@ -184,3 +188,93 @@ session: t.Fatalf("old content still in manifest:\n%s", outS) } } + +func TestFetch_HappyPath(t *testing.T) { + wantArgs := []string{ + "get", "cm", "authbridge-config-email-agent", + "-n", "team1", "-o", "yaml", + } + stub := func(ctx context.Context, args ...string) ([]byte, error) { + if !equalArgs(args, wantArgs) { + t.Fatalf("kubectl args: got %v want %v", args, wantArgs) + } + return []byte(fixtureCMYAML), nil + } + fp, err := Fetch(context.Background(), stub, "team1", "email-agent") + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if len(fp.ConfigMapYAML) == 0 { + t.Fatal("ConfigMapYAML empty") + } + if len(fp.InnerYAML) == 0 { + t.Fatal("InnerYAML empty") + } + if fp.PipelineEnd <= fp.PipelineStart { + t.Fatalf("pipeline range invalid: [%d, %d)", fp.PipelineStart, fp.PipelineEnd) + } + subtree := fp.InnerYAML[fp.PipelineStart:fp.PipelineEnd] + if !strings.Contains(string(subtree), "pipeline:") { + t.Fatalf("subtree missing header: %q", subtree) + } + if !strings.Contains(string(subtree), "jwt-validation") { + t.Fatalf("subtree missing body: %q", subtree) + } +} + +func TestFetch_KubectlError(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + return nil, fmt.Errorf("forbidden") + } + _, err := Fetch(context.Background(), stub, "team1", "email-agent") + if err == nil { + t.Fatal("want error from kubectl") + } + if !strings.Contains(err.Error(), "forbidden") { + t.Fatalf("error should surface kubectl message: %v", err) + } +} + +func TestApply_PassesManifest(t *testing.T) { + captured := make([]byte, 0) + stub := func(ctx context.Context, args ...string) ([]byte, error) { + // Args should be: apply --server-side --force-conflicts=false -f + if len(args) < 4 || args[0] != "apply" || args[1] != "--server-side" { + t.Fatalf("kubectl args: %v", args) + } + path := args[len(args)-1] + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read manifest: %v", err) + } + captured = b + return []byte("configmap/foo applied\n"), nil + } + manifest := []byte("apiVersion: v1\nkind: ConfigMap\n") + at, err := Apply(context.Background(), stub, manifest) + if err != nil { + t.Fatalf("Apply: %v", err) + } + if at.IsZero() { + t.Fatal("apply time should be set") + } + if time.Since(at) > 5*time.Second { + t.Fatalf("apply time too far in past: %v", at) + } + if string(captured) != string(manifest) { + t.Fatalf("manifest captured by stub differs from input") + } +} + +// equalArgs checks two []string for equality. +func equalArgs(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} From 461aecd9256f0496ffd616bf80a3dd80c0746583 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:35:55 -0400 Subject: [PATCH 07/24] feat(abctl): Add line-diff renderer for edit confirmation LCS-on-lines with lipgloss styling (green +, red -, dim context). Returns empty string for byte-equal inputs. Used by the edit overlay's confirm prompt to show the user what they're about to apply. Hand-rolled rather than pulling a diff library; pipeline subtrees are typically <50 lines so even quadratic LCS is trivially fast. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/diff.go | 136 +++++++++++++++++++++++++ authbridge/cmd/abctl/edit/diff_test.go | 49 +++++++++ 2 files changed, 185 insertions(+) create mode 100644 authbridge/cmd/abctl/edit/diff.go create mode 100644 authbridge/cmd/abctl/edit/diff_test.go diff --git a/authbridge/cmd/abctl/edit/diff.go b/authbridge/cmd/abctl/edit/diff.go new file mode 100644 index 000000000..d0b65c244 --- /dev/null +++ b/authbridge/cmd/abctl/edit/diff.go @@ -0,0 +1,136 @@ +package edit + +import ( + "strings" + + "github.com/charmbracelet/lipgloss" +) + +var ( + diffStyleAdd = lipgloss.NewStyle().Foreground(lipgloss.Color("2")) // green + diffStyleRemove = lipgloss.NewStyle().Foreground(lipgloss.Color("1")) // red + diffStyleContext = lipgloss.NewStyle().Faint(true) +) + +// Diff renders a line-based diff of old vs new with lipgloss styling. +// Returns the empty string when inputs are byte-equal. The algorithm is +// LCS-on-lines; output is one rendered line per diff entry, terminated +// by newlines. +// +// LCS is O(N*M) in the line count; pipeline subtrees are typically <50 +// lines so the cost is negligible. The diff is intended for the abctl +// edit confirmation overlay — it shows the user every line that +// changed, in original order. +func Diff(old, new []byte) string { + if string(old) == string(new) { + return "" + } + oldLines := splitLinesKeepNewline(old) + newLines := splitLinesKeepNewline(new) + ops := lcsDiff(oldLines, newLines) + var b strings.Builder + for _, op := range ops { + switch op.kind { + case opEqual: + b.WriteString(diffStyleContext.Render(" " + strings.TrimRight(op.line, "\n"))) + b.WriteByte('\n') + case opRemove: + b.WriteString(diffStyleRemove.Render("-" + strings.TrimRight(op.line, "\n"))) + b.WriteByte('\n') + case opAdd: + b.WriteString(diffStyleAdd.Render("+" + strings.TrimRight(op.line, "\n"))) + b.WriteByte('\n') + } + } + return b.String() +} + +func splitLinesKeepNewline(b []byte) []string { + if len(b) == 0 { + return nil + } + s := string(b) + out := strings.SplitAfter(s, "\n") + // strings.SplitAfter on "a\n" returns ["a\n", ""] — drop trailing empty. + if len(out) > 0 && out[len(out)-1] == "" { + out = out[:len(out)-1] + } + return out +} + +type opKind int + +const ( + opEqual opKind = iota + opRemove + opAdd +) + +type diffOp struct { + kind opKind + line string +} + +// lcsDiff is a textbook LCS-then-backtrack implementation. Returns the +// edit script as a sequence of equal/remove/add ops in order. +func lcsDiff(old, new []string) []diffOp { + m, n := len(old), len(new) + if m == 0 { + out := make([]diffOp, n) + for i, l := range new { + out[i] = diffOp{opAdd, l} + } + return out + } + if n == 0 { + out := make([]diffOp, m) + for i, l := range old { + out[i] = diffOp{opRemove, l} + } + return out + } + dp := make([][]int, m+1) + for i := range dp { + dp[i] = make([]int, n+1) + } + for i := 1; i <= m; i++ { + for j := 1; j <= n; j++ { + if old[i-1] == new[j-1] { + dp[i][j] = dp[i-1][j-1] + 1 + } else if dp[i-1][j] >= dp[i][j-1] { + dp[i][j] = dp[i-1][j] + } else { + dp[i][j] = dp[i][j-1] + } + } + } + var rev []diffOp + i, j := m, n + for i > 0 && j > 0 { + switch { + case old[i-1] == new[j-1]: + rev = append(rev, diffOp{opEqual, old[i-1]}) + i-- + j-- + case dp[i-1][j] >= dp[i][j-1]: + rev = append(rev, diffOp{opRemove, old[i-1]}) + i-- + default: + rev = append(rev, diffOp{opAdd, new[j-1]}) + j-- + } + } + for i > 0 { + rev = append(rev, diffOp{opRemove, old[i-1]}) + i-- + } + for j > 0 { + rev = append(rev, diffOp{opAdd, new[j-1]}) + j-- + } + out := make([]diffOp, len(rev)) + for k, op := range rev { + out[len(rev)-1-k] = op + } + return out +} diff --git a/authbridge/cmd/abctl/edit/diff_test.go b/authbridge/cmd/abctl/edit/diff_test.go new file mode 100644 index 000000000..b0abb2e8f --- /dev/null +++ b/authbridge/cmd/abctl/edit/diff_test.go @@ -0,0 +1,49 @@ +package edit + +import ( + "strings" + "testing" +) + +func TestDiff_Equal(t *testing.T) { + got := Diff([]byte("a\nb\nc\n"), []byte("a\nb\nc\n")) + if got != "" { + t.Fatalf("equal inputs should produce empty diff, got %q", got) + } +} + +func TestDiff_OneLineChange(t *testing.T) { + old := []byte("a\nb\nc\n") + new := []byte("a\nB\nc\n") + got := Diff(old, new) + if !strings.Contains(got, "-b") { + t.Fatalf("missing -b line:\n%s", got) + } + if !strings.Contains(got, "+B") { + t.Fatalf("missing +B line:\n%s", got) + } +} + +func TestDiff_AddAndRemove(t *testing.T) { + old := []byte("a\nb\nc\n") + new := []byte("a\nc\nd\n") + got := Diff(old, new) + if !strings.Contains(got, "-b") { + t.Fatalf("missing -b:\n%s", got) + } + if !strings.Contains(got, "+d") { + t.Fatalf("missing +d:\n%s", got) + } +} + +func TestDiff_PreservesContext(t *testing.T) { + old := []byte("line1\nline2\nline3\nline4\n") + new := []byte("line1\nline2\nLINE3\nline4\n") + got := Diff(old, new) + if !strings.Contains(got, "line1") || !strings.Contains(got, "line4") { + t.Fatalf("missing context lines:\n%s", got) + } + if !strings.Contains(got, "-line3") || !strings.Contains(got, "+LINE3") { + t.Fatalf("missing change markers:\n%s", got) + } +} From b4296041c7e03aa99f39704d472bbafbd7075e23 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:38:21 -0400 Subject: [PATCH 08/24] feat(abctl): Poll /reload/status to detect framework reload Watches last_success_unix advancing past applyTime (success) and reloads_failed incrementing past the pre-apply baseline (failure with the framework's error message). 1s cadence. Caller sets the 120s timeout via context.WithTimeout. Transient HTTP errors retry until ctx expires. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/status.go | 94 ++++++++++++++++++++++++ authbridge/cmd/abctl/edit/status_test.go | 89 ++++++++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 authbridge/cmd/abctl/edit/status.go create mode 100644 authbridge/cmd/abctl/edit/status_test.go diff --git a/authbridge/cmd/abctl/edit/status.go b/authbridge/cmd/abctl/edit/status.go new file mode 100644 index 000000000..b30121b91 --- /dev/null +++ b/authbridge/cmd/abctl/edit/status.go @@ -0,0 +1,94 @@ +package edit + +import ( + "context" + "encoding/json" + "net/http" + "time" +) + +// ReloadStatus is the wire shape of the framework's /reload/status endpoint. +// Only the fields abctl uses are decoded. +type ReloadStatus struct { + LastSuccessUnix int64 `json:"last_success_unix"` + ReloadsOK uint64 `json:"reloads_ok"` + ReloadsFailed uint64 `json:"reloads_failed"` + LastError string `json:"last_error"` +} + +// PollResultStatus is a sum type for PollUntilReloaded outcomes. +type PollResultStatus int + +const ( + PollUnknown PollResultStatus = iota + PollSuccess + PollFailure + PollTimeout +) + +// PollResult is what PollUntilReloaded returns. +type PollResult struct { + Status PollResultStatus + LastError string // populated when Status == PollFailure +} + +// pollInterval is the cadence between /reload/status fetches. 1s balances +// user-visible spinner progress with not hammering the cluster on slow +// reloads. +const pollInterval = 1 * time.Second + +// PollUntilReloaded watches statusURL/reload/status until either: +// - LastSuccessUnix > applyTime.Unix() → PollSuccess. +// - ReloadsFailed exceeds the value at first successful poll → PollFailure +// with LastError populated. +// - ctx is done → PollTimeout. (Caller is expected to set a 120s timeout +// via context.WithTimeout.) +// +// HTTP errors (network, non-200) are retried until ctx expires; we treat +// them as "framework not yet reachable, keep waiting." +func PollUntilReloaded(ctx context.Context, statusURL string, applyTime time.Time) PollResult { + url := statusURL + "/reload/status" + client := &http.Client{Timeout: 2 * time.Second} + + var baselineFailed uint64 + first := true + + for { + select { + case <-ctx.Done(): + return PollResult{Status: PollTimeout} + default: + } + + req, err := http.NewRequestWithContext(ctx, "GET", url, nil) + if err != nil { + return PollResult{Status: PollTimeout} + } + resp, err := client.Do(req) + if err == nil && resp.StatusCode == 200 { + var rs ReloadStatus + decodeErr := json.NewDecoder(resp.Body).Decode(&rs) + resp.Body.Close() + if decodeErr == nil { + if first { + baselineFailed = rs.ReloadsFailed + first = false + } + if rs.LastSuccessUnix > applyTime.Unix() { + return PollResult{Status: PollSuccess} + } + if rs.ReloadsFailed > baselineFailed { + return PollResult{Status: PollFailure, LastError: rs.LastError} + } + } + } else if resp != nil { + resp.Body.Close() + } + + select { + case <-ctx.Done(): + return PollResult{Status: PollTimeout} + case <-time.After(pollInterval): + } + } +} diff --git a/authbridge/cmd/abctl/edit/status_test.go b/authbridge/cmd/abctl/edit/status_test.go new file mode 100644 index 000000000..a4d99c62e --- /dev/null +++ b/authbridge/cmd/abctl/edit/status_test.go @@ -0,0 +1,89 @@ +package edit + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" +) + +// makeStatusServer returns a httptest.Server whose /reload/status +// endpoint reads its body from the supplied function — letting the +// test advance state between polls. +func makeStatusServer(t *testing.T, body func() ReloadStatus) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/reload/status" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(body()) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestPollUntilReloaded_Success(t *testing.T) { + applyTime := time.Now() + var calls atomic.Int32 + srv := makeStatusServer(t, func() ReloadStatus { + c := calls.Add(1) + if c == 1 { + return ReloadStatus{LastSuccessUnix: applyTime.Add(-1 * time.Hour).Unix()} + } + return ReloadStatus{LastSuccessUnix: time.Now().Unix()} + }) + res := PollUntilReloaded(context.Background(), srv.URL, applyTime) + if res.Status != PollSuccess { + t.Fatalf("status = %v, want PollSuccess", res.Status) + } +} + +func TestPollUntilReloaded_Failure(t *testing.T) { + applyTime := time.Now() + var calls atomic.Int32 + srv := makeStatusServer(t, func() ReloadStatus { + c := calls.Add(1) + if c == 1 { + return ReloadStatus{ReloadsFailed: 5} + } + return ReloadStatus{ReloadsFailed: 6, LastError: "invalid YAML at line 3"} + }) + res := PollUntilReloaded(context.Background(), srv.URL, applyTime) + if res.Status != PollFailure { + t.Fatalf("status = %v, want PollFailure", res.Status) + } + if res.LastError != "invalid YAML at line 3" { + t.Fatalf("LastError: %q", res.LastError) + } +} + +func TestPollUntilReloaded_Timeout(t *testing.T) { + applyTime := time.Now() + srv := makeStatusServer(t, func() ReloadStatus { + return ReloadStatus{LastSuccessUnix: applyTime.Add(-1 * time.Hour).Unix()} + }) + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + res := PollUntilReloaded(ctx, srv.URL, applyTime) + if res.Status != PollTimeout { + t.Fatalf("status = %v, want PollTimeout", res.Status) + } +} + +func TestPollUntilReloaded_HTTPError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "internal", http.StatusInternalServerError) + })) + defer srv.Close() + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + res := PollUntilReloaded(ctx, srv.URL, time.Now()) + if res.Status != PollTimeout { + t.Fatalf("status = %v, want PollTimeout", res.Status) + } +} From ab54a129a4095c370415364411aeefd7cf02f487 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:41:06 -0400 Subject: [PATCH 09/24] feat(abctl): Add edit overlay state + render editPhase enum, editState struct, renderEditOverlay function for each phase (fetching/editing/validating/diff/applying/waiting/error). No state-machine wiring yet (Task 8 adds Cmd factories, Task 9 wires Update handlers + the e keybind). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/tui/edit_overlay.go | 86 +++++++++++++++++++ authbridge/cmd/abctl/tui/edit_overlay_test.go | 52 +++++++++++ 2 files changed, 138 insertions(+) create mode 100644 authbridge/cmd/abctl/tui/edit_overlay.go create mode 100644 authbridge/cmd/abctl/tui/edit_overlay_test.go diff --git a/authbridge/cmd/abctl/tui/edit_overlay.go b/authbridge/cmd/abctl/tui/edit_overlay.go new file mode 100644 index 000000000..6f52b67cc --- /dev/null +++ b/authbridge/cmd/abctl/tui/edit_overlay.go @@ -0,0 +1,86 @@ +package tui + +import ( + "fmt" + "strings" + "time" + + "github.com/charmbracelet/lipgloss" + + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" +) + +// editPhase tracks where the edit state machine currently sits. +type editPhase int + +const ( + editPhaseDone editPhase = iota // not editing + editPhaseFetching + editPhaseEditing // $EDITOR is running; bubbletea is suspended + editPhaseValidating + editPhaseDiff + editPhaseApplying + editPhaseWaiting + editPhaseError +) + +// editState lives on *model when an edit is in flight. +type editState struct { + phase editPhase + fetched *edit.FetchedPipeline + tempPath string + editedRaw []byte // bytes the user wrote in $EDITOR + diff string // colorized output from edit.Diff + err string // single-line message in editPhaseError + applyTime time.Time +} + +// renderEditOverlay returns the overlay content (rendered into a +// styled box) for the current edit phase. width/height are the +// terminal's full dimensions; the overlay sizes itself to fit +// comfortably inside. +func renderEditOverlay(s editState, width, height int) string { + box := lipgloss.NewStyle(). + Border(lipgloss.RoundedBorder()). + Padding(1, 2). + Width(min(width-4, 100)) + + var b strings.Builder + switch s.phase { + case editPhaseFetching: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString("Fetching ConfigMap…") + case editPhaseEditing: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString(fmt.Sprintf("Editor open at %s", s.tempPath)) + case editPhaseValidating: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString("Validating YAML…") + case editPhaseDiff: + b.WriteString(styleTitle.Render("Edit pipeline — review diff")) + b.WriteString("\n\n") + b.WriteString(s.diff) + b.WriteString("\n") + b.WriteString(styleHint.Render("apply this change? (y/N)")) + case editPhaseApplying: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString("Applying to ConfigMap…") + case editPhaseWaiting: + b.WriteString(styleTitle.Render("Edit pipeline")) + b.WriteString("\n\n") + b.WriteString("Waiting for hot-reload…") + b.WriteString("\n") + b.WriteString(styleHint.Render("(this can take up to 120s while kubelet syncs the ConfigMap)")) + case editPhaseError: + b.WriteString(styleTitle.Render("Edit pipeline — error")) + b.WriteString("\n\n") + b.WriteString(s.err) + b.WriteString("\n\n") + b.WriteString(styleHint.Render("[r] re-edit [Esc] back to Pipeline")) + } + return box.Render(b.String()) +} diff --git a/authbridge/cmd/abctl/tui/edit_overlay_test.go b/authbridge/cmd/abctl/tui/edit_overlay_test.go new file mode 100644 index 000000000..3333d439c --- /dev/null +++ b/authbridge/cmd/abctl/tui/edit_overlay_test.go @@ -0,0 +1,52 @@ +package tui + +import ( + "strings" + "testing" +) + +func TestEditOverlayRender_Fetching(t *testing.T) { + s := editState{phase: editPhaseFetching} + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "Fetching ConfigMap") { + t.Fatalf("fetching phase missing message:\n%s", out) + } +} + +func TestEditOverlayRender_Diff(t *testing.T) { + s := editState{ + phase: editPhaseDiff, + diff: "-old line\n+new line\n", + } + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "old line") || !strings.Contains(out, "new line") { + t.Fatalf("diff content missing:\n%s", out) + } + if !strings.Contains(out, "(y/N)") { + t.Fatalf("confirm prompt missing:\n%s", out) + } +} + +func TestEditOverlayRender_Applying(t *testing.T) { + s := editState{phase: editPhaseApplying} + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "Applying") { + t.Fatalf("applying phase missing message:\n%s", out) + } +} + +func TestEditOverlayRender_Waiting(t *testing.T) { + s := editState{phase: editPhaseWaiting} + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "reload") { + t.Fatalf("waiting phase should mention reload:\n%s", out) + } +} + +func TestEditOverlayRender_Error(t *testing.T) { + s := editState{phase: editPhaseError, err: "kubectl: forbidden"} + out := renderEditOverlay(s, 80, 20) + if !strings.Contains(out, "forbidden") { + t.Fatalf("error message not surfaced:\n%s", out) + } +} From d023baf8b8c4653c9f532251d091b2cc5caa8e2b Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:43:15 -0400 Subject: [PATCH 10/24] feat(abctl): Add tea.Cmd factories for the edit phases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FetchCmd / ApplyCmd / PollCmd wrap the existing Fetch/Apply/PollUntilReloaded implementations in tea.Cmd shapes; each emits its corresponding tea.Msg (FetchedMsg/AppliedMsg/PolledMsg). Tested with stub Runners and a fake status server. The validate + diff phases don't need separate Cmd factories — they're synchronous, small, and live inside the TUI Update handlers (Task 9). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/edit.go | 76 ++++++++++++++++++++++++++ authbridge/cmd/abctl/edit/edit_test.go | 70 ++++++++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 authbridge/cmd/abctl/edit/edit.go create mode 100644 authbridge/cmd/abctl/edit/edit_test.go diff --git a/authbridge/cmd/abctl/edit/edit.go b/authbridge/cmd/abctl/edit/edit.go new file mode 100644 index 000000000..5c0aeac8b --- /dev/null +++ b/authbridge/cmd/abctl/edit/edit.go @@ -0,0 +1,76 @@ +package edit + +import ( + "context" + "os" + "time" + + tea "github.com/charmbracelet/bubbletea" +) + +// FetchedMsg is the result of FetchCmd. On success: Fetched and TempPath +// are both set, Err is nil. On failure: Err is populated, others are zero. +type FetchedMsg struct { + Fetched *FetchedPipeline + TempPath string // path to the tempfile holding just the pipeline subtree + Err error +} + +// FetchCmd returns a tea.Cmd that fetches the agent's ConfigMap, locates +// the pipeline subtree, writes the subtree to a tempfile (ready for +// $EDITOR), and emits FetchedMsg. The tempfile lives in $TMPDIR; abctl +// leaves it in place on every exit path (success, error, abort) so users +// can recover an in-progress edit. +func FetchCmd(ctx context.Context, run Runner, namespace, agent string) tea.Cmd { + return func() tea.Msg { + fp, err := Fetch(ctx, run, namespace, agent) + if err != nil { + return FetchedMsg{Err: err} + } + tmp, err := os.CreateTemp("", "abctl-pipeline-*.yaml") + if err != nil { + return FetchedMsg{Err: err} + } + subtree := fp.InnerYAML[fp.PipelineStart:fp.PipelineEnd] + if _, err := tmp.Write(subtree); err != nil { + tmp.Close() + return FetchedMsg{Err: err} + } + path := tmp.Name() + if err := tmp.Close(); err != nil { + return FetchedMsg{Err: err} + } + return FetchedMsg{Fetched: fp, TempPath: path} + } +} + +// AppliedMsg is the result of ApplyCmd. +type AppliedMsg struct { + ApplyTime time.Time + Err error +} + +// ApplyCmd returns a tea.Cmd that runs kubectl apply --server-side on +// the supplied manifest and emits AppliedMsg with the apply timestamp. +func ApplyCmd(ctx context.Context, run Runner, manifest []byte) tea.Cmd { + return func() tea.Msg { + at, err := Apply(ctx, run, manifest) + return AppliedMsg{ApplyTime: at, Err: err} + } +} + +// PolledMsg is the result of PollCmd. +type PolledMsg struct { + Result PollResult +} + +// PollCmd returns a tea.Cmd that polls /reload/status until the framework +// reload completes (success or failure) or ctx expires. Emits PolledMsg. +// +// Caller should construct ctx with a 120s WithTimeout so the poll +// terminates if kubelet doesn't sync within a reasonable window. +func PollCmd(ctx context.Context, statusURL string, applyTime time.Time) tea.Cmd { + return func() tea.Msg { + return PolledMsg{Result: PollUntilReloaded(ctx, statusURL, applyTime)} + } +} diff --git a/authbridge/cmd/abctl/edit/edit_test.go b/authbridge/cmd/abctl/edit/edit_test.go new file mode 100644 index 000000000..a4ce1e735 --- /dev/null +++ b/authbridge/cmd/abctl/edit/edit_test.go @@ -0,0 +1,70 @@ +package edit + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestFetchCmd_Success(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + return []byte(fixtureCMYAML), nil + } + cmd := FetchCmd(context.Background(), stub, "team1", "email-agent") + msg := cmd().(FetchedMsg) + if msg.Err != nil { + t.Fatalf("FetchedMsg.Err = %v", msg.Err) + } + if msg.Fetched == nil { + t.Fatal("FetchedMsg.Fetched is nil") + } + if msg.TempPath == "" { + t.Fatal("FetchedMsg.TempPath empty (should be the path to the subtree-only tempfile)") + } +} + +func TestFetchCmd_Error(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + return nil, fmt.Errorf("forbidden") + } + cmd := FetchCmd(context.Background(), stub, "team1", "email-agent") + msg := cmd().(FetchedMsg) + if msg.Err == nil { + t.Fatal("expected error") + } + if !strings.Contains(msg.Err.Error(), "forbidden") { + t.Fatalf("error: %v", msg.Err) + } +} + +func TestApplyCmd_Success(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + return []byte("applied"), nil + } + cmd := ApplyCmd(context.Background(), stub, []byte("manifest")) + msg := cmd().(AppliedMsg) + if msg.Err != nil { + t.Fatalf("err = %v", msg.Err) + } + if msg.ApplyTime.IsZero() { + t.Fatal("ApplyTime not set") + } +} + +func TestPollCmd_Success(t *testing.T) { + applyTime := time.Now() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"last_success_unix":99999999999}`)) + })) + defer srv.Close() + cmd := PollCmd(context.Background(), srv.URL, applyTime) + msg := cmd().(PolledMsg) + if msg.Result.Status != PollSuccess { + t.Fatalf("status: %v", msg.Result.Status) + } +} From 6fc8a3c36a207d58d0dd30d54697df6fff77eb9f Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:51:50 -0400 Subject: [PATCH 11/24] feat(abctl): Wire edit state machine + e keybind Adds editState/statusURL/editRunner fields on *model. Update handlers for the four edit messages (FetchedMsg, editorExitedMsg, AppliedMsg, PolledMsg) advance the state machine. View() overlays the edit UI when an edit is in flight; keys.go's handleEditKey takes over key input during that time (y/N/r/Esc). The e keybind on panePipeline kicks off the flow. statusURL is set in the existing portForwardReadyMsg handler from PortForward's new StatusEndpoint() accessor. End-to-end test exercises the full flow with a stub kubectl Runner and a fake /reload/status server. Bypasses the real $EDITOR by writing the edited subtree to the tempfile directly and injecting editorExitedMsg{err: nil}. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/tui/app.go | 109 +++++++++++ authbridge/cmd/abctl/tui/edit_e2e_test.go | 198 ++++++++++++++++++++ authbridge/cmd/abctl/tui/keys.go | 66 ++++++- authbridge/cmd/abctl/tui/namespaces_pane.go | 4 + 4 files changed, 375 insertions(+), 2 deletions(-) create mode 100644 authbridge/cmd/abctl/tui/edit_e2e_test.go diff --git a/authbridge/cmd/abctl/tui/app.go b/authbridge/cmd/abctl/tui/app.go index 2e3c736ee..ca86ffc9d 100644 --- a/authbridge/cmd/abctl/tui/app.go +++ b/authbridge/cmd/abctl/tui/app.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "os/exec" "sort" "strings" "time" @@ -14,11 +15,13 @@ import ( "github.com/charmbracelet/bubbles/viewport" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "gopkg.in/yaml.v3" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" "github.com/kagenti/kagenti-extensions/authbridge/authlib/session" "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/apiclient" "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/cluster" + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" ) // Pane identifiers. @@ -98,6 +101,10 @@ type portForwardReadyMsg struct { err error } +// editorExitedMsg is sent by openEditorCmd when the user's $EDITOR +// process exits. err is nil on a clean (0) exit. +type editorExitedMsg struct{ err error } + // Model is the top-level Bubble Tea model. type model struct { endpoint string @@ -191,6 +198,19 @@ type model struct { // activePF is the live port-forward tunnel, if any. Closed on pod-switch // or quit. activePF cluster.PortForward + + // editState tracks an in-flight pipeline edit (the "e" flow). + // editState.phase == editPhaseDone means no edit is active. + editState editState + + // statusURL is the agent's :9093 stat-server URL via the picker's + // port-forward; populated by portForwardReadyMsg. Used by edit.PollCmd + // to watch /reload/status. + statusURL string + + // editRunner is the kubectl Runner the edit flow uses for fetch/apply. + // Set in newPickerModel to edit.DefaultRunner; tests inject a stub. + editRunner edit.Runner } // New returns a fresh model pointed at the given client. ctx governs both @@ -495,10 +515,75 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.activePF = msg.pf m.endpoint = msg.endpoint + m.statusURL = msg.pf.StatusEndpoint() m.client = apiclient.New(m.endpoint) m.pane = paneSessions return m, m.initSessionView() + case edit.FetchedMsg: + if msg.Err != nil { + m.editState.phase = editPhaseError + m.editState.err = msg.Err.Error() + return m, nil + } + m.editState.fetched = msg.Fetched + m.editState.tempPath = msg.TempPath + m.editState.phase = editPhaseEditing + return m, openEditorCmd(msg.TempPath) + + case editorExitedMsg: + if msg.err != nil { + m.editState.phase = editPhaseError + m.editState.err = "editor exited: " + msg.err.Error() + return m, nil + } + edited, err := os.ReadFile(m.editState.tempPath) + if err != nil { + m.editState.phase = editPhaseError + m.editState.err = "read edited file: " + err.Error() + return m, nil + } + m.editState.editedRaw = edited + originalSubtree := m.editState.fetched.InnerYAML[m.editState.fetched.PipelineStart:m.editState.fetched.PipelineEnd] + if string(edited) == string(originalSubtree) { + m.editState = editState{phase: editPhaseDone} + return m, nil + } + if !validYAML(edited) { + m.editState.phase = editPhaseError + m.editState.err = "edited file is not valid YAML" + return m, nil + } + m.editState.diff = edit.Diff(originalSubtree, edited) + m.editState.phase = editPhaseDiff + return m, nil + + case edit.AppliedMsg: + if msg.Err != nil { + m.editState.phase = editPhaseError + m.editState.err = "apply failed: " + msg.Err.Error() + return m, nil + } + m.editState.applyTime = msg.ApplyTime + m.editState.phase = editPhaseWaiting + return m, edit.PollCmd(m.ctx, m.statusURL, msg.ApplyTime) + + case edit.PolledMsg: + switch msg.Result.Status { + case edit.PollSuccess: + m.editState = editState{phase: editPhaseDone} + return m, m.loadPipelineCmd() + case edit.PollFailure: + m.editState.phase = editPhaseError + m.editState.err = "reload failed: " + msg.Result.LastError + return m, nil + case edit.PollTimeout: + m.editState.phase = editPhaseError + m.editState.err = "reload not observed in 120s; check kubectl logs" + return m, nil + } + return m, nil + case tea.KeyMsg: return m, m.handleKey(msg) } @@ -573,6 +658,11 @@ sortAndRebuild: // View composes the full screen. func (m *model) View() string { + // Edit overlay takes over the screen while an edit is in flight. + if m.editState.phase != editPhaseDone { + return renderEditOverlay(m.editState, m.width, m.height) + } + if m.pane == paneNamespaces { title := "abctl · pick namespace" // m.namespaces == nil → still loading (don't flash the empty-state @@ -753,3 +843,22 @@ func Run(ctx context.Context, opts RunOptions) error { _, err := p.Run() return err } + +// openEditorCmd returns a tea.Cmd that suspends bubbletea, runs $EDITOR +// (vi if unset) on path, and emits editorExitedMsg when the editor exits. +func openEditorCmd(path string) tea.Cmd { + editor := os.Getenv("EDITOR") + if editor == "" { + editor = "vi" + } + c := exec.Command("sh", "-c", editor+" "+path) + return tea.ExecProcess(c, func(err error) tea.Msg { + return editorExitedMsg{err: err} + }) +} + +// validYAML returns true iff b parses as valid YAML. +func validYAML(b []byte) bool { + var v any + return yaml.Unmarshal(b, &v) == nil +} diff --git a/authbridge/cmd/abctl/tui/edit_e2e_test.go b/authbridge/cmd/abctl/tui/edit_e2e_test.go new file mode 100644 index 000000000..63e803b4c --- /dev/null +++ b/authbridge/cmd/abctl/tui/edit_e2e_test.go @@ -0,0 +1,198 @@ +package tui + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" +) + +const editFixtureCMYAML = `apiVersion: v1 +kind: ConfigMap +metadata: + name: authbridge-config-email-agent + namespace: team1 +data: + config.yaml: | + mode: proxy-sidecar + pipeline: + inbound: + - name: jwt-validation + session: + enabled: true +` + +// editFakeRunner records args + returns canned responses. +type editFakeRunner struct { + getResponse []byte + captured []string + applyManifest []byte +} + +func (f *editFakeRunner) run(ctx context.Context, args ...string) ([]byte, error) { + f.captured = append(f.captured, strings.Join(args, " ")) + if len(args) > 0 && args[0] == "get" { + return f.getResponse, nil + } + if len(args) > 0 && args[0] == "apply" { + path := args[len(args)-1] + b, err := os.ReadFile(path) + if err != nil { + return nil, err + } + f.applyManifest = b + return []byte("applied"), nil + } + return nil, nil +} + +// TestEditFlow_HappyPath drives the full state machine with stubs: +// e → fetch → editor → validate → diff → y → apply → poll → done. +// +// Bypasses the real $EDITOR by writing the "edited" content to the +// tempfile directly, then injecting editorExitedMsg{err: nil}. +func TestEditFlow_HappyPath(t *testing.T) { + runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "last_success_unix": int64(99999999999), + }) + })) + defer srv.Close() + + m := newPickerModel(context.Background(), nil, nil) + m.statusURL = srv.URL + m.editRunner = runner.run + m.selectedNamespace = "team1" + m.selectedPod = "email-agent" + m.pane = panePipeline + + // Press "e". + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) + mm := updated.(*model) + if mm.editState.phase != editPhaseFetching { + t.Fatalf("phase = %v, want editPhaseFetching", mm.editState.phase) + } + if cmd == nil { + t.Fatal("expected fetch Cmd") + } + + // Run FetchCmd. + fetchedMsg := cmd().(edit.FetchedMsg) + if fetchedMsg.Err != nil { + t.Fatalf("Fetch failed: %v", fetchedMsg.Err) + } + defer os.Remove(fetchedMsg.TempPath) + + // Bypass the editor: write a modified subtree directly. + editedSubtree := []byte("pipeline:\n inbound:\n - name: jwt-validation\n config:\n new_key: new_value\n") + if err := os.WriteFile(fetchedMsg.TempPath, editedSubtree, 0o600); err != nil { + t.Fatal(err) + } + + updated, _ = mm.Update(fetchedMsg) + mm = updated.(*model) + if mm.editState.phase != editPhaseEditing { + t.Fatalf("phase = %v, want editPhaseEditing", mm.editState.phase) + } + + // Inject editorExitedMsg directly (skips the real ExecProcess). + updated, _ = mm.Update(editorExitedMsg{err: nil}) + mm = updated.(*model) + if mm.editState.phase != editPhaseDiff { + t.Fatalf("phase = %v, want editPhaseDiff (validate should pass)", mm.editState.phase) + } + if mm.editState.diff == "" { + t.Fatal("diff should be populated") + } + + // Confirm with "y". + updated, cmd = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'y'}}) + mm = updated.(*model) + if mm.editState.phase != editPhaseApplying { + t.Fatalf("phase = %v, want editPhaseApplying", mm.editState.phase) + } + if cmd == nil { + t.Fatal("expected apply Cmd") + } + + // Run ApplyCmd. + appliedMsg := cmd().(edit.AppliedMsg) + if appliedMsg.Err != nil { + t.Fatalf("apply failed: %v", appliedMsg.Err) + } + + updated, cmd = mm.Update(appliedMsg) + mm = updated.(*model) + if mm.editState.phase != editPhaseWaiting { + t.Fatalf("phase = %v, want editPhaseWaiting", mm.editState.phase) + } + if cmd == nil { + t.Fatal("expected poll Cmd") + } + + // Run PollCmd. + polledMsg := cmd().(edit.PolledMsg) + if polledMsg.Result.Status != edit.PollSuccess { + t.Fatalf("poll status = %v, want PollSuccess", polledMsg.Result.Status) + } + + updated, _ = mm.Update(polledMsg) + mm = updated.(*model) + if mm.editState.phase != editPhaseDone { + t.Fatalf("phase = %v, want editPhaseDone", mm.editState.phase) + } + + // The applied manifest should contain the new content. + if !strings.Contains(string(runner.applyManifest), "new_key: new_value") { + t.Fatalf("manifest missing new content:\n%s", runner.applyManifest) + } +} + +// TestEditFlow_NCancelsAtDiff verifies "N" at the confirm prompt +// returns to panePipeline without applying. +func TestEditFlow_NCancelsAtDiff(t *testing.T) { + runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} + m := newPickerModel(context.Background(), nil, nil) + m.editRunner = runner.run + m.selectedNamespace = "team1" + m.selectedPod = "email-agent" + m.pane = panePipeline + + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) + mm := updated.(*model) + fetchedMsg := cmd().(edit.FetchedMsg) + defer os.Remove(fetchedMsg.TempPath) + + // Pretend the user edited. + editedSubtree := []byte("pipeline:\n inbound:\n - name: jwt-validation\n config:\n x: 1\n") + _ = os.WriteFile(fetchedMsg.TempPath, editedSubtree, 0o600) + + updated, _ = mm.Update(fetchedMsg) + mm = updated.(*model) + updated, _ = mm.Update(editorExitedMsg{err: nil}) + mm = updated.(*model) + if mm.editState.phase != editPhaseDiff { + t.Fatalf("setup: phase = %v, want editPhaseDiff", mm.editState.phase) + } + + // Press "N". + updated, _ = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'N'}}) + mm = updated.(*model) + if mm.editState.phase != editPhaseDone { + t.Fatalf("phase = %v, want editPhaseDone (N should cancel)", mm.editState.phase) + } + for _, c := range runner.captured { + if strings.HasPrefix(c, "apply") { + t.Fatalf("apply ran despite N: %q", c) + } + } +} diff --git a/authbridge/cmd/abctl/tui/keys.go b/authbridge/cmd/abctl/tui/keys.go index 430060238..95e455c86 100644 --- a/authbridge/cmd/abctl/tui/keys.go +++ b/authbridge/cmd/abctl/tui/keys.go @@ -5,6 +5,8 @@ import ( "time" tea "github.com/charmbracelet/bubbletea" + + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" ) // handleKey processes every key press. The filter-input overlay takes @@ -75,6 +77,11 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { return cmd } + // Edit overlay takes over key input while an edit is in flight. + if m.editState.phase != editPhaseDone { + return m.handleEditKey(msg) + } + // Filter-mode: input box consumes most keys. Esc cancels, Enter commits. if m.filtering { switch msg.String() { @@ -198,6 +205,16 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { } return nil + case "e": + if m.pane != panePipeline { + return nil + } + if m.editState.phase != editPhaseDone { + return nil // already editing + } + m.editState = editState{phase: editPhaseFetching} + return edit.FetchCmd(m.ctx, m.editRunner, m.selectedNamespace, m.selectedPod) + case "g": m.goTop() return nil @@ -320,9 +337,9 @@ func (m *model) helpView() string { return "[↑↓] scroll [y] yank [esc] back [q] quit" case panePipeline: if m.parentCtx != nil { - return "[↑↓] nav [↵] plugin detail [tab] sessions [esc] pods [q] quit" + return "[↑↓] nav [↵] plugin detail [e] edit [tab] sessions [esc] pods [q] quit" } - return "[↑↓] nav [↵] plugin detail [tab] sessions [q] quit" + return "[↑↓] nav [↵] plugin detail [e] edit [tab] sessions [q] quit" case panePluginDetail: return "[↑↓] scroll [esc] back [q] quit" } @@ -364,3 +381,48 @@ func (m *model) layout() { m.showDetail(m.detailEvent) } } + +// handleEditKey is the keymap that takes over while an edit is in flight. +func (m *model) handleEditKey(msg tea.KeyMsg) tea.Cmd { + switch m.editState.phase { + case editPhaseDiff: + switch msg.String() { + case "y", "Y": + m.editState.phase = editPhaseApplying + newSubtree := m.editState.editedRaw + newInner := edit.Splice( + m.editState.fetched.InnerYAML, + m.editState.fetched.PipelineStart, + m.editState.fetched.PipelineEnd, + newSubtree, + ) + manifest, err := edit.BuildManifest(m.editState.fetched.ConfigMapYAML, newInner) + if err != nil { + m.editState.phase = editPhaseError + m.editState.err = "build manifest: " + err.Error() + return nil + } + return edit.ApplyCmd(m.ctx, m.editRunner, manifest) + case "n", "N", "esc": + m.editState = editState{phase: editPhaseDone} + return nil + } + return nil + case editPhaseError: + switch msg.String() { + case "r": + m.editState.phase = editPhaseEditing + return openEditorCmd(m.editState.tempPath) + case "esc": + m.editState = editState{phase: editPhaseDone} + return nil + } + return nil + } + // Other phases: only Esc cancels (best-effort). + if msg.String() == "esc" { + m.editState = editState{phase: editPhaseDone} + return nil + } + return nil +} diff --git a/authbridge/cmd/abctl/tui/namespaces_pane.go b/authbridge/cmd/abctl/tui/namespaces_pane.go index 1c2728967..2c9c34427 100644 --- a/authbridge/cmd/abctl/tui/namespaces_pane.go +++ b/authbridge/cmd/abctl/tui/namespaces_pane.go @@ -12,6 +12,7 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/cluster" + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" ) // newNamespacesTable builds an empty namespaces picker table. @@ -78,5 +79,8 @@ func newPickerModel(ctx context.Context, lister cluster.Lister, pf cluster.PortF portForwarder: pf, namespacesTbl: newNamespacesTable(), podsTbl: newPodsTable(), + + // Edit flow: + editRunner: edit.DefaultRunner, } } From ca6a1692da9255b1a988fea8e9248f3838277ed8 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 06:54:27 -0400 Subject: [PATCH 12/24] docs(abctl): Document e keybind + add opt-in edit e2e test Adds keybinds table rows for the edit flow (e/y/N/r/Esc) and a new "Editing the pipeline" section explaining the four operations (edit/reorder/remove/add), the RBAC requirement (update configmaps), the tempfile lifecycle (left in place on every exit path), and the hot-reload wait window (~90s typical, 120s timeout). Adds edit/e2e_test.go gated behind -tags=e2e: drives Fetch against a real cluster (requires make demo-ibac). Skipped by default. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/README.md | 46 +++++++++++++++++++++++++++ authbridge/cmd/abctl/edit/e2e_test.go | 31 ++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 authbridge/cmd/abctl/edit/e2e_test.go diff --git a/authbridge/cmd/abctl/README.md b/authbridge/cmd/abctl/README.md index 2c2cb9294..b41a7efe4 100644 --- a/authbridge/cmd/abctl/README.md +++ b/authbridge/cmd/abctl/README.md @@ -89,9 +89,55 @@ The UI has three panes. `Enter` drills in; `Esc` backs out. | `p` | any | pause/resume stream | | `y` | detail | yank event JSON to `/tmp` | | `g` / `G` | lists | jump to top / bottom | +| `e` | pipeline | edit pipeline subtree in `$EDITOR` | +| `y` | edit/diff | apply the edit | +| `N` | edit/diff | abort the edit | +| `r` | edit/error | re-open the editor with the same tempfile | +| `Esc` | edit/* | abort the edit, return to Pipeline pane | | `?` | any | (reserved for future help overlay) | | `q` / `Ctrl+C` | any | quit | +## Editing the pipeline + +Press `e` on the Pipeline pane to edit the agent's runtime `pipeline:` +subtree in `$EDITOR` (or `vi` if unset). On save, abctl shows a diff +and asks `apply this change? (y/N)`. Confirming runs +`kubectl apply --server-side` against the per-agent ConfigMap, then +polls the framework's `/reload/status` until the reload completes +(success or failure). + +The single edit flow covers four operations: +- **Edit a value** — change a config field of an existing plugin +- **Reorder** — move a plugin's lines up or down +- **Remove** — delete a plugin's entry from `inbound:` or `outbound:` +- **Add** — append a new plugin entry + +All four work because they're all just lines you change inside the +pipeline subtree. + +### Permissions + +abctl shells out to `kubectl`; kubectl uses your kubeconfig. Editing +requires `update` on `configmaps` in the agent's namespace (in +addition to `get pods` which the picker already needs). RBAC denial +surfaces verbatim in the overlay. + +### Tempfile lifecycle + +abctl writes the editable pipeline subtree to `$TMPDIR/abctl-pipeline-*.yaml` +on every edit. The tempfile is **left in place on every exit path** +(success, error, abort) so an interrupted edit is recoverable. Clean +up `/tmp/abctl-pipeline-*` periodically. + +### Hot-reload window + +The framework reloads via a config-file watcher; kubelet syncs +ConfigMap edits into the pod's mount within ~60s, then the framework +debounces and reloads. Total wall-clock from `apply` to reload is +typically under 90s. abctl shows a spinner during the wait. If +`/reload/status` doesn't observe a successful reload within 120s, +abctl gives up watching and tells you to check `kubectl logs deploy/`. + ## Trust model `abctl` does no authentication — same as the server. Use only against diff --git a/authbridge/cmd/abctl/edit/e2e_test.go b/authbridge/cmd/abctl/edit/e2e_test.go new file mode 100644 index 000000000..a3715e41f --- /dev/null +++ b/authbridge/cmd/abctl/edit/e2e_test.go @@ -0,0 +1,31 @@ +//go:build e2e + +package edit + +import ( + "context" + "strings" + "testing" + "time" +) + +// TestE2E_FetchExistingAgent verifies Fetch works against a real cluster. +// Requires `make demo-ibac` to have run. +// +// Run: +// go test -tags=e2e ./edit/ -run TestE2E_FetchExistingAgent -v +func TestE2E_FetchExistingAgent(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + fp, err := Fetch(ctx, DefaultRunner, "team1", "email-agent") + if err != nil { + t.Fatalf("Fetch: %v", err) + } + subtree := fp.InnerYAML[fp.PipelineStart:fp.PipelineEnd] + if !strings.Contains(string(subtree), "pipeline:") { + t.Fatalf("subtree missing header: %s", subtree) + } + if fp.PipelineEnd <= fp.PipelineStart { + t.Fatalf("invalid range: [%d, %d)", fp.PipelineStart, fp.PipelineEnd) + } +} From c6b57dc09476269477b54033cacc32f67fbe7dd1 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 07:05:08 -0400 Subject: [PATCH 13/24] refactor(abctl): Address final review on the pipeline editor Five fixes from the whole-branch review: 1. Splice: tui/app.go's editorExitedMsg handler now normalizes the user's edit to end with a newline before splicing. Without this, a missing trailing newline concatenated the last edit line with the next top-level YAML key, producing garbage that passed per- subtree validation but broke the spliced inner. Regression tests in edit/configmap_test.go and tui/edit_e2e_test.go. 2. validYAML on empty input: rejected explicitly. An empty pipeline subtree silently wiped the framework's pipeline; now surfaces as an error overlay with [Esc] to abort. 3. No-changes feedback: editorExitedMsg now flashes 'no changes; nothing to apply' before transitioning to editPhaseDone, matching spec acceptance criterion #2. 4. YAML parse errors: the parser's line/col now propagates to the error overlay (e.g. 'invalid YAML: yaml: line 5: ...'), matching spec criterion #9. validYAML helper deleted (inline parse). 5. gofmt the three new files (edit/configmap.go, edit/e2e_test.go, tui/picker_test.go). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/configmap.go | 2 +- authbridge/cmd/abctl/edit/configmap_test.go | 39 +++++++++++++++++++++ authbridge/cmd/abctl/edit/e2e_test.go | 3 +- authbridge/cmd/abctl/tui/app.go | 25 +++++++++---- authbridge/cmd/abctl/tui/edit_e2e_test.go | 39 +++++++++++++++++++++ authbridge/cmd/abctl/tui/picker_test.go | 2 +- 6 files changed, 100 insertions(+), 10 deletions(-) diff --git a/authbridge/cmd/abctl/edit/configmap.go b/authbridge/cmd/abctl/edit/configmap.go index d43ba43e7..d7980605f 100644 --- a/authbridge/cmd/abctl/edit/configmap.go +++ b/authbridge/cmd/abctl/edit/configmap.go @@ -57,7 +57,7 @@ func FindPipelineRange(innerYAML []byte) (start, end int, err error) { } pipelineKeyLine := doc.Content[pipelineKeyIdx].Line // 1-indexed - var nextKeyLine int // 1-indexed; 0 if pipeline is last + var nextKeyLine int // 1-indexed; 0 if pipeline is last if pipelineKeyIdx+2 < len(doc.Content) { nextKeyLine = doc.Content[pipelineKeyIdx+2].Line } diff --git a/authbridge/cmd/abctl/edit/configmap_test.go b/authbridge/cmd/abctl/edit/configmap_test.go index d1fa0485e..0123cc8f6 100644 --- a/authbridge/cmd/abctl/edit/configmap_test.go +++ b/authbridge/cmd/abctl/edit/configmap_test.go @@ -1,6 +1,7 @@ package edit import ( + "bytes" "context" "fmt" "os" @@ -266,6 +267,44 @@ func TestApply_PassesManifest(t *testing.T) { } } +// TestSplice_HandlesMissingTrailingNewline documents the contract: callers MUST +// normalize trailing '\n' before calling Splice. Without normalization the last +// line of the new subtree concatenates with the next top-level YAML key, +// producing garbage that passes per-subtree validation but breaks the full +// inner YAML. The normalization is performed by tui/app.go's editorExitedMsg +// handler before invoking Splice. +func TestSplice_HandlesMissingTrailingNewline(t *testing.T) { + const orig = `mode: proxy-sidecar +pipeline: + inbound: + - name: a + +session: + enabled: true +` + start, end, err := FindPipelineRange([]byte(orig)) + if err != nil { + t.Fatal(err) + } + // newSubtree deliberately missing trailing newline. + newSubtree := []byte("pipeline:\n inbound:\n - name: b") + spliced := Splice([]byte(orig), start, end, newSubtree) + // The TUI's editorExitedMsg handler is responsible for appending \n + // before splicing, so a Splice given non-newline-terminated input + // produces concatenated garbage. This test documents the contract: + // callers MUST normalize trailing \n before calling Splice. The + // normalization happens in tui/app.go. + if !bytes.HasSuffix(spliced, []byte("session:\n enabled: true\n")) { + // The garbage we're guarding against in the TUI: + // Verify that without normalization, the splice DOES produce + // garbage joining "name: b" to "session:". This ensures the + // contract documented in Splice's godoc is observable. + if !bytes.Contains(spliced, []byte("- name: bsession:")) { + t.Fatalf("expected to observe the splice-without-normalization garbage; got:\n%s", spliced) + } + } +} + // equalArgs checks two []string for equality. func equalArgs(a, b []string) bool { if len(a) != len(b) { diff --git a/authbridge/cmd/abctl/edit/e2e_test.go b/authbridge/cmd/abctl/edit/e2e_test.go index a3715e41f..2ed89eee5 100644 --- a/authbridge/cmd/abctl/edit/e2e_test.go +++ b/authbridge/cmd/abctl/edit/e2e_test.go @@ -13,7 +13,8 @@ import ( // Requires `make demo-ibac` to have run. // // Run: -// go test -tags=e2e ./edit/ -run TestE2E_FetchExistingAgent -v +// +// go test -tags=e2e ./edit/ -run TestE2E_FetchExistingAgent -v func TestE2E_FetchExistingAgent(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() diff --git a/authbridge/cmd/abctl/tui/app.go b/authbridge/cmd/abctl/tui/app.go index ca86ffc9d..968cba832 100644 --- a/authbridge/cmd/abctl/tui/app.go +++ b/authbridge/cmd/abctl/tui/app.go @@ -1,6 +1,7 @@ package tui import ( + "bytes" "context" "encoding/json" "fmt" @@ -543,15 +544,30 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.editState.err = "read edited file: " + err.Error() return m, nil } + // Fix 2: reject empty input before it can silently wipe the pipeline. + if len(bytes.TrimSpace(edited)) == 0 { + m.editState.phase = editPhaseError + m.editState.err = "edited file is empty; press [Esc] to abort" + return m, nil + } + // Fix 1: normalize trailing newline so Splice doesn't concatenate the + // last edit line with the next top-level YAML key. + if len(edited) > 0 && edited[len(edited)-1] != '\n' { + edited = append(edited, '\n') + } m.editState.editedRaw = edited originalSubtree := m.editState.fetched.InnerYAML[m.editState.fetched.PipelineStart:m.editState.fetched.PipelineEnd] + // Fix 3: surface "no changes" rather than silently transitioning to done. if string(edited) == string(originalSubtree) { + m.setFlash("no changes; nothing to apply") m.editState = editState{phase: editPhaseDone} return m, nil } - if !validYAML(edited) { + // Fix 4: preserve YAML parse error line/col for the overlay. + var yamlVal any + if err := yaml.Unmarshal(edited, &yamlVal); err != nil { m.editState.phase = editPhaseError - m.editState.err = "edited file is not valid YAML" + m.editState.err = "invalid YAML: " + err.Error() return m, nil } m.editState.diff = edit.Diff(originalSubtree, edited) @@ -857,8 +873,3 @@ func openEditorCmd(path string) tea.Cmd { }) } -// validYAML returns true iff b parses as valid YAML. -func validYAML(b []byte) bool { - var v any - return yaml.Unmarshal(b, &v) == nil -} diff --git a/authbridge/cmd/abctl/tui/edit_e2e_test.go b/authbridge/cmd/abctl/tui/edit_e2e_test.go index 63e803b4c..594249fa7 100644 --- a/authbridge/cmd/abctl/tui/edit_e2e_test.go +++ b/authbridge/cmd/abctl/tui/edit_e2e_test.go @@ -196,3 +196,42 @@ func TestEditFlow_NCancelsAtDiff(t *testing.T) { } } } + +// TestEditFlow_NormalizesTrailingNewline verifies that the +// editorExitedMsg handler appends a trailing newline to the user's +// edit if missing — preventing the last line of the new subtree +// from concatenating with the next top-level YAML key. +func TestEditFlow_NormalizesTrailingNewline(t *testing.T) { + runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} + m := newPickerModel(context.Background(), nil, nil) + m.editRunner = runner.run + m.selectedNamespace = "team1" + m.selectedPod = "email-agent" + m.pane = panePipeline + + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) + mm := updated.(*model) + fetchedMsg := cmd().(edit.FetchedMsg) + defer os.Remove(fetchedMsg.TempPath) + + // Write an edit deliberately missing a trailing newline. + edited := []byte("pipeline:\n inbound:\n - name: jwt-validation\n config:\n x: 1") + if edited[len(edited)-1] == '\n' { + t.Fatal("test fixture should be missing trailing newline") + } + if err := os.WriteFile(fetchedMsg.TempPath, edited, 0o600); err != nil { + t.Fatal(err) + } + + updated, _ = mm.Update(fetchedMsg) + mm = updated.(*model) + updated, _ = mm.Update(editorExitedMsg{err: nil}) + mm = updated.(*model) + if mm.editState.phase != editPhaseDiff { + t.Fatalf("phase = %v, want editPhaseDiff", mm.editState.phase) + } + if len(mm.editState.editedRaw) == 0 || mm.editState.editedRaw[len(mm.editState.editedRaw)-1] != '\n' { + t.Fatalf("editedRaw should be normalized to end with newline; got %q", + mm.editState.editedRaw) + } +} diff --git a/authbridge/cmd/abctl/tui/picker_test.go b/authbridge/cmd/abctl/tui/picker_test.go index ee28dd2b4..59197b951 100644 --- a/authbridge/cmd/abctl/tui/picker_test.go +++ b/authbridge/cmd/abctl/tui/picker_test.go @@ -263,7 +263,7 @@ type fakePortForward struct { func (p *fakePortForward) Endpoint() string { return p.endpoint } func (p *fakePortForward) StatusEndpoint() string { return "" } // unused by current picker tests -func (p *fakePortForward) Close() error { p.parent.closeCount++; return nil } +func (p *fakePortForward) Close() error { p.parent.closeCount++; return nil } func TestPodEnterStartsPortForwardAndTransitions(t *testing.T) { pf := &fakePortForwarder{endpoint: "http://127.0.0.1:60000"} From 9dfb2e2d3868016b932d37a4b2ac07daa70d31f3 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 07:55:55 -0400 Subject: [PATCH 14/24] fix(abctl): Resolve agent name from pod label before ConfigMap fetch The Pipeline pane's "e" keybind was passing the selected pod name directly into the authbridge-config- lookup, producing NotFound errors like: configmaps "authbridge-config-email-agent-779bb85688-4x8zg" not found The ConfigMap is named after the agent (Deployment), not the pod. Add ResolveAgentName which queries the pod's app.kubernetes.io/name label (with a strip-last-two-segments fallback) and call it from FetchCmd before constructing the ConfigMap name. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/configmap.go | 22 ++++++++++++++++ authbridge/cmd/abctl/edit/configmap_test.go | 29 +++++++++++++++++++++ authbridge/cmd/abctl/edit/edit.go | 9 +++++-- authbridge/cmd/abctl/tui/edit_e2e_test.go | 4 +++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/authbridge/cmd/abctl/edit/configmap.go b/authbridge/cmd/abctl/edit/configmap.go index d7980605f..5ddaba5c5 100644 --- a/authbridge/cmd/abctl/edit/configmap.go +++ b/authbridge/cmd/abctl/edit/configmap.go @@ -183,6 +183,28 @@ type FetchedPipeline struct { PipelineEnd int // byte offset where the subtree ends } +// ResolveAgentName looks up the agent name for a pod via its +// app.kubernetes.io/name label, which matches the per-agent ConfigMap +// suffix (authbridge-config-). Falls back to stripping the last +// two dash-separated segments (the ReplicaSet hash + pod suffix) when +// the label is absent. +func ResolveAgentName(ctx context.Context, run Runner, namespace, pod string) (string, error) { + out, err := run(ctx, "get", "pod", pod, "-n", namespace, + "-o", "jsonpath={.metadata.labels.app\\.kubernetes\\.io/name}") + if err != nil { + return "", err + } + name := strings.TrimSpace(string(out)) + if name != "" { + return name, nil + } + parts := strings.Split(pod, "-") + if len(parts) >= 3 { + return strings.Join(parts[:len(parts)-2], "-"), nil + } + return pod, nil +} + // Fetch reads the per-agent ConfigMap (authbridge-config-), extracts // the inner runtime YAML from data.config.yaml, and locates the pipeline // subtree's byte range. Returns an error if the ConfigMap doesn't exist, diff --git a/authbridge/cmd/abctl/edit/configmap_test.go b/authbridge/cmd/abctl/edit/configmap_test.go index 0123cc8f6..43152760c 100644 --- a/authbridge/cmd/abctl/edit/configmap_test.go +++ b/authbridge/cmd/abctl/edit/configmap_test.go @@ -305,6 +305,35 @@ session: } } +func TestResolveAgentName_FromLabel(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + if len(args) < 2 || args[0] != "get" || args[1] != "pod" { + t.Fatalf("unexpected args: %v", args) + } + return []byte("email-agent\n"), nil + } + got, err := ResolveAgentName(context.Background(), stub, "team1", "email-agent-779bb85688-4x8zg") + if err != nil { + t.Fatalf("ResolveAgentName: %v", err) + } + if got != "email-agent" { + t.Fatalf("got %q want %q", got, "email-agent") + } +} + +func TestResolveAgentName_FallbackToPodSuffixStrip(t *testing.T) { + stub := func(ctx context.Context, args ...string) ([]byte, error) { + return []byte(""), nil // label absent + } + got, err := ResolveAgentName(context.Background(), stub, "team1", "email-agent-779bb85688-4x8zg") + if err != nil { + t.Fatalf("ResolveAgentName: %v", err) + } + if got != "email-agent" { + t.Fatalf("got %q want %q (RS-hash + pod-suffix should be stripped)", got, "email-agent") + } +} + // equalArgs checks two []string for equality. func equalArgs(a, b []string) bool { if len(a) != len(b) { diff --git a/authbridge/cmd/abctl/edit/edit.go b/authbridge/cmd/abctl/edit/edit.go index 5c0aeac8b..71ef31659 100644 --- a/authbridge/cmd/abctl/edit/edit.go +++ b/authbridge/cmd/abctl/edit/edit.go @@ -16,13 +16,18 @@ type FetchedMsg struct { Err error } -// FetchCmd returns a tea.Cmd that fetches the agent's ConfigMap, locates +// FetchCmd returns a tea.Cmd that resolves the pod's agent name (via the +// app.kubernetes.io/name label), fetches the agent's ConfigMap, locates // the pipeline subtree, writes the subtree to a tempfile (ready for // $EDITOR), and emits FetchedMsg. The tempfile lives in $TMPDIR; abctl // leaves it in place on every exit path (success, error, abort) so users // can recover an in-progress edit. -func FetchCmd(ctx context.Context, run Runner, namespace, agent string) tea.Cmd { +func FetchCmd(ctx context.Context, run Runner, namespace, pod string) tea.Cmd { return func() tea.Msg { + agent, err := ResolveAgentName(ctx, run, namespace, pod) + if err != nil { + return FetchedMsg{Err: err} + } fp, err := Fetch(ctx, run, namespace, agent) if err != nil { return FetchedMsg{Err: err} diff --git a/authbridge/cmd/abctl/tui/edit_e2e_test.go b/authbridge/cmd/abctl/tui/edit_e2e_test.go index 594249fa7..d24b51828 100644 --- a/authbridge/cmd/abctl/tui/edit_e2e_test.go +++ b/authbridge/cmd/abctl/tui/edit_e2e_test.go @@ -39,6 +39,10 @@ type editFakeRunner struct { func (f *editFakeRunner) run(ctx context.Context, args ...string) ([]byte, error) { f.captured = append(f.captured, strings.Join(args, " ")) if len(args) > 0 && args[0] == "get" { + // Pod-label lookup for ResolveAgentName: kubectl get pod ... -o jsonpath=... + if len(args) >= 2 && args[1] == "pod" { + return []byte("email-agent"), nil + } return f.getResponse, nil } if len(args) > 0 && args[0] == "apply" { From f7b46c4038643ab51829b142b03232e15f43eb85 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 07:59:53 -0400 Subject: [PATCH 15/24] fix(abctl): Take SSA field-manager ownership on pipeline edit The kagenti-operator's webhook is the field-manager for data.config.yaml on initial ConfigMap creation. Without --force-conflicts, kubectl SSA aborts every abctl edit with a multi-line "conflict with kagenti-webhook" error. The user has explicitly confirmed the change at the diff prompt, so the intent IS to take ownership. Switch Apply to use a dedicated --field-manager=abctl with --force-conflicts=true so subsequent edits land cleanly. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/configmap.go | 12 ++++++++++-- authbridge/cmd/abctl/edit/configmap_test.go | 14 +++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/authbridge/cmd/abctl/edit/configmap.go b/authbridge/cmd/abctl/edit/configmap.go index 5ddaba5c5..8d7619ea2 100644 --- a/authbridge/cmd/abctl/edit/configmap.go +++ b/authbridge/cmd/abctl/edit/configmap.go @@ -254,7 +254,12 @@ func extractInnerYAML(cmYAML []byte) ([]byte, error) { return nil, fmt.Errorf("ConfigMap data has no config.yaml key") } -// Apply writes manifest to a tempfile and runs kubectl apply --server-side. +// Apply writes manifest to a tempfile and runs kubectl apply --server-side +// with --force-conflicts=true and a dedicated abctl field-manager. The +// kagenti-operator's webhook owns data.config.yaml on initial creation; +// the user has explicitly confirmed this edit by pressing "y" at the +// diff prompt, so taking field-manager ownership is the intended outcome. +// // Returns the wall-clock time at which the apply call started; the caller // uses this to compare against /reload/status's last_success_unix to know // whether the framework has picked up the change yet. @@ -272,7 +277,10 @@ func Apply(ctx context.Context, run Runner, manifest []byte) (time.Time, error) return time.Time{}, fmt.Errorf("close temp manifest: %w", err) } applyTime := time.Now() - if _, err := run(ctx, "apply", "--server-side", "--force-conflicts=false", "-f", tmp.Name()); err != nil { + if _, err := run(ctx, "apply", "--server-side", + "--field-manager=abctl", + "--force-conflicts=true", + "-f", tmp.Name()); err != nil { return time.Time{}, err } return applyTime, nil diff --git a/authbridge/cmd/abctl/edit/configmap_test.go b/authbridge/cmd/abctl/edit/configmap_test.go index 43152760c..ffd08bbef 100644 --- a/authbridge/cmd/abctl/edit/configmap_test.go +++ b/authbridge/cmd/abctl/edit/configmap_test.go @@ -239,10 +239,22 @@ func TestFetch_KubectlError(t *testing.T) { func TestApply_PassesManifest(t *testing.T) { captured := make([]byte, 0) stub := func(ctx context.Context, args ...string) ([]byte, error) { - // Args should be: apply --server-side --force-conflicts=false -f + // Args should be: apply --server-side --field-manager=abctl --force-conflicts=true -f if len(args) < 4 || args[0] != "apply" || args[1] != "--server-side" { t.Fatalf("kubectl args: %v", args) } + hasFM, hasForce := false, false + for _, a := range args { + if a == "--field-manager=abctl" { + hasFM = true + } + if a == "--force-conflicts=true" { + hasForce = true + } + } + if !hasFM || !hasForce { + t.Fatalf("expected --field-manager=abctl and --force-conflicts=true; got %v", args) + } path := args[len(args)-1] b, err := os.ReadFile(path) if err != nil { From 72e8f373574ff7a674fc6d89e6f196651d783dac Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 08:03:57 -0400 Subject: [PATCH 16/24] fix(abctl): Strip server-managed metadata before SSA apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fetched ConfigMap manifest carries resourceVersion, uid, creationTimestamp, managedFields, and generation from the server. Sending those back through `kubectl apply --server-side` produces intermittent 409 ("the object has been modified") errors whenever any controller has touched the CM since our Fetch — common with the kagenti-operator reconciling agent state between the user opening the editor and pressing y. BuildManifest now drops those keys from metadata before emitting the manifest. SSA reconstructs them server-side. Apply becomes robust against concurrent operator writes, matching the optimistic- concurrency-free model SSA was designed for. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/configmap.go | 28 +++++++++++++++++ authbridge/cmd/abctl/edit/configmap_test.go | 34 +++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/authbridge/cmd/abctl/edit/configmap.go b/authbridge/cmd/abctl/edit/configmap.go index 8d7619ea2..74c4eab3e 100644 --- a/authbridge/cmd/abctl/edit/configmap.go +++ b/authbridge/cmd/abctl/edit/configmap.go @@ -149,6 +149,34 @@ func BuildManifest(origCMYAML, newInner []byte) ([]byte, error) { configValueNode.Style = yaml.LiteralStyle configValueNode.Value = string(newInner) + // Strip server-managed metadata fields. Including a stale + // resourceVersion turns a server-side apply into a 409 ("object has + // been modified") whenever any controller has touched the CM since + // our Fetch. uid / creationTimestamp / managedFields / generation + // must not flow back either; SSA reconstructs them server-side. + for i := 0; i < len(doc.Content); i += 2 { + if doc.Content[i].Value != "metadata" { + continue + } + md := doc.Content[i+1] + if md.Kind != yaml.MappingNode { + break + } + stripped := md.Content[:0] + for j := 0; j < len(md.Content); j += 2 { + k := md.Content[j].Value + switch k { + case "resourceVersion", "uid", "creationTimestamp", + "managedFields", "generation", "selfLink": + // drop both the key and the value + continue + } + stripped = append(stripped, md.Content[j], md.Content[j+1]) + } + md.Content = stripped + break + } + out, err := yaml.Marshal(&root) if err != nil { return nil, fmt.Errorf("emit ConfigMap manifest: %w", err) diff --git a/authbridge/cmd/abctl/edit/configmap_test.go b/authbridge/cmd/abctl/edit/configmap_test.go index ffd08bbef..2775feb76 100644 --- a/authbridge/cmd/abctl/edit/configmap_test.go +++ b/authbridge/cmd/abctl/edit/configmap_test.go @@ -317,6 +317,40 @@ session: } } +func TestBuildManifest_StripsServerManagedMetadata(t *testing.T) { + const fetched = `apiVersion: v1 +kind: ConfigMap +metadata: + name: authbridge-config-email-agent + namespace: team1 + resourceVersion: "12345" + uid: abc-123 + creationTimestamp: "2026-05-29T00:00:00Z" + generation: 7 + managedFields: + - manager: kagenti-webhook + operation: Apply +data: + config.yaml: | + pipeline: + inbound: + - name: jwt-validation +` + out, err := BuildManifest([]byte(fetched), []byte("pipeline:\n inbound:\n - name: jwt-validation\n")) + if err != nil { + t.Fatalf("BuildManifest: %v", err) + } + outS := string(out) + for _, drop := range []string{"resourceVersion", "uid:", "creationTimestamp", "managedFields", "generation:"} { + if strings.Contains(outS, drop) { + t.Fatalf("manifest should not contain %q (would break SSA):\n%s", drop, outS) + } + } + if !strings.Contains(outS, "name: authbridge-config-email-agent") || !strings.Contains(outS, "namespace: team1") { + t.Fatalf("manifest lost name/namespace:\n%s", outS) + } +} + func TestResolveAgentName_FromLabel(t *testing.T) { stub := func(ctx context.Context, args ...string) ([]byte, error) { if len(args) < 2 || args[0] != "get" || args[1] != "pod" { From 8c4724e38ae2942a531d3e7618f16e2e0a10dfcd Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 08:27:48 -0400 Subject: [PATCH 17/24] feat(abctl): Auto-rollback ConfigMap on failed in-pod reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user applies a pipeline edit but the framework's reload fails in-pod (build error from an unknown plugin name, malformed config, etc.), the running pipeline is unaffected (the framework keeps the previous one) — but the ConfigMap on disk now holds the bad YAML. The next "e" reopens the bad config and the user has to manually fix it. Add an editPhaseRollback that fires on PollFailure or PollTimeout: re-apply the original ConfigMap content (built from the InnerYAML captured at Fetch time) so the on-disk CM matches the still-running previous pipeline. The error overlay then reports "reload failed: ; rolled back to previous ConfigMap". If the rollback Apply itself fails the error message escalates to flag that the CM and running pipeline are now out of sync. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/edit.go | 20 +++++++ authbridge/cmd/abctl/tui/app.go | 38 ++++++++++--- authbridge/cmd/abctl/tui/edit_e2e_test.go | 66 +++++++++++++++++++++++ authbridge/cmd/abctl/tui/edit_overlay.go | 5 ++ 4 files changed, 121 insertions(+), 8 deletions(-) diff --git a/authbridge/cmd/abctl/edit/edit.go b/authbridge/cmd/abctl/edit/edit.go index 71ef31659..36af5f4ee 100644 --- a/authbridge/cmd/abctl/edit/edit.go +++ b/authbridge/cmd/abctl/edit/edit.go @@ -64,6 +64,26 @@ func ApplyCmd(ctx context.Context, run Runner, manifest []byte) tea.Cmd { } } +// RolledBackMsg is the result of RollbackCmd. ReloadErr is the error +// from the failed in-pod reload (the reason we're rolling back); Err +// is any error from the rollback Apply itself. +type RolledBackMsg struct { + ReloadErr string + Err error +} + +// RollbackCmd re-applies the supplied (original) manifest to undo a +// successful API write whose subsequent in-pod reload failed. The +// running pipeline never moved (the framework keeps the previous +// pipeline on build failure), so this just reconciles the ConfigMap +// on disk back to what's actually serving. +func RollbackCmd(ctx context.Context, run Runner, manifest []byte, reloadErr string) tea.Cmd { + return func() tea.Msg { + _, err := Apply(ctx, run, manifest) + return RolledBackMsg{ReloadErr: reloadErr, Err: err} + } +} + // PolledMsg is the result of PollCmd. type PolledMsg struct { Result PollResult diff --git a/authbridge/cmd/abctl/tui/app.go b/authbridge/cmd/abctl/tui/app.go index 968cba832..7ac1db525 100644 --- a/authbridge/cmd/abctl/tui/app.go +++ b/authbridge/cmd/abctl/tui/app.go @@ -589,15 +589,38 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case edit.PollSuccess: m.editState = editState{phase: editPhaseDone} return m, m.loadPipelineCmd() - case edit.PollFailure: - m.editState.phase = editPhaseError - m.editState.err = "reload failed: " + msg.Result.LastError - return m, nil - case edit.PollTimeout: - m.editState.phase = editPhaseError - m.editState.err = "reload not observed in 120s; check kubectl logs" + case edit.PollFailure, edit.PollTimeout: + // In-pod reload didn't take. The running pipeline is still + // the previous one; reconcile the ConfigMap back to match. + reason := msg.Result.LastError + if msg.Result.Status == edit.PollTimeout { + reason = "reload not observed in 120s" + } + origManifest, mErr := edit.BuildManifest( + m.editState.fetched.ConfigMapYAML, + m.editState.fetched.InnerYAML, + ) + if mErr != nil { + m.editState.phase = editPhaseError + m.editState.err = "reload failed: " + reason + + "\n(rollback build failed: " + mErr.Error() + ")" + return m, nil + } + m.editState.phase = editPhaseRollback + return m, edit.RollbackCmd(m.ctx, m.editRunner, origManifest, reason) + } + return m, nil + + case edit.RolledBackMsg: + m.editState.phase = editPhaseError + if msg.Err != nil { + m.editState.err = "reload failed: " + msg.ReloadErr + + "\nrollback also failed: " + msg.Err.Error() + + "\nConfigMap and running pipeline are out of sync; check kubectl" return m, nil } + m.editState.err = "reload failed: " + msg.ReloadErr + + "\nrolled back to previous ConfigMap" return m, nil case tea.KeyMsg: @@ -872,4 +895,3 @@ func openEditorCmd(path string) tea.Cmd { return editorExitedMsg{err: err} }) } - diff --git a/authbridge/cmd/abctl/tui/edit_e2e_test.go b/authbridge/cmd/abctl/tui/edit_e2e_test.go index d24b51828..dff6b0768 100644 --- a/authbridge/cmd/abctl/tui/edit_e2e_test.go +++ b/authbridge/cmd/abctl/tui/edit_e2e_test.go @@ -239,3 +239,69 @@ func TestEditFlow_NormalizesTrailingNewline(t *testing.T) { mm.editState.editedRaw) } } + +// TestEditFlow_RollbackOnReloadFailure verifies that when the in-pod +// reload fails (PollFailure), abctl re-applies the original ConfigMap +// content so the on-disk CM matches the still-running previous pipeline. +func TestEditFlow_RollbackOnReloadFailure(t *testing.T) { + runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} + m := newPickerModel(context.Background(), nil, nil) + m.editRunner = runner.run + m.selectedNamespace = "team1" + m.selectedPod = "email-agent" + m.pane = panePipeline + + // Drive through fetch → editor → diff → apply. + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) + mm := updated.(*model) + fetchedMsg := cmd().(edit.FetchedMsg) + defer os.Remove(fetchedMsg.TempPath) + + editedSubtree := []byte("pipeline:\n inbound:\n - name: bogus\n") + _ = os.WriteFile(fetchedMsg.TempPath, editedSubtree, 0o600) + + updated, _ = mm.Update(fetchedMsg) + mm = updated.(*model) + updated, _ = mm.Update(editorExitedMsg{err: nil}) + mm = updated.(*model) + updated, cmd = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'y'}}) + mm = updated.(*model) + appliedMsg := cmd().(edit.AppliedMsg) + updated, _ = mm.Update(appliedMsg) + mm = updated.(*model) + + // Inject a PollFailure manually. + failure := edit.PolledMsg{Result: edit.PollResult{ + Status: edit.PollFailure, + LastError: "unknown plugin: bogus", + }} + updated, cmd = mm.Update(failure) + mm = updated.(*model) + if mm.editState.phase != editPhaseRollback { + t.Fatalf("phase = %v, want editPhaseRollback", mm.editState.phase) + } + if cmd == nil { + t.Fatal("expected RollbackCmd") + } + + rolledBack := cmd().(edit.RolledBackMsg) + if rolledBack.Err != nil { + t.Fatalf("rollback Apply error: %v", rolledBack.Err) + } + updated, _ = mm.Update(rolledBack) + mm = updated.(*model) + if mm.editState.phase != editPhaseError { + t.Fatalf("phase = %v, want editPhaseError after rollback", mm.editState.phase) + } + if !strings.Contains(mm.editState.err, "rolled back") { + t.Fatalf("error should mention rollback; got %q", mm.editState.err) + } + // The rollback Apply (the LAST apply call) should have the + // ORIGINAL pipeline content, not the bogus one. + if strings.Contains(string(runner.applyManifest), "name: bogus") { + t.Fatalf("rollback manifest still contains bogus content:\n%s", runner.applyManifest) + } + if !strings.Contains(string(runner.applyManifest), "name: jwt-validation") { + t.Fatalf("rollback manifest missing original content:\n%s", runner.applyManifest) + } +} diff --git a/authbridge/cmd/abctl/tui/edit_overlay.go b/authbridge/cmd/abctl/tui/edit_overlay.go index 6f52b67cc..d0f5fd245 100644 --- a/authbridge/cmd/abctl/tui/edit_overlay.go +++ b/authbridge/cmd/abctl/tui/edit_overlay.go @@ -21,6 +21,7 @@ const ( editPhaseDiff editPhaseApplying editPhaseWaiting + editPhaseRollback // re-applying the original ConfigMap after a failed reload editPhaseError ) @@ -75,6 +76,10 @@ func renderEditOverlay(s editState, width, height int) string { b.WriteString("Waiting for hot-reload…") b.WriteString("\n") b.WriteString(styleHint.Render("(this can take up to 120s while kubelet syncs the ConfigMap)")) + case editPhaseRollback: + b.WriteString(styleTitle.Render("Edit pipeline — rolling back")) + b.WriteString("\n\n") + b.WriteString("Reload failed. Restoring previous ConfigMap…") case editPhaseError: b.WriteString(styleTitle.Render("Edit pipeline — error")) b.WriteString("\n\n") From 9d004700c86b9c23e856902e39fbbecd3098ccca Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 08:30:54 -0400 Subject: [PATCH 18/24] fix(abctl): Drop late edit-flow messages after user aborts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user presses Esc during the edit flow's async phases (Fetching, Editing, Applying, Waiting, Rollback), editState is reset to Done and fetched is cleared. But the in-flight tea.Cmd is still running and eventually returns its msg to Update(). Each handler was unconditionally writing to editState — and PolledMsg / RolledBackMsg deref editState.fetched, which is now nil. Result: nil-pointer panic. Gate every async edit-flow message handler on the expected current phase (and on fetched != nil where used). Late messages are dropped silently. Adds TestEditFlow_LatePolledMsgAfterAbort as a regression — sends a PolledMsg with editState in its zero/Done state and asserts no panic. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/tui/app.go | 19 +++++++++++++++++++ authbridge/cmd/abctl/tui/edit_e2e_test.go | 22 ++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/authbridge/cmd/abctl/tui/app.go b/authbridge/cmd/abctl/tui/app.go index 7ac1db525..799421506 100644 --- a/authbridge/cmd/abctl/tui/app.go +++ b/authbridge/cmd/abctl/tui/app.go @@ -522,6 +522,9 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, m.initSessionView() case edit.FetchedMsg: + if m.editState.phase != editPhaseFetching { + return m, nil + } if msg.Err != nil { m.editState.phase = editPhaseError m.editState.err = msg.Err.Error() @@ -533,6 +536,9 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, openEditorCmd(msg.TempPath) case editorExitedMsg: + if m.editState.phase != editPhaseEditing || m.editState.fetched == nil { + return m, nil + } if msg.err != nil { m.editState.phase = editPhaseError m.editState.err = "editor exited: " + msg.err.Error() @@ -575,6 +581,10 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil case edit.AppliedMsg: + // Drop late AppliedMsg deliveries (user aborted before this returned). + if m.editState.phase != editPhaseApplying { + return m, nil + } if msg.Err != nil { m.editState.phase = editPhaseError m.editState.err = "apply failed: " + msg.Err.Error() @@ -585,6 +595,12 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, edit.PollCmd(m.ctx, m.statusURL, msg.ApplyTime) case edit.PolledMsg: + // Drop late PolledMsg deliveries after the user aborted the edit + // (phase reset to Done) or the state machine moved on. Without + // this guard, m.editState.fetched is nil and we'd panic below. + if m.editState.phase != editPhaseWaiting || m.editState.fetched == nil { + return m, nil + } switch msg.Result.Status { case edit.PollSuccess: m.editState = editState{phase: editPhaseDone} @@ -612,6 +628,9 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil case edit.RolledBackMsg: + if m.editState.phase != editPhaseRollback { + return m, nil + } m.editState.phase = editPhaseError if msg.Err != nil { m.editState.err = "reload failed: " + msg.ReloadErr + diff --git a/authbridge/cmd/abctl/tui/edit_e2e_test.go b/authbridge/cmd/abctl/tui/edit_e2e_test.go index dff6b0768..9842d83c3 100644 --- a/authbridge/cmd/abctl/tui/edit_e2e_test.go +++ b/authbridge/cmd/abctl/tui/edit_e2e_test.go @@ -305,3 +305,25 @@ func TestEditFlow_RollbackOnReloadFailure(t *testing.T) { t.Fatalf("rollback manifest missing original content:\n%s", runner.applyManifest) } } + +// TestEditFlow_LatePolledMsgAfterAbort verifies that a PolledMsg +// arriving after the user has aborted the edit (editState reset to +// Done, fetched = nil) is dropped rather than panicking on a nil +// dereference of editState.fetched. +func TestEditFlow_LatePolledMsgAfterAbort(t *testing.T) { + m := newPickerModel(context.Background(), nil, nil) + m.pane = panePipeline + // editState.phase is editPhaseDone (zero value), fetched is nil. + + defer func() { + if r := recover(); r != nil { + t.Fatalf("late PolledMsg should be dropped, but panicked: %v", r) + } + }() + late := edit.PolledMsg{Result: edit.PollResult{Status: edit.PollFailure, LastError: "anything"}} + updated, _ := m.Update(late) + mm := updated.(*model) + if mm.editState.phase != editPhaseDone { + t.Fatalf("phase changed on late PolledMsg: %v", mm.editState.phase) + } +} From b27b1d519fef2d495aacbfedc21a21c0494e7ea8 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 08:46:04 -0400 Subject: [PATCH 19/24] feat(abctl): Background hot-reload watch on Esc; flash result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pressing Esc during the Waiting (or Rollback) phase used to drop the result silently — the user had no way to learn whether the hot-reload eventually succeeded or failed. Add editPhaseBackground: Esc during those phases now keeps editState populated but tells View() not to render the overlay. PolledMsg and RolledBackMsg handlers route Background results to setFlash instead of the error overlay. Auto-rollback still fires in the background on PollFailure / PollTimeout so the on-disk ConfigMap stays consistent with the running pipeline regardless of whether the user was watching. Esc during the still-cancellable phases (Fetching, Editing, Applying) keeps its existing semantics: full reset to Done. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/tui/app.go | 43 ++++++++++++++++--- authbridge/cmd/abctl/tui/edit_e2e_test.go | 52 +++++++++++++++++++++++ authbridge/cmd/abctl/tui/edit_overlay.go | 6 +++ authbridge/cmd/abctl/tui/keys.go | 13 +++++- 4 files changed, 105 insertions(+), 9 deletions(-) diff --git a/authbridge/cmd/abctl/tui/app.go b/authbridge/cmd/abctl/tui/app.go index 799421506..e292970a8 100644 --- a/authbridge/cmd/abctl/tui/app.go +++ b/authbridge/cmd/abctl/tui/app.go @@ -595,14 +595,19 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, edit.PollCmd(m.ctx, m.statusURL, msg.ApplyTime) case edit.PolledMsg: - // Drop late PolledMsg deliveries after the user aborted the edit - // (phase reset to Done) or the state machine moved on. Without - // this guard, m.editState.fetched is nil and we'd panic below. - if m.editState.phase != editPhaseWaiting || m.editState.fetched == nil { + // Drop late PolledMsg deliveries after the user fully aborted + // (phase reset to Done) or the state machine moved on. fetched + // can be nil in those cases; both Waiting (overlay) and + // Background (footer flash) are valid targets. + if (m.editState.phase != editPhaseWaiting && m.editState.phase != editPhaseBackground) || m.editState.fetched == nil { return m, nil } + bg := m.editState.phase == editPhaseBackground switch msg.Result.Status { case edit.PollSuccess: + if bg { + m.setFlash("hot-reload succeeded") + } m.editState = editState{phase: editPhaseDone} return m, m.loadPipelineCmd() case edit.PollFailure, edit.PollTimeout: @@ -617,18 +622,40 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.editState.fetched.InnerYAML, ) if mErr != nil { + if bg { + m.setFlash("hot-reload failed: " + reason + "; rollback build failed too") + m.editState = editState{phase: editPhaseDone} + return m, nil + } m.editState.phase = editPhaseError m.editState.err = "reload failed: " + reason + "\n(rollback build failed: " + mErr.Error() + ")" return m, nil } - m.editState.phase = editPhaseRollback + // Stay backgrounded if user already Esc'd. + if bg { + m.editState.phase = editPhaseBackground + } else { + m.editState.phase = editPhaseRollback + } return m, edit.RollbackCmd(m.ctx, m.editRunner, origManifest, reason) } return m, nil case edit.RolledBackMsg: - if m.editState.phase != editPhaseRollback { + if m.editState.phase != editPhaseRollback && m.editState.phase != editPhaseBackground { + return m, nil + } + bg := m.editState.phase == editPhaseBackground + if bg { + if msg.Err != nil { + m.setFlash("hot-reload failed: " + msg.ReloadErr + + "; rollback failed: " + msg.Err.Error()) + } else { + m.setFlash("hot-reload failed: " + msg.ReloadErr + + "; rolled back to previous ConfigMap") + } + m.editState = editState{phase: editPhaseDone} return m, nil } m.editState.phase = editPhaseError @@ -717,7 +744,9 @@ sortAndRebuild: // View composes the full screen. func (m *model) View() string { // Edit overlay takes over the screen while an edit is in flight. - if m.editState.phase != editPhaseDone { + // editPhaseBackground intentionally falls through — the user backed + // out and wants the normal UI back; flash messages handle reporting. + if m.editState.phase != editPhaseDone && m.editState.phase != editPhaseBackground { return renderEditOverlay(m.editState, m.width, m.height) } diff --git a/authbridge/cmd/abctl/tui/edit_e2e_test.go b/authbridge/cmd/abctl/tui/edit_e2e_test.go index 9842d83c3..17df3d735 100644 --- a/authbridge/cmd/abctl/tui/edit_e2e_test.go +++ b/authbridge/cmd/abctl/tui/edit_e2e_test.go @@ -327,3 +327,55 @@ func TestEditFlow_LatePolledMsgAfterAbort(t *testing.T) { t.Fatalf("phase changed on late PolledMsg: %v", mm.editState.phase) } } + +// TestEditFlow_BackgroundedSuccess verifies that pressing Esc during +// Waiting moves the watch to the background and a later PollSuccess +// flashes the result instead of just being dropped. +func TestEditFlow_BackgroundedSuccess(t *testing.T) { + runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} + m := newPickerModel(context.Background(), nil, nil) + m.editRunner = runner.run + m.selectedNamespace = "team1" + m.selectedPod = "email-agent" + m.pane = panePipeline + + // Drive through fetch → editor → diff → apply → waiting. + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) + mm := updated.(*model) + fetchedMsg := cmd().(edit.FetchedMsg) + defer os.Remove(fetchedMsg.TempPath) + _ = os.WriteFile(fetchedMsg.TempPath, + []byte("pipeline:\n inbound:\n - name: jwt-validation\n config:\n x: 1\n"), 0o600) + updated, _ = mm.Update(fetchedMsg) + mm = updated.(*model) + updated, _ = mm.Update(editorExitedMsg{err: nil}) + mm = updated.(*model) + updated, cmd = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'y'}}) + mm = updated.(*model) + updated, _ = mm.Update(cmd().(edit.AppliedMsg)) + mm = updated.(*model) + if mm.editState.phase != editPhaseWaiting { + t.Fatalf("setup: phase = %v, want editPhaseWaiting", mm.editState.phase) + } + + // Press Esc — should background, not reset. + updated, _ = mm.Update(tea.KeyMsg{Type: tea.KeyEsc}) + mm = updated.(*model) + if mm.editState.phase != editPhaseBackground { + t.Fatalf("Esc-during-waiting: phase = %v, want editPhaseBackground", mm.editState.phase) + } + if mm.editState.fetched == nil { + t.Fatal("fetched should remain populated for late PolledMsg handling") + } + + // Late PollSuccess arrives — should flash and reset to Done. + success := edit.PolledMsg{Result: edit.PollResult{Status: edit.PollSuccess}} + updated, _ = mm.Update(success) + mm = updated.(*model) + if mm.editState.phase != editPhaseDone { + t.Fatalf("after late success: phase = %v, want editPhaseDone", mm.editState.phase) + } + if !strings.Contains(mm.flash, "succeeded") { + t.Fatalf("expected success flash; got %q", mm.flash) + } +} diff --git a/authbridge/cmd/abctl/tui/edit_overlay.go b/authbridge/cmd/abctl/tui/edit_overlay.go index d0f5fd245..5739a7c6b 100644 --- a/authbridge/cmd/abctl/tui/edit_overlay.go +++ b/authbridge/cmd/abctl/tui/edit_overlay.go @@ -22,6 +22,12 @@ const ( editPhaseApplying editPhaseWaiting editPhaseRollback // re-applying the original ConfigMap after a failed reload + // editPhaseBackground means: user pressed Esc during Waiting/Rollback; + // the in-flight Cmd is still running and we want to flash its result + // in the footer rather than reopen the overlay. Overlay renders nothing + // in this phase. fetched/applyTime stay populated so the PolledMsg + // handler can still trigger rollback if the in-pod reload failed. + editPhaseBackground editPhaseError ) diff --git a/authbridge/cmd/abctl/tui/keys.go b/authbridge/cmd/abctl/tui/keys.go index 95e455c86..63b59102e 100644 --- a/authbridge/cmd/abctl/tui/keys.go +++ b/authbridge/cmd/abctl/tui/keys.go @@ -419,9 +419,18 @@ func (m *model) handleEditKey(msg tea.KeyMsg) tea.Cmd { } return nil } - // Other phases: only Esc cancels (best-effort). + // Other phases: Esc backgrounds (Waiting / Rollback) so the in-flight + // Cmd's eventual result lands as a footer flash, or cancels outright + // (Fetching / Editing / Applying — phases where the result is still + // safe to drop). if msg.String() == "esc" { - m.editState = editState{phase: editPhaseDone} + switch m.editState.phase { + case editPhaseWaiting, editPhaseRollback: + m.editState.phase = editPhaseBackground + m.setFlash("hot-reload watch moved to background; you'll be notified") + default: + m.editState = editState{phase: editPhaseDone} + } return nil } return nil From b60741004035781df784bb2376b0ed63cb1986d1 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 09:02:15 -0400 Subject: [PATCH 20/24] fix(abctl): Match framework's /reload/status JSON wire format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The poller's ReloadStatus struct decoded `last_success_unix` (int64), but authlib/reloader/status.go emits `last_success` (RFC3339 string). The mismatched key silently zeroed the field, so the success comparison `LastSuccess.After(applyTime)` was never true and the poll never returned PollSuccess. With reloads_failed already at its baseline (no NEW failures after apply), PollFailure also never fired, and the user saw an indefinite "Waiting for hot-reload…" until the 120s timeout, even when the framework had already swapped pipelines seconds earlier. Switch to LastSuccess time.Time matching the framework wire shape exactly. ReloadsFailed/OK switch from uint64 to int64 to match the framework's signed-int wire shape too. The baseline-failed sentinel (-1) replaces the boolean first-poll flag for clarity. Updates the three test fixtures that were emitting the old key. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/edit_test.go | 3 ++- authbridge/cmd/abctl/edit/status.go | 26 +++++++++++++---------- authbridge/cmd/abctl/edit/status_test.go | 6 +++--- authbridge/cmd/abctl/tui/edit_e2e_test.go | 3 ++- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/authbridge/cmd/abctl/edit/edit_test.go b/authbridge/cmd/abctl/edit/edit_test.go index a4ce1e735..173a9c996 100644 --- a/authbridge/cmd/abctl/edit/edit_test.go +++ b/authbridge/cmd/abctl/edit/edit_test.go @@ -59,7 +59,8 @@ func TestPollCmd_Success(t *testing.T) { applyTime := time.Now() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"last_success_unix":99999999999}`)) + ts := time.Now().Add(1 * time.Hour).Format(time.RFC3339Nano) + _, _ = w.Write([]byte(`{"last_success":"` + ts + `"}`)) })) defer srv.Close() cmd := PollCmd(context.Background(), srv.URL, applyTime) diff --git a/authbridge/cmd/abctl/edit/status.go b/authbridge/cmd/abctl/edit/status.go index b30121b91..25993c6da 100644 --- a/authbridge/cmd/abctl/edit/status.go +++ b/authbridge/cmd/abctl/edit/status.go @@ -8,12 +8,13 @@ import ( ) // ReloadStatus is the wire shape of the framework's /reload/status endpoint. -// Only the fields abctl uses are decoded. +// Only the fields abctl uses are decoded. Keys must match +// authlib/reloader/status.go exactly. type ReloadStatus struct { - LastSuccessUnix int64 `json:"last_success_unix"` - ReloadsOK uint64 `json:"reloads_ok"` - ReloadsFailed uint64 `json:"reloads_failed"` - LastError string `json:"last_error"` + LastSuccess time.Time `json:"last_success"` + ReloadsOK int64 `json:"reloads_ok"` + ReloadsFailed int64 `json:"reloads_failed"` + LastError string `json:"last_error"` } // PollResultStatus is a sum type for PollUntilReloaded outcomes. @@ -32,13 +33,18 @@ type PollResult struct { LastError string // populated when Status == PollFailure } +// baselineFailedSentinel marks the "baseline not yet captured" state for +// the reloads_failed counter. The first successful poll snaps it to the +// current value so we only react to NEW failures after our apply. +const baselineFailedSentinel int64 = -1 + // pollInterval is the cadence between /reload/status fetches. 1s balances // user-visible spinner progress with not hammering the cluster on slow // reloads. const pollInterval = 1 * time.Second // PollUntilReloaded watches statusURL/reload/status until either: -// - LastSuccessUnix > applyTime.Unix() → PollSuccess. +// - LastSuccess > applyTime → PollSuccess. // - ReloadsFailed exceeds the value at first successful poll → PollFailure // with LastError populated. // - ctx is done → PollTimeout. (Caller is expected to set a 120s timeout @@ -50,8 +56,7 @@ func PollUntilReloaded(ctx context.Context, statusURL string, applyTime time.Tim url := statusURL + "/reload/status" client := &http.Client{Timeout: 2 * time.Second} - var baselineFailed uint64 - first := true + baselineFailed := baselineFailedSentinel for { select { @@ -70,11 +75,10 @@ func PollUntilReloaded(ctx context.Context, statusURL string, applyTime time.Tim decodeErr := json.NewDecoder(resp.Body).Decode(&rs) resp.Body.Close() if decodeErr == nil { - if first { + if baselineFailed == baselineFailedSentinel { baselineFailed = rs.ReloadsFailed - first = false } - if rs.LastSuccessUnix > applyTime.Unix() { + if rs.LastSuccess.After(applyTime) { return PollResult{Status: PollSuccess} } if rs.ReloadsFailed > baselineFailed { diff --git a/authbridge/cmd/abctl/edit/status_test.go b/authbridge/cmd/abctl/edit/status_test.go index a4d99c62e..ab06dc6c0 100644 --- a/authbridge/cmd/abctl/edit/status_test.go +++ b/authbridge/cmd/abctl/edit/status_test.go @@ -33,9 +33,9 @@ func TestPollUntilReloaded_Success(t *testing.T) { srv := makeStatusServer(t, func() ReloadStatus { c := calls.Add(1) if c == 1 { - return ReloadStatus{LastSuccessUnix: applyTime.Add(-1 * time.Hour).Unix()} + return ReloadStatus{LastSuccess: applyTime.Add(-1 * time.Hour)} } - return ReloadStatus{LastSuccessUnix: time.Now().Unix()} + return ReloadStatus{LastSuccess: time.Now()} }) res := PollUntilReloaded(context.Background(), srv.URL, applyTime) if res.Status != PollSuccess { @@ -65,7 +65,7 @@ func TestPollUntilReloaded_Failure(t *testing.T) { func TestPollUntilReloaded_Timeout(t *testing.T) { applyTime := time.Now() srv := makeStatusServer(t, func() ReloadStatus { - return ReloadStatus{LastSuccessUnix: applyTime.Add(-1 * time.Hour).Unix()} + return ReloadStatus{LastSuccess: applyTime.Add(-1 * time.Hour)} }) ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) defer cancel() diff --git a/authbridge/cmd/abctl/tui/edit_e2e_test.go b/authbridge/cmd/abctl/tui/edit_e2e_test.go index 17df3d735..1f0474168 100644 --- a/authbridge/cmd/abctl/tui/edit_e2e_test.go +++ b/authbridge/cmd/abctl/tui/edit_e2e_test.go @@ -8,6 +8,7 @@ import ( "os" "strings" "testing" + "time" tea "github.com/charmbracelet/bubbletea" @@ -67,7 +68,7 @@ func TestEditFlow_HappyPath(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(map[string]any{ - "last_success_unix": int64(99999999999), + "last_success": time.Now().Add(1 * time.Hour).Format(time.RFC3339Nano), }) })) defer srv.Close() From e204f1f8091d47bbde5eda8c07e61cf15fd51111 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 09:23:49 -0400 Subject: [PATCH 21/24] fix(abctl): Address CodeRabbit review on the pipeline editor PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five reviewer findings worth addressing: 1. portforward.go: two consecutive freeLocalPort() calls could pick the same port between the listener-close and the next bind. Loop until distinct (max 8 attempts) before giving up. 2. edit.go: PollCmd documented a 120s timeout but never enforced it — the TUI passed m.ctx straight through. Wrap with WithTimeout internally so the contract is real and callers can't accidentally leave the edit flow stuck in waiting. 3. keys.go: `e` was reachable in --endpoint mode where editRunner / statusURL / selectedNamespace / selectedPod are all unset. Hitting it would crash later in the kubectl call. Gate the keypath and flash a hint instead. 4. keys.go: `r` after a fetch failure called openEditorCmd("") because tempPath was never set. Detect that case and re-issue FetchCmd instead. 5. edit_e2e_test.go: TestEditFlow_RollbackOnReloadFailure was ignoring three setup errors (FetchedMsg.Err, os.WriteFile, AppliedMsg.Err). Fail fast on those instead. Skipped two stale findings (the >= second-granularity comparison was made obsolete by the time.Time switch in b607410; two analysis-chain- only comments had no actionable text). Tests: all four packages green (-race -timeout 60s). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/cluster/portforward.go | 18 +++++++++++++++--- authbridge/cmd/abctl/edit/edit.go | 17 ++++++++++++----- authbridge/cmd/abctl/tui/edit_e2e_test.go | 14 +++++++++++++- authbridge/cmd/abctl/tui/keys.go | 15 +++++++++++++++ 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/authbridge/cmd/abctl/cluster/portforward.go b/authbridge/cmd/abctl/cluster/portforward.go index 0b24c682b..310857ccb 100644 --- a/authbridge/cmd/abctl/cluster/portforward.go +++ b/authbridge/cmd/abctl/cluster/portforward.go @@ -83,9 +83,21 @@ func (k *kubectlPortForwarder) Start(ctx context.Context, namespace, pod string) if err != nil { return nil, err } - statusPort, err := freeLocalPort() - if err != nil { - return nil, err + // freeLocalPort closes the listener it picks, leaving a tiny window + // where two consecutive calls can return the same port (TIME_WAIT + // cleared, kernel re-issues). Loop until we get a distinct one. + var statusPort int + for attempts := 0; attempts < 8; attempts++ { + statusPort, err = freeLocalPort() + if err != nil { + return nil, err + } + if statusPort != port { + break + } + } + if statusPort == port { + return nil, fmt.Errorf("port-forward: could not allocate two distinct local ports") } // We do NOT bind ctx to the subprocess — kubectl port-forward should // outlive the per-call context (which is just for the readiness diff --git a/authbridge/cmd/abctl/edit/edit.go b/authbridge/cmd/abctl/edit/edit.go index 36af5f4ee..a425fbaeb 100644 --- a/authbridge/cmd/abctl/edit/edit.go +++ b/authbridge/cmd/abctl/edit/edit.go @@ -89,13 +89,20 @@ type PolledMsg struct { Result PollResult } +// pollDeadline bounds how long PollCmd waits for an in-pod reload to +// reach a terminal state. Picked to outlast the worst-case kubelet +// ConfigMap sync (~60s) plus the framework's drain window (30s) plus +// jitter, while still surfacing a stuck reload in a reasonable time. +const pollDeadline = 120 * time.Second + // PollCmd returns a tea.Cmd that polls /reload/status until the framework -// reload completes (success or failure) or ctx expires. Emits PolledMsg. -// -// Caller should construct ctx with a 120s WithTimeout so the poll -// terminates if kubelet doesn't sync within a reasonable window. +// reload completes (success or failure) or pollDeadline elapses. Emits +// PolledMsg. The deadline is enforced internally; the caller's ctx is +// only used for parent-cancellation (e.g. process shutdown). func PollCmd(ctx context.Context, statusURL string, applyTime time.Time) tea.Cmd { return func() tea.Msg { - return PolledMsg{Result: PollUntilReloaded(ctx, statusURL, applyTime)} + c, cancel := context.WithTimeout(ctx, pollDeadline) + defer cancel() + return PolledMsg{Result: PollUntilReloaded(c, statusURL, applyTime)} } } diff --git a/authbridge/cmd/abctl/tui/edit_e2e_test.go b/authbridge/cmd/abctl/tui/edit_e2e_test.go index 1f0474168..51a3e9115 100644 --- a/authbridge/cmd/abctl/tui/edit_e2e_test.go +++ b/authbridge/cmd/abctl/tui/edit_e2e_test.go @@ -168,6 +168,7 @@ func TestEditFlow_NCancelsAtDiff(t *testing.T) { runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} m := newPickerModel(context.Background(), nil, nil) m.editRunner = runner.run + m.statusURL = "http://stub" m.selectedNamespace = "team1" m.selectedPod = "email-agent" m.pane = panePipeline @@ -210,6 +211,7 @@ func TestEditFlow_NormalizesTrailingNewline(t *testing.T) { runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} m := newPickerModel(context.Background(), nil, nil) m.editRunner = runner.run + m.statusURL = "http://stub" m.selectedNamespace = "team1" m.selectedPod = "email-agent" m.pane = panePipeline @@ -248,6 +250,7 @@ func TestEditFlow_RollbackOnReloadFailure(t *testing.T) { runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} m := newPickerModel(context.Background(), nil, nil) m.editRunner = runner.run + m.statusURL = "http://stub" m.selectedNamespace = "team1" m.selectedPod = "email-agent" m.pane = panePipeline @@ -256,10 +259,15 @@ func TestEditFlow_RollbackOnReloadFailure(t *testing.T) { updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) mm := updated.(*model) fetchedMsg := cmd().(edit.FetchedMsg) + if fetchedMsg.Err != nil { + t.Fatalf("Fetch failed: %v", fetchedMsg.Err) + } defer os.Remove(fetchedMsg.TempPath) editedSubtree := []byte("pipeline:\n inbound:\n - name: bogus\n") - _ = os.WriteFile(fetchedMsg.TempPath, editedSubtree, 0o600) + if err := os.WriteFile(fetchedMsg.TempPath, editedSubtree, 0o600); err != nil { + t.Fatal(err) + } updated, _ = mm.Update(fetchedMsg) mm = updated.(*model) @@ -268,6 +276,9 @@ func TestEditFlow_RollbackOnReloadFailure(t *testing.T) { updated, cmd = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'y'}}) mm = updated.(*model) appliedMsg := cmd().(edit.AppliedMsg) + if appliedMsg.Err != nil { + t.Fatalf("apply failed: %v", appliedMsg.Err) + } updated, _ = mm.Update(appliedMsg) mm = updated.(*model) @@ -336,6 +347,7 @@ func TestEditFlow_BackgroundedSuccess(t *testing.T) { runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} m := newPickerModel(context.Background(), nil, nil) m.editRunner = runner.run + m.statusURL = "http://stub" m.selectedNamespace = "team1" m.selectedPod = "email-agent" m.pane = panePipeline diff --git a/authbridge/cmd/abctl/tui/keys.go b/authbridge/cmd/abctl/tui/keys.go index 63b59102e..16c0a52c7 100644 --- a/authbridge/cmd/abctl/tui/keys.go +++ b/authbridge/cmd/abctl/tui/keys.go @@ -209,6 +209,14 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { if m.pane != panePipeline { return nil } + // `e` requires the picker-mode cluster fields. In --endpoint + // mode none of these are set, so the keypath would crash later + // trying to kubectl-fetch with an empty pod/namespace. Surface + // the limitation in the footer instead of opening a broken edit. + if m.editRunner == nil || m.statusURL == "" || m.selectedNamespace == "" || m.selectedPod == "" { + m.setFlash("pipeline editing requires the picker (no --endpoint)") + return nil + } if m.editState.phase != editPhaseDone { return nil // already editing } @@ -411,6 +419,13 @@ func (m *model) handleEditKey(msg tea.KeyMsg) tea.Cmd { case editPhaseError: switch msg.String() { case "r": + // If the fetch never completed (tempPath empty), retry the + // fetch instead of opening $EDITOR on "" (which leaves the + // user with nothing to edit and a misleading flow). + if m.editState.tempPath == "" { + m.editState = editState{phase: editPhaseFetching} + return edit.FetchCmd(m.ctx, m.editRunner, m.selectedNamespace, m.selectedPod) + } m.editState.phase = editPhaseEditing return openEditorCmd(m.editState.tempPath) case "esc": From e02651fa3b722b4c0c46913d0f2fc8fb74b50c5c Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 09:37:43 -0400 Subject: [PATCH 22/24] fix(abctl): Address subagent review on the pipeline editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five reviewer findings worth addressing: 1. Late-message guards used phase as the discriminator. That's fine for abandoned-and-stayed-aborted edits, but breaks the abandoned-then-restarted-quickly case: Edit 1's stale PolledMsg could land on Edit 2's same-phase overlay and trigger an unwanted rollback. Add an editState.generation counter, bump it on every fresh edit start, and tag every async tea.Cmd with the gen at issue time via withGen() — handlers drop messages whose gen doesn't match. genFetchedMsg / genAppliedMsg / genPolledMsg / genRolledBackMsg wrap the upstream edit.* messages; editorExitedMsg gets a gen field too. New regression test TestEditFlow_StaleMsgFromAbandonedEditDropped covers the case. 2. Document the rollback's third-party-edit caveat: with --force-conflicts=true on Apply, a rollback built from editState.fetched.ConfigMapYAML silently overwrites concurrent operator/kubectl/kustomize edits. Comment now flags this; the running pipeline is unaffected (framework keeps the previous in-memory pipeline on build failure), but the on-disk CM is not a true undo. 3. PollUntilReloaded had no backoff and no unreachable surfacing. After 5 consecutive transport errors, return PollFailure with a "reload status endpoint unreachable" message instead of waiting the full 120s deadline. Add capped exponential backoff (1s → 5s) so a flapping endpoint isn't hammered. 4. Pull "config.yaml" into a configMapDataKey constant so a future operator key rename flips one line. The "no data.config.yaml key" error now uses the constant via fmt.Errorf %q. 5. Sweep edit-tempfiles older than 24h on abctl launch (edit.SweepStaleTempfiles). Tempfiles are still left in place on every exit path for crash recovery; the sweep just bounds $TMPDIR over time. Tests: all four packages green (-race -timeout 30s), including the new abandon-restart regression test. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/configmap.go | 15 ++- authbridge/cmd/abctl/edit/edit.go | 32 ++++++ authbridge/cmd/abctl/edit/status.go | 48 +++++++-- authbridge/cmd/abctl/main.go | 7 ++ authbridge/cmd/abctl/tui/app.go | 108 ++++++++++++++++---- authbridge/cmd/abctl/tui/edit_e2e_test.go | 119 ++++++++++++++++++---- authbridge/cmd/abctl/tui/edit_overlay.go | 8 ++ authbridge/cmd/abctl/tui/keys.go | 18 ++-- 8 files changed, 294 insertions(+), 61 deletions(-) diff --git a/authbridge/cmd/abctl/edit/configmap.go b/authbridge/cmd/abctl/edit/configmap.go index 74c4eab3e..87af5b36e 100644 --- a/authbridge/cmd/abctl/edit/configmap.go +++ b/authbridge/cmd/abctl/edit/configmap.go @@ -20,6 +20,13 @@ import ( "gopkg.in/yaml.v3" ) +// configMapDataKey is the key inside the ConfigMap's data: mapping that +// holds the runtime YAML. The kagenti-operator and the editor both +// agree on this name; if a future operator change renames it, all four +// callers (Fetch, BuildManifest, the two extractInnerYAML branches) +// flip together by changing this constant. +const configMapDataKey = "config.yaml" + // FindPipelineRange returns the byte offsets [start, end) in innerYAML // that span the "pipeline:" subtree, including the "pipeline:" key line // itself but not any following top-level keys. Used by the editor to @@ -134,13 +141,13 @@ func BuildManifest(origCMYAML, newInner []byte) ([]byte, error) { } var configValueNode *yaml.Node for i := 0; i < len(dataNode.Content); i += 2 { - if dataNode.Content[i].Value == "config.yaml" { + if dataNode.Content[i].Value == configMapDataKey { configValueNode = dataNode.Content[i+1] break } } if configValueNode == nil { - return nil, fmt.Errorf("ConfigMap data has no config.yaml key") + return nil, fmt.Errorf("ConfigMap data has no %q key", configMapDataKey) } // Set the value to a literal-block scalar carrying newInner. @@ -273,13 +280,13 @@ func extractInnerYAML(cmYAML []byte) ([]byte, error) { if doc.Content[i].Value == "data" { dataNode := doc.Content[i+1] for j := 0; j+1 < len(dataNode.Content); j += 2 { - if dataNode.Content[j].Value == "config.yaml" { + if dataNode.Content[j].Value == configMapDataKey { return []byte(dataNode.Content[j+1].Value), nil } } } } - return nil, fmt.Errorf("ConfigMap data has no config.yaml key") + return nil, fmt.Errorf("ConfigMap data has no %q key", configMapDataKey) } // Apply writes manifest to a tempfile and runs kubectl apply --server-side diff --git a/authbridge/cmd/abctl/edit/edit.go b/authbridge/cmd/abctl/edit/edit.go index a425fbaeb..794d6b536 100644 --- a/authbridge/cmd/abctl/edit/edit.go +++ b/authbridge/cmd/abctl/edit/edit.go @@ -3,11 +3,43 @@ package edit import ( "context" "os" + "path/filepath" "time" tea "github.com/charmbracelet/bubbletea" ) +// tempFileMaxAge is how long a stale edit tempfile is allowed to sit +// in $TMPDIR before SweepStaleTempfiles deletes it. 24h covers crash +// recovery (a user can re-open last day's edit) without letting the +// directory grow without bound. +const tempFileMaxAge = 24 * time.Hour + +// SweepStaleTempfiles deletes abctl-pipeline-*.yaml tempfiles older +// than tempFileMaxAge from os.TempDir(). Errors are non-fatal — a +// best-effort cleanup at startup; the editor still works without it. +// Returns the number of files removed (for diagnostics). +func SweepStaleTempfiles() int { + matches, err := filepath.Glob(filepath.Join(os.TempDir(), "abctl-pipeline-*.yaml")) + if err != nil { + return 0 + } + cutoff := time.Now().Add(-tempFileMaxAge) + n := 0 + for _, p := range matches { + fi, err := os.Stat(p) + if err != nil { + continue + } + if fi.ModTime().Before(cutoff) { + if err := os.Remove(p); err == nil { + n++ + } + } + } + return n +} + // FetchedMsg is the result of FetchCmd. On success: Fetched and TempPath // are both set, Err is nil. On failure: Err is populated, others are zero. type FetchedMsg struct { diff --git a/authbridge/cmd/abctl/edit/status.go b/authbridge/cmd/abctl/edit/status.go index 25993c6da..98a3ce44a 100644 --- a/authbridge/cmd/abctl/edit/status.go +++ b/authbridge/cmd/abctl/edit/status.go @@ -43,20 +43,38 @@ const baselineFailedSentinel int64 = -1 // reloads. const pollInterval = 1 * time.Second +// pollMaxBackoff caps the exponential backoff applied to consecutive +// transport errors. Keeps a flapping endpoint from getting hammered +// without making the user wait too long once it recovers. +const pollMaxBackoff = 5 * time.Second + +// unreachableThreshold is the number of consecutive transport-layer +// failures (network errors, non-200) after which the poller gives up +// rather than waiting for the full deadline. Picked so a normal kubelet +// hiccup (one or two failed polls) is tolerated, but a port-forward +// drop or framework crash surfaces fast. +const unreachableThreshold = 5 + // PollUntilReloaded watches statusURL/reload/status until either: // - LastSuccess > applyTime → PollSuccess. // - ReloadsFailed exceeds the value at first successful poll → PollFailure // with LastError populated. // - ctx is done → PollTimeout. (Caller is expected to set a 120s timeout // via context.WithTimeout.) +// - 5 consecutive transport errors → PollFailure with a synthesized +// "reload status unreachable" LastError, so a port-forward drop or +// crashed framework surfaces fast instead of silently sitting through +// the whole deadline. // -// HTTP errors (network, non-200) are retried until ctx expires; we treat -// them as "framework not yet reachable, keep waiting." +// Backoff: poll at pollInterval, doubled on each transport error up to +// pollMaxBackoff. Reset to pollInterval on the next successful response. func PollUntilReloaded(ctx context.Context, statusURL string, applyTime time.Time) PollResult { url := statusURL + "/reload/status" client := &http.Client{Timeout: 2 * time.Second} baselineFailed := baselineFailedSentinel + consecErrors := 0 + wait := pollInterval for { select { @@ -70,11 +88,14 @@ func PollUntilReloaded(ctx context.Context, statusURL string, applyTime time.Tim return PollResult{Status: PollTimeout} } resp, err := client.Do(req) - if err == nil && resp.StatusCode == 200 { + ok := err == nil && resp.StatusCode == 200 + if ok { var rs ReloadStatus decodeErr := json.NewDecoder(resp.Body).Decode(&rs) resp.Body.Close() if decodeErr == nil { + consecErrors = 0 + wait = pollInterval if baselineFailed == baselineFailedSentinel { baselineFailed = rs.ReloadsFailed } @@ -85,14 +106,29 @@ func PollUntilReloaded(ctx context.Context, statusURL string, applyTime time.Tim return PollResult{Status: PollFailure, LastError: rs.LastError} } } - } else if resp != nil { - resp.Body.Close() + } else { + if resp != nil { + resp.Body.Close() + } + consecErrors++ + if consecErrors >= unreachableThreshold { + return PollResult{ + Status: PollFailure, + LastError: "reload status endpoint unreachable (port-forward dropped or framework down?)", + } + } + if wait < pollMaxBackoff { + wait *= 2 + if wait > pollMaxBackoff { + wait = pollMaxBackoff + } + } } select { case <-ctx.Done(): return PollResult{Status: PollTimeout} - case <-time.After(pollInterval): + case <-time.After(wait): } } } diff --git a/authbridge/cmd/abctl/main.go b/authbridge/cmd/abctl/main.go index 4b5ab7356..101202088 100644 --- a/authbridge/cmd/abctl/main.go +++ b/authbridge/cmd/abctl/main.go @@ -16,6 +16,7 @@ import ( "syscall" "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/cluster" + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/tui" ) @@ -24,6 +25,12 @@ func main() { "AuthBridge session API URL (e.g. http://localhost:9094). When omitted, abctl opens a Namespaces → Pods picker.") flag.Parse() + // Best-effort sweep of edit-tempfiles older than 24h. Tempfiles are + // intentionally left in place on every exit path (success / abort / + // crash) so a user can recover an in-progress edit; the sweep keeps + // $TMPDIR bounded for users who edit often. + _ = edit.SweepStaleTempfiles() + // Friendly check: if picker mode and no kubectl, fail fast with a // clear message instead of a stack trace later. if *endpoint == "" { diff --git a/authbridge/cmd/abctl/tui/app.go b/authbridge/cmd/abctl/tui/app.go index e292970a8..aab775fdf 100644 --- a/authbridge/cmd/abctl/tui/app.go +++ b/authbridge/cmd/abctl/tui/app.go @@ -103,8 +103,56 @@ type portForwardReadyMsg struct { } // editorExitedMsg is sent by openEditorCmd when the user's $EDITOR -// process exits. err is nil on a clean (0) exit. -type editorExitedMsg struct{ err error } +// process exits. err is nil on a clean (0) exit. Carries gen so a +// stale editor result from Edit 1 can't overwrite Edit 2's tempfile +// processing. +type editorExitedMsg struct { + gen int + err error +} + +// genFetchedMsg / genAppliedMsg / genPolledMsg / genRolledBackMsg wrap +// the upstream edit-package messages with the editState.generation +// captured at Cmd-issue time. Handlers drop messages whose gen doesn't +// match m.editState.generation, which prevents Edit 1's late results +// from leaking onto Edit 2's overlay (and would otherwise be able to +// trigger an unintended rollback). +type genFetchedMsg struct { + gen int + edit.FetchedMsg +} +type genAppliedMsg struct { + gen int + edit.AppliedMsg +} +type genPolledMsg struct { + gen int + edit.PolledMsg +} +type genRolledBackMsg struct { + gen int + edit.RolledBackMsg +} + +// withGen wraps a tea.Cmd to tag its result with the supplied gen. +// Inspects the upstream msg type and rewrites it into the matching +// generational wrapper. Unknown types pass through untouched. +func withGen(gen int, c tea.Cmd) tea.Cmd { + return func() tea.Msg { + switch m := c().(type) { + case edit.FetchedMsg: + return genFetchedMsg{gen: gen, FetchedMsg: m} + case edit.AppliedMsg: + return genAppliedMsg{gen: gen, AppliedMsg: m} + case edit.PolledMsg: + return genPolledMsg{gen: gen, PolledMsg: m} + case edit.RolledBackMsg: + return genRolledBackMsg{gen: gen, RolledBackMsg: m} + default: + return m + } + } +} // Model is the top-level Bubble Tea model. type model struct { @@ -521,8 +569,8 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.pane = paneSessions return m, m.initSessionView() - case edit.FetchedMsg: - if m.editState.phase != editPhaseFetching { + case genFetchedMsg: + if msg.gen != m.editState.generation || m.editState.phase != editPhaseFetching { return m, nil } if msg.Err != nil { @@ -533,10 +581,10 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.editState.fetched = msg.Fetched m.editState.tempPath = msg.TempPath m.editState.phase = editPhaseEditing - return m, openEditorCmd(msg.TempPath) + return m, openEditorCmd(m.editState.generation, msg.TempPath) case editorExitedMsg: - if m.editState.phase != editPhaseEditing || m.editState.fetched == nil { + if msg.gen != m.editState.generation || m.editState.phase != editPhaseEditing || m.editState.fetched == nil { return m, nil } if msg.err != nil { @@ -580,9 +628,9 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.editState.phase = editPhaseDiff return m, nil - case edit.AppliedMsg: - // Drop late AppliedMsg deliveries (user aborted before this returned). - if m.editState.phase != editPhaseApplying { + case genAppliedMsg: + // Drop stale or aborted-edit AppliedMsg deliveries. + if msg.gen != m.editState.generation || m.editState.phase != editPhaseApplying { return m, nil } if msg.Err != nil { @@ -592,14 +640,16 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.editState.applyTime = msg.ApplyTime m.editState.phase = editPhaseWaiting - return m, edit.PollCmd(m.ctx, m.statusURL, msg.ApplyTime) - - case edit.PolledMsg: - // Drop late PolledMsg deliveries after the user fully aborted - // (phase reset to Done) or the state machine moved on. fetched - // can be nil in those cases; both Waiting (overlay) and - // Background (footer flash) are valid targets. - if (m.editState.phase != editPhaseWaiting && m.editState.phase != editPhaseBackground) || m.editState.fetched == nil { + return m, withGen(m.editState.generation, edit.PollCmd(m.ctx, m.statusURL, msg.ApplyTime)) + + case genPolledMsg: + // Drop stale (different gen), fully-aborted (phase=Done), or + // out-of-state-machine deliveries. fetched can be nil in those + // cases; both Waiting (overlay) and Background (footer flash) + // are valid targets for an in-flight watch. + if msg.gen != m.editState.generation || + (m.editState.phase != editPhaseWaiting && m.editState.phase != editPhaseBackground) || + m.editState.fetched == nil { return m, nil } bg := m.editState.phase == editPhaseBackground @@ -613,6 +663,17 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case edit.PollFailure, edit.PollTimeout: // In-pod reload didn't take. The running pipeline is still // the previous one; reconcile the ConfigMap back to match. + // + // Caveat: the rollback bytes come from m.editState.fetched — + // captured at this edit's Fetch time. With Apply's + // --force-conflicts=true and field-manager=abctl, if a third + // party (operator reconcile, kubectl edit, kustomize) modified + // the ConfigMap between our forward apply and this rollback, + // our rollback silently reverts their change too. This is not + // a true undo. The framework's running pipeline is unaffected + // (build failure → keeps the previous in-memory pipeline), but + // the on-disk ConfigMap can lose third-party state. Surfacing + // a "third-party change detected" path is a future option. reason := msg.Result.LastError if msg.Result.Status == edit.PollTimeout { reason = "reload not observed in 120s" @@ -638,12 +699,13 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } else { m.editState.phase = editPhaseRollback } - return m, edit.RollbackCmd(m.ctx, m.editRunner, origManifest, reason) + return m, withGen(m.editState.generation, edit.RollbackCmd(m.ctx, m.editRunner, origManifest, reason)) } return m, nil - case edit.RolledBackMsg: - if m.editState.phase != editPhaseRollback && m.editState.phase != editPhaseBackground { + case genRolledBackMsg: + if msg.gen != m.editState.generation || + (m.editState.phase != editPhaseRollback && m.editState.phase != editPhaseBackground) { return m, nil } bg := m.editState.phase == editPhaseBackground @@ -933,13 +995,15 @@ func Run(ctx context.Context, opts RunOptions) error { // openEditorCmd returns a tea.Cmd that suspends bubbletea, runs $EDITOR // (vi if unset) on path, and emits editorExitedMsg when the editor exits. -func openEditorCmd(path string) tea.Cmd { +// gen is captured at call time so the handler can detect a stale exit +// from an aborted-then-restarted edit. +func openEditorCmd(gen int, path string) tea.Cmd { editor := os.Getenv("EDITOR") if editor == "" { editor = "vi" } c := exec.Command("sh", "-c", editor+" "+path) return tea.ExecProcess(c, func(err error) tea.Msg { - return editorExitedMsg{err: err} + return editorExitedMsg{gen: gen, err: err} }) } diff --git a/authbridge/cmd/abctl/tui/edit_e2e_test.go b/authbridge/cmd/abctl/tui/edit_e2e_test.go index 51a3e9115..13b5e4f1f 100644 --- a/authbridge/cmd/abctl/tui/edit_e2e_test.go +++ b/authbridge/cmd/abctl/tui/edit_e2e_test.go @@ -91,7 +91,7 @@ func TestEditFlow_HappyPath(t *testing.T) { } // Run FetchCmd. - fetchedMsg := cmd().(edit.FetchedMsg) + fetchedMsg := cmd().(genFetchedMsg) if fetchedMsg.Err != nil { t.Fatalf("Fetch failed: %v", fetchedMsg.Err) } @@ -110,7 +110,7 @@ func TestEditFlow_HappyPath(t *testing.T) { } // Inject editorExitedMsg directly (skips the real ExecProcess). - updated, _ = mm.Update(editorExitedMsg{err: nil}) + updated, _ = mm.Update(editorExitedMsg{gen: mm.editState.generation, err: nil}) mm = updated.(*model) if mm.editState.phase != editPhaseDiff { t.Fatalf("phase = %v, want editPhaseDiff (validate should pass)", mm.editState.phase) @@ -130,7 +130,7 @@ func TestEditFlow_HappyPath(t *testing.T) { } // Run ApplyCmd. - appliedMsg := cmd().(edit.AppliedMsg) + appliedMsg := cmd().(genAppliedMsg) if appliedMsg.Err != nil { t.Fatalf("apply failed: %v", appliedMsg.Err) } @@ -145,7 +145,7 @@ func TestEditFlow_HappyPath(t *testing.T) { } // Run PollCmd. - polledMsg := cmd().(edit.PolledMsg) + polledMsg := cmd().(genPolledMsg) if polledMsg.Result.Status != edit.PollSuccess { t.Fatalf("poll status = %v, want PollSuccess", polledMsg.Result.Status) } @@ -175,7 +175,7 @@ func TestEditFlow_NCancelsAtDiff(t *testing.T) { updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) mm := updated.(*model) - fetchedMsg := cmd().(edit.FetchedMsg) + fetchedMsg := cmd().(genFetchedMsg) defer os.Remove(fetchedMsg.TempPath) // Pretend the user edited. @@ -184,7 +184,7 @@ func TestEditFlow_NCancelsAtDiff(t *testing.T) { updated, _ = mm.Update(fetchedMsg) mm = updated.(*model) - updated, _ = mm.Update(editorExitedMsg{err: nil}) + updated, _ = mm.Update(editorExitedMsg{gen: mm.editState.generation, err: nil}) mm = updated.(*model) if mm.editState.phase != editPhaseDiff { t.Fatalf("setup: phase = %v, want editPhaseDiff", mm.editState.phase) @@ -218,7 +218,7 @@ func TestEditFlow_NormalizesTrailingNewline(t *testing.T) { updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) mm := updated.(*model) - fetchedMsg := cmd().(edit.FetchedMsg) + fetchedMsg := cmd().(genFetchedMsg) defer os.Remove(fetchedMsg.TempPath) // Write an edit deliberately missing a trailing newline. @@ -232,7 +232,7 @@ func TestEditFlow_NormalizesTrailingNewline(t *testing.T) { updated, _ = mm.Update(fetchedMsg) mm = updated.(*model) - updated, _ = mm.Update(editorExitedMsg{err: nil}) + updated, _ = mm.Update(editorExitedMsg{gen: mm.editState.generation, err: nil}) mm = updated.(*model) if mm.editState.phase != editPhaseDiff { t.Fatalf("phase = %v, want editPhaseDiff", mm.editState.phase) @@ -258,7 +258,7 @@ func TestEditFlow_RollbackOnReloadFailure(t *testing.T) { // Drive through fetch → editor → diff → apply. updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) mm := updated.(*model) - fetchedMsg := cmd().(edit.FetchedMsg) + fetchedMsg := cmd().(genFetchedMsg) if fetchedMsg.Err != nil { t.Fatalf("Fetch failed: %v", fetchedMsg.Err) } @@ -271,22 +271,22 @@ func TestEditFlow_RollbackOnReloadFailure(t *testing.T) { updated, _ = mm.Update(fetchedMsg) mm = updated.(*model) - updated, _ = mm.Update(editorExitedMsg{err: nil}) + updated, _ = mm.Update(editorExitedMsg{gen: mm.editState.generation, err: nil}) mm = updated.(*model) updated, cmd = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'y'}}) mm = updated.(*model) - appliedMsg := cmd().(edit.AppliedMsg) + appliedMsg := cmd().(genAppliedMsg) if appliedMsg.Err != nil { t.Fatalf("apply failed: %v", appliedMsg.Err) } updated, _ = mm.Update(appliedMsg) mm = updated.(*model) - // Inject a PollFailure manually. - failure := edit.PolledMsg{Result: edit.PollResult{ - Status: edit.PollFailure, - LastError: "unknown plugin: bogus", - }} + // Inject a PollFailure manually with the current generation. + failure := genPolledMsg{ + gen: mm.editState.generation, + PolledMsg: edit.PolledMsg{Result: edit.PollResult{Status: edit.PollFailure, LastError: "unknown plugin: bogus"}}, + } updated, cmd = mm.Update(failure) mm = updated.(*model) if mm.editState.phase != editPhaseRollback { @@ -296,7 +296,7 @@ func TestEditFlow_RollbackOnReloadFailure(t *testing.T) { t.Fatal("expected RollbackCmd") } - rolledBack := cmd().(edit.RolledBackMsg) + rolledBack := cmd().(genRolledBackMsg) if rolledBack.Err != nil { t.Fatalf("rollback Apply error: %v", rolledBack.Err) } @@ -332,7 +332,12 @@ func TestEditFlow_LatePolledMsgAfterAbort(t *testing.T) { t.Fatalf("late PolledMsg should be dropped, but panicked: %v", r) } }() - late := edit.PolledMsg{Result: edit.PollResult{Status: edit.PollFailure, LastError: "anything"}} + // Late message with a stale gen should also be dropped — the gen + // guard is the more robust discriminator. + late := genPolledMsg{ + gen: 42, + PolledMsg: edit.PolledMsg{Result: edit.PollResult{Status: edit.PollFailure, LastError: "anything"}}, + } updated, _ := m.Update(late) mm := updated.(*model) if mm.editState.phase != editPhaseDone { @@ -355,17 +360,17 @@ func TestEditFlow_BackgroundedSuccess(t *testing.T) { // Drive through fetch → editor → diff → apply → waiting. updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) mm := updated.(*model) - fetchedMsg := cmd().(edit.FetchedMsg) + fetchedMsg := cmd().(genFetchedMsg) defer os.Remove(fetchedMsg.TempPath) _ = os.WriteFile(fetchedMsg.TempPath, []byte("pipeline:\n inbound:\n - name: jwt-validation\n config:\n x: 1\n"), 0o600) updated, _ = mm.Update(fetchedMsg) mm = updated.(*model) - updated, _ = mm.Update(editorExitedMsg{err: nil}) + updated, _ = mm.Update(editorExitedMsg{gen: mm.editState.generation, err: nil}) mm = updated.(*model) updated, cmd = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'y'}}) mm = updated.(*model) - updated, _ = mm.Update(cmd().(edit.AppliedMsg)) + updated, _ = mm.Update(cmd().(genAppliedMsg)) mm = updated.(*model) if mm.editState.phase != editPhaseWaiting { t.Fatalf("setup: phase = %v, want editPhaseWaiting", mm.editState.phase) @@ -382,7 +387,10 @@ func TestEditFlow_BackgroundedSuccess(t *testing.T) { } // Late PollSuccess arrives — should flash and reset to Done. - success := edit.PolledMsg{Result: edit.PollResult{Status: edit.PollSuccess}} + success := genPolledMsg{ + gen: mm.editState.generation, + PolledMsg: edit.PolledMsg{Result: edit.PollResult{Status: edit.PollSuccess}}, + } updated, _ = mm.Update(success) mm = updated.(*model) if mm.editState.phase != editPhaseDone { @@ -392,3 +400,70 @@ func TestEditFlow_BackgroundedSuccess(t *testing.T) { t.Fatalf("expected success flash; got %q", mm.flash) } } + +// TestEditFlow_StaleMsgFromAbandonedEditDropped verifies that a +// PolledMsg from Edit 1 (whose user Esc'd then started Edit 2 quickly) +// is dropped and does NOT route Edit 1's reload result onto Edit 2. +func TestEditFlow_StaleMsgFromAbandonedEditDropped(t *testing.T) { + runner := &editFakeRunner{getResponse: []byte(editFixtureCMYAML)} + m := newPickerModel(context.Background(), nil, nil) + m.editRunner = runner.run + m.statusURL = "http://stub" + m.selectedNamespace = "team1" + m.selectedPod = "email-agent" + m.pane = panePipeline + + // Edit 1: drive to Waiting, then abort. + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) + mm := updated.(*model) + fetchedMsg1 := cmd().(genFetchedMsg) + defer os.Remove(fetchedMsg1.TempPath) + gen1 := mm.editState.generation + + _ = os.WriteFile(fetchedMsg1.TempPath, + []byte("pipeline:\n inbound:\n - name: jwt-validation\n config:\n x: 1\n"), 0o600) + updated, _ = mm.Update(fetchedMsg1) + mm = updated.(*model) + updated, _ = mm.Update(editorExitedMsg{gen: gen1, err: nil}) + mm = updated.(*model) + updated, cmd = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'y'}}) + mm = updated.(*model) + updated, _ = mm.Update(cmd().(genAppliedMsg)) + mm = updated.(*model) + + // User aborts Edit 1. + updated, _ = mm.Update(tea.KeyMsg{Type: tea.KeyEsc}) + mm = updated.(*model) + // (phase is now editPhaseBackground; Esc on Waiting backgrounds. + // Move it fully out so we can start Edit 2.) + mm.editState = editState{phase: editPhaseDone, generation: gen1} + + // Edit 2 starts. + updated, cmd = mm.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'e'}}) + mm = updated.(*model) + if mm.editState.generation == gen1 { + t.Fatal("Edit 2's generation should differ from Edit 1's") + } + gen2 := mm.editState.generation + + // Edit 1's stale PollFailure arrives now. + stale := genPolledMsg{ + gen: gen1, + PolledMsg: edit.PolledMsg{Result: edit.PollResult{Status: edit.PollFailure, LastError: "edit 1 reload"}}, + } + updated, returnedCmd := mm.Update(stale) + mm = updated.(*model) + // Edit 2 must remain in Fetching (its only allowed phase post-`e`), + // not jump to Rollback or Error from Edit 1's stale msg. + if mm.editState.phase != editPhaseFetching { + t.Fatalf("phase = %v, want editPhaseFetching (stale msg should be ignored)", mm.editState.phase) + } + if mm.editState.generation != gen2 { + t.Fatalf("Edit 2's gen mutated: %d vs %d", mm.editState.generation, gen2) + } + if returnedCmd != nil { + t.Fatal("stale PolledMsg should not produce a Cmd") + } + // Quiet "cmd is unused after Edit 2 dispatch" by referencing it. + _ = cmd +} diff --git a/authbridge/cmd/abctl/tui/edit_overlay.go b/authbridge/cmd/abctl/tui/edit_overlay.go index 5739a7c6b..d0113f817 100644 --- a/authbridge/cmd/abctl/tui/edit_overlay.go +++ b/authbridge/cmd/abctl/tui/edit_overlay.go @@ -40,6 +40,14 @@ type editState struct { diff string // colorized output from edit.Diff err string // single-line message in editPhaseError applyTime time.Time + // generation is bumped each time a fresh edit cycle begins (initial + // `e`, retry from error, restart after abort). Each tea.Cmd captures + // the value at issue time; handlers drop messages whose captured + // generation doesn't match the current one. Without this, a late + // PolledMsg from Edit 1 arriving after the user has Esc'd and + // started Edit 2 would route Edit 1's reload result onto Edit 2's + // overlay (same phase, different transaction). + generation int } // renderEditOverlay returns the overlay content (rendered into a diff --git a/authbridge/cmd/abctl/tui/keys.go b/authbridge/cmd/abctl/tui/keys.go index 16c0a52c7..05dfaf95a 100644 --- a/authbridge/cmd/abctl/tui/keys.go +++ b/authbridge/cmd/abctl/tui/keys.go @@ -220,8 +220,9 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { if m.editState.phase != editPhaseDone { return nil // already editing } - m.editState = editState{phase: editPhaseFetching} - return edit.FetchCmd(m.ctx, m.editRunner, m.selectedNamespace, m.selectedPod) + gen := m.editState.generation + 1 + m.editState = editState{phase: editPhaseFetching, generation: gen} + return withGen(gen, edit.FetchCmd(m.ctx, m.editRunner, m.selectedNamespace, m.selectedPod)) case "g": m.goTop() @@ -410,7 +411,7 @@ func (m *model) handleEditKey(msg tea.KeyMsg) tea.Cmd { m.editState.err = "build manifest: " + err.Error() return nil } - return edit.ApplyCmd(m.ctx, m.editRunner, manifest) + return withGen(m.editState.generation, edit.ApplyCmd(m.ctx, m.editRunner, manifest)) case "n", "N", "esc": m.editState = editState{phase: editPhaseDone} return nil @@ -421,13 +422,16 @@ func (m *model) handleEditKey(msg tea.KeyMsg) tea.Cmd { case "r": // If the fetch never completed (tempPath empty), retry the // fetch instead of opening $EDITOR on "" (which leaves the - // user with nothing to edit and a misleading flow). + // user with nothing to edit and a misleading flow). A retry + // bumps gen so any straggling messages from the failed + // attempt are dropped. if m.editState.tempPath == "" { - m.editState = editState{phase: editPhaseFetching} - return edit.FetchCmd(m.ctx, m.editRunner, m.selectedNamespace, m.selectedPod) + gen := m.editState.generation + 1 + m.editState = editState{phase: editPhaseFetching, generation: gen} + return withGen(gen, edit.FetchCmd(m.ctx, m.editRunner, m.selectedNamespace, m.selectedPod)) } m.editState.phase = editPhaseEditing - return openEditorCmd(m.editState.tempPath) + return openEditorCmd(m.editState.generation, m.editState.tempPath) case "esc": m.editState = editState{phase: editPhaseDone} return nil From 7656ef10d4098540f5e0cf2775123c1aaecbd9a7 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 10:31:06 -0400 Subject: [PATCH 23/24] fix(abctl): Address pdettori review on configmap.go Two reviewer findings: 1. Splice trusted that start/end were within [0, len(innerYAML)] and start <= end. If a future caller ever passes stale offsets, the slice expressions would panic. Add a one-line guard at the top that returns innerYAML unchanged on out-of-range inputs. New subtest TestSplice_OutOfBoundsReturnsInputUnchanged covers all three boundary cases. 2. The metadata-strip loop reuses md.Content's backing array via md.Content[:0]. That's safe (we only ever shrink) and avoids an alloc, but a reader might wonder. Add a one-line comment naming the choice. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/edit/configmap.go | 10 ++++++++++ authbridge/cmd/abctl/edit/configmap_test.go | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/authbridge/cmd/abctl/edit/configmap.go b/authbridge/cmd/abctl/edit/configmap.go index 87af5b36e..d170b6a13 100644 --- a/authbridge/cmd/abctl/edit/configmap.go +++ b/authbridge/cmd/abctl/edit/configmap.go @@ -97,7 +97,15 @@ func FindPipelineRange(innerYAML []byte) (start, end int, err error) { // and returns the result. Used to apply the user's edit to just the pipeline // subtree, leaving everything outside it byte-for-byte unchanged. Comments, // blank lines, and field ordering outside the pipeline subtree all survive. +// +// Returns innerYAML unchanged if start/end fall outside [0, len(innerYAML)] +// or if start > end. Callers normally derive offsets from FindPipelineRange +// on the same bytes, but the guard keeps the public API safe against +// mismatched callers (e.g. stale offsets after a mutation). func Splice(innerYAML []byte, start, end int, newSubtree []byte) []byte { + if start < 0 || end > len(innerYAML) || start > end { + return innerYAML + } var b bytes.Buffer b.Grow(len(innerYAML) - (end - start) + len(newSubtree)) b.Write(innerYAML[:start]) @@ -169,6 +177,8 @@ func BuildManifest(origCMYAML, newInner []byte) ([]byte, error) { if md.Kind != yaml.MappingNode { break } + // Reuse the backing array — we only ever shrink (drop pairs), never + // grow, so aliasing the original slice is safe and avoids an alloc. stripped := md.Content[:0] for j := 0; j < len(md.Content); j += 2 { k := md.Content[j].Value diff --git a/authbridge/cmd/abctl/edit/configmap_test.go b/authbridge/cmd/abctl/edit/configmap_test.go index 2775feb76..55e1c9aaa 100644 --- a/authbridge/cmd/abctl/edit/configmap_test.go +++ b/authbridge/cmd/abctl/edit/configmap_test.go @@ -380,6 +380,26 @@ func TestResolveAgentName_FallbackToPodSuffixStrip(t *testing.T) { } } +func TestSplice_OutOfBoundsReturnsInputUnchanged(t *testing.T) { + in := []byte("abc\ndef\n") + cases := []struct { + name string + start, end int + }{ + {"negative start", -1, 4}, + {"end past len", 0, 9999}, + {"start > end", 5, 2}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := Splice(in, c.start, c.end, []byte("X")) + if string(got) != string(in) { + t.Fatalf("want input back unchanged, got %q", got) + } + }) + } +} + // equalArgs checks two []string for equality. func equalArgs(a, b []string) bool { if len(a) != len(b) { From 92031284b23e36779694d15c3b482dd8988688c7 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 29 May 2026 10:33:26 -0400 Subject: [PATCH 24/24] docs(abctl): Sync README with the editor's full behavior The README's "Editing the pipeline" section had drifted from the actual behavior accumulated across the PR's iterations. Update it to cover: - Esc keybind is now phase-dependent: cancel during fetch/edit/apply, background during waiting/rollback (with footer flashes on result). - `kubectl apply` uses --field-manager=abctl --force-conflicts=true, taking SSA ownership from the kagenti-operator's webhook on first edit. - Auto-rollback fires when the in-pod reload fails, reconciling the on-disk ConfigMap back to what's actually running. Includes the third-party-edit caveat. - `e` is gated to picker mode; --endpoint mode flashes a hint. - Agent-name resolution from the pod's app.kubernetes.io/name label (with strip-last-two-segments fallback). - Tempfile sweep (>24h) runs on abctl launch; replaces the prior "clean up periodically" instruction. - Poll loop terminal states: Success / Failure / Unreachable / Timeout, including the new 5-consecutive-error unreachable signal. `r` keybind row now distinguishes its two cases (re-edit on post-fetch errors; re-fetch on fetch errors). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/README.md | 80 +++++++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 10 deletions(-) diff --git a/authbridge/cmd/abctl/README.md b/authbridge/cmd/abctl/README.md index b41a7efe4..26cdf87d6 100644 --- a/authbridge/cmd/abctl/README.md +++ b/authbridge/cmd/abctl/README.md @@ -92,8 +92,9 @@ The UI has three panes. `Enter` drills in; `Esc` backs out. | `e` | pipeline | edit pipeline subtree in `$EDITOR` | | `y` | edit/diff | apply the edit | | `N` | edit/diff | abort the edit | -| `r` | edit/error | re-open the editor with the same tempfile | -| `Esc` | edit/* | abort the edit, return to Pipeline pane | +| `r` | edit/error | retry: re-open the editor (post-edit failure) or refetch (fetch failure) | +| `Esc` | edit/{fetching,editing,applying} | abort the edit, return to Pipeline pane | +| `Esc` | edit/{waiting,rollback} | background the watch; result lands as a footer flash | | `?` | any | (reserved for future help overlay) | | `q` / `Ctrl+C` | any | quit | @@ -102,9 +103,11 @@ The UI has three panes. `Enter` drills in; `Esc` backs out. Press `e` on the Pipeline pane to edit the agent's runtime `pipeline:` subtree in `$EDITOR` (or `vi` if unset). On save, abctl shows a diff and asks `apply this change? (y/N)`. Confirming runs -`kubectl apply --server-side` against the per-agent ConfigMap, then -polls the framework's `/reload/status` until the reload completes -(success or failure). +`kubectl apply --server-side` against the per-agent ConfigMap with +`--field-manager=abctl --force-conflicts=true` (taking ownership of +`data.config.yaml` from the kagenti-operator's webhook on first +edit), then polls the framework's `/reload/status` until the reload +completes (success or failure). The single edit flow covers four operations: - **Edit a value** — change a config field of an existing plugin @@ -115,6 +118,50 @@ The single edit flow covers four operations: All four work because they're all just lines you change inside the pipeline subtree. +`e` is only available in picker mode. With `--endpoint`, the cluster +fields needed to fetch and apply aren't populated; pressing `e` +flashes a hint instead of opening a broken edit. + +### Agent-name resolution + +The per-agent ConfigMap is named `authbridge-config-`. abctl +resolves `` from the selected pod's `app.kubernetes.io/name` +label (kagenti-operator sets this). If the label is absent, abctl +falls back to stripping the last two dash-separated segments of the +pod name (the ReplicaSet hash + pod suffix). + +### Auto-rollback on reload failure + +If `kubectl apply` succeeds but the in-pod reload fails (unknown +plugin name, malformed config, validation error), the framework +keeps the previous in-memory pipeline serving requests. The on-disk +ConfigMap, however, now holds the bad YAML. abctl detects this via +`/reload/status` and re-applies the original ConfigMap content +captured at Fetch time, reconciling the on-disk state back to what's +actually running. The error overlay then reports +`reload failed: ; rolled back to previous ConfigMap`. + +The rollback is best-effort — with `--force-conflicts=true`, if a +third party (controller, kubectl edit, kustomize) modified the +ConfigMap between Fetch and the failed reload, the rollback +overwrites their change. The running pipeline is unaffected. + +### Backgrounding the watch + +Pressing `Esc` while waiting for hot-reload (or during rollback) +moves the watch to the background instead of aborting it. The +overlay closes, the footer flashes +`hot-reload watch moved to background; you'll be notified`, and you +can resume navigating the TUI. When the watch terminates, the +result lands as a one-line flash: + +- `hot-reload succeeded` +- `hot-reload failed: ; rolled back to previous ConfigMap` +- `hot-reload failed: ; rollback failed: ` (rare) + +Flashes auto-dismiss after a few seconds; if you miss one, query +`/reload/status` directly via the port-forward. + ### Permissions abctl shells out to `kubectl`; kubectl uses your kubeconfig. Editing @@ -126,17 +173,30 @@ surfaces verbatim in the overlay. abctl writes the editable pipeline subtree to `$TMPDIR/abctl-pipeline-*.yaml` on every edit. The tempfile is **left in place on every exit path** -(success, error, abort) so an interrupted edit is recoverable. Clean -up `/tmp/abctl-pipeline-*` periodically. +(success, error, abort) so an interrupted edit is recoverable. On +abctl launch, files older than 24h in this glob are swept +automatically — no manual cleanup needed. ### Hot-reload window The framework reloads via a config-file watcher; kubelet syncs ConfigMap edits into the pod's mount within ~60s, then the framework debounces and reloads. Total wall-clock from `apply` to reload is -typically under 90s. abctl shows a spinner during the wait. If -`/reload/status` doesn't observe a successful reload within 120s, -abctl gives up watching and tells you to check `kubectl logs deploy/`. +typically under 90s. abctl shows a spinner during the wait. + +The poller terminates with one of: + +- **Success** — `/reload/status.last_success` advances past the apply + time. +- **Failure** — `reloads_failed` increments past its baseline; the + framework's `last_error` is shown. +- **Unreachable** — 5 consecutive transport errors against + `:9093/reload/status` (port-forward dropped, framework crashed, + etc.) surface as `reload status endpoint unreachable` after a few + seconds rather than waiting the full deadline. +- **Timeout** — none of the above within 120s. Triggers an + auto-rollback so the on-disk ConfigMap doesn't drift from the + running pipeline. ## Trust model