feat(abctl): In-place pipeline editor with hot-reload#448
Merged
Conversation
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Fetch reads authbridge-config-<agent> 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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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 rossoctl#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 rossoctl#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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an interactive pipeline editor to abctl: press ChangesPipeline Editor Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant TUI Model
participant EditPackage
participant kubectl
participant ConfigMap
participant Framework
participant Editor Process
User->>TUI Model: press 'e' on Pipeline pane
TUI Model->>TUI Model: editState.phase = fetching
TUI Model->>EditPackage: FetchCmd(namespace, pod)
EditPackage->>kubectl: kubectl get configmap
kubectl->>ConfigMap: return manifest YAML
EditPackage->>EditPackage: extractInnerYAML + FindPipelineRange
EditPackage-->>TUI Model: FetchedMsg{TempPath}
TUI Model->>Editor Process: open $EDITOR on TempPath
Editor Process->>TUI Model: editorExitedMsg
TUI Model->>TUI Model: validate YAML, compute Diff
User->>TUI Model: press 'y' to apply
TUI Model->>EditPackage: BuildManifest + ApplyCmd(manifest)
EditPackage->>kubectl: kubectl apply --server-side (temp file)
EditPackage->>TUI Model: AppliedMsg{ApplyTime}
TUI Model->>EditPackage: PollCmd(statusURL, ApplyTime)
EditPackage->>Framework: GET /reload/status (poll loop)
alt Reload Success
Framework-->>EditPackage: LastSuccessUnix > ApplyTime
EditPackage->>TUI Model: PolledMsg{PollSuccess}
else Reload Failure
Framework-->>EditPackage: ReloadsFailed increased
EditPackage->>TUI Model: PolledMsg{PollFailure, LastError}
else Timeout
EditPackage->>TUI Model: PolledMsg{PollTimeout}
end
TUI Model->>TUI Model: phase = done, refresh pipeline
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The Pipeline pane's "e" keybind was passing the selected pod name directly into the authbridge-config-<agent> 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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
authbridge/cmd/abctl/tui/edit_overlay.go (2)
49-84: ⚡ Quick winAdd a default case to handle unexpected phase values.
The switch on
s.phasehas no default branch. If an invalid phase is somehow set, the overlay would render an empty box with no diagnostic. Adding a default case that renders an error message would make debugging easier.🛡️ Proposed fix
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")) + default: + b.WriteString(styleTitle.Render("Edit pipeline — unknown phase")) + b.WriteString("\n\n") + b.WriteString(fmt.Sprintf("Internal error: unexpected phase %d", s.phase)) } return box.Render(b.String()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authbridge/cmd/abctl/tui/edit_overlay.go` around lines 49 - 84, The switch on s.phase (cases like editPhaseFetching, editPhaseEditing, editPhaseValidating, editPhaseDiff, editPhaseApplying, editPhaseWaiting, editPhaseError) needs a default branch to handle unexpected values: add a default case that renders a clear diagnostic using styleTitle/styleHint (e.g., title "Edit pipeline — unknown state") and include the actual s.phase value (via fmt.Sprintf) and any available s.err or a short guidance string so the overlay never renders empty when s.phase is invalid.
42-42: 💤 Low valueHeight parameter is unused.
The
heightparameter is accepted but never referenced. If vertical sizing is intentionally automatic (lipgloss auto-sizes to content when height is unset), consider removing the parameter or adding a comment explaining why it's reserved for future use.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authbridge/cmd/abctl/tui/edit_overlay.go` at line 42, The renderEditOverlay function currently accepts a height parameter that is never used; either remove the unused parameter from the function signature (renderEditOverlay) and all its callsites, or explicitly use it for vertical sizing in the overlay rendering logic (or add a concise comment inside renderEditOverlay explaining that lipgloss auto-sizes vertically and height is reserved for future use) so the intent is clear and the linter warning is resolved.authbridge/cmd/abctl/tui/edit_overlay_test.go (2)
46-52: ⚡ Quick winVerify recovery hints in error-phase test.
The error phase renders recovery instructions (
"[r] re-edit [Esc] back to Pipeline"), but the test only checks that the error message"forbidden"is present. Consider asserting that the recovery hints are also rendered to ensure users always see their options.🧪 Proposed enhancement
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) } + if !strings.Contains(out, "[r] re-edit") || !strings.Contains(out, "[Esc] back") { + t.Fatalf("error phase should show recovery hints:\n%s", out) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authbridge/cmd/abctl/tui/edit_overlay_test.go` around lines 46 - 52, The test TestEditOverlayRender_Error only asserts the error text is present; update it to also assert the recovery hint string rendered in the error phase by checking renderEditOverlay output for the hint (e.g. the literal "[r] re-edit [Esc] back to Pipeline" or its key fragments). Locate the test and render function references (TestEditOverlayRender_Error, renderEditOverlay, editState with editPhaseError) and add one or two additional assertions that the recovery instructions appear in out to ensure the error-phase UI shows the user options.
8-52: ⚡ Quick winAdd test coverage for
editPhaseEditingandeditPhaseValidating.The test suite covers five of the seven non-Done phases but omits
editPhaseEditing(which displays the temp path) andeditPhaseValidating(which shows "Validating YAML…"). Adding tests for these phases would ensure complete coverage of the overlay rendering logic.📝 Suggested tests
func TestEditOverlayRender_Editing(t *testing.T) { s := editState{ phase: editPhaseEditing, tempPath: "/tmp/pipeline-edit-12345.yaml", } out := renderEditOverlay(s, 80, 20) if !strings.Contains(out, "/tmp/pipeline-edit-12345.yaml") { t.Fatalf("editing phase should show temp path:\n%s", out) } } func TestEditOverlayRender_Validating(t *testing.T) { s := editState{phase: editPhaseValidating} out := renderEditOverlay(s, 80, 20) if !strings.Contains(out, "Validating YAML") { t.Fatalf("validating phase missing message:\n%s", out) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authbridge/cmd/abctl/tui/edit_overlay_test.go` around lines 8 - 52, Add two unit tests to cover editPhaseEditing and editPhaseValidating: create TestEditOverlayRender_Editing that builds an editState with phase: editPhaseEditing and tempPath set to a sample path and assert renderEditOverlay(s, 80, 20) contains that tempPath, and create TestEditOverlayRender_Validating that sets phase: editPhaseValidating and asserts the output contains "Validating YAML" (use the same renderEditOverlay call signature). Reference editState, tempPath, editPhaseEditing, editPhaseValidating, and renderEditOverlay when locating where to add these tests.authbridge/cmd/abctl/edit/configmap_test.go (1)
297-305: ⚡ Quick winMake the missing-trailing-newline regression assertion unconditional.
The current conditional can pass without proving the documented
"- name: bsession:"corruption. Assert that corruption directly so this contract can’t silently regress.Suggested fix
- 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) - } - } + // Verify that without normalization, splice produces the documented + // concatenation bug at the boundary. + if !bytes.Contains(spliced, []byte("- name: bsession:")) { + t.Fatalf("expected splice boundary corruption, got:\n%s", spliced) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authbridge/cmd/abctl/edit/configmap_test.go` around lines 297 - 305, The test currently guards the regression assertion behind a bytes.HasSuffix check so it can pass without proving the documented "- name: bsession:" corruption; make the regression assertion unconditional by always checking that spliced contains the corruption marker and failing the test if it does not (replace the outer if !bytes.HasSuffix(...) block with a direct if !bytes.Contains(spliced, []byte("- name: bsession:")) { t.Fatalf(...) }), keeping the explanatory comment about the TUI/Splice contract and using the existing variables spliced and the test helper t.Fatalf to surface regressions.authbridge/cmd/abctl/edit/status_test.go (1)
30-57: ⚡ Quick winUse bounded contexts in polling tests to avoid stuck CI.
PollUntilReloadedis loop-based; usingcontext.Background()here can hang indefinitely on regressions. Mirror the timeout pattern used in the other tests.♻️ Suggested change
func TestPollUntilReloaded_Success(t *testing.T) { applyTime := time.Now() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() 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) + res := PollUntilReloaded(ctx, srv.URL, applyTime) if res.Status != PollSuccess { t.Fatalf("status = %v, want PollSuccess", res.Status) } } func TestPollUntilReloaded_Failure(t *testing.T) { applyTime := time.Now() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() 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) + res := PollUntilReloaded(ctx, srv.URL, applyTime) if res.Status != PollFailure { t.Fatalf("status = %v, want PollFailure", res.Status) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authbridge/cmd/abctl/edit/status_test.go` around lines 30 - 57, Both tests create an unbounded context causing potential hangs; replace the context.Background() calls in TestPollUntilReloaded_Success and TestPollUntilReloaded_Failure with a bounded context (e.g., ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second); defer cancel()) and pass ctx into PollUntilReloaded so the loop in PollUntilReloaded can time out and the test won't hang indefinitely.authbridge/cmd/abctl/edit/edit_test.go (1)
13-28: ⚡ Quick winHarden tests against resource leaks and indefinite polling.
Please clean up the generated tempfile in
TestFetchCmd_Successand use a bounded context inTestPollCmd_Successto avoid hang-prone tests.♻️ Suggested change
import ( "context" "fmt" "net/http" "net/http/httptest" + "os" "strings" "testing" "time" ) @@ if msg.TempPath == "" { t.Fatal("FetchedMsg.TempPath empty (should be the path to the subtree-only tempfile)") } + t.Cleanup(func() { _ = os.Remove(msg.TempPath) }) } @@ - cmd := PollCmd(context.Background(), srv.URL, applyTime) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + cmd := PollCmd(ctx, srv.URL, applyTime)Also applies to: 58-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authbridge/cmd/abctl/edit/edit_test.go` around lines 13 - 28, TestFetchCmd_Success currently leaves the subtree tempfile behind and TestPollCmd_Success can hang; update TestFetchCmd_Success to remove the generated tempfile after the assertions by deleting the file at FetchedMsg.TempPath (use os.Remove or os.RemoveAll) and ensure it only runs when TempPath is non-empty, and update TestPollCmd_Success to use a bounded context (wrap context.Background() with context.WithTimeout and defer cancel) when calling PollCmd so the test cannot block indefinitely; reference the test functions TestFetchCmd_Success and TestPollCmd_Success and the returned FetchedMsg.TempPath from FetchCmd/PollCmd to locate where to add cleanup and the timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@authbridge/cmd/abctl/cluster/portforward.go`:
- Around line 86-89: freeLocalPort() is called twice and can return the same
port for both port and statusPort causing kubectl port-forward collisions;
change the logic around the second call (where statusPort is assigned) to loop
and call freeLocalPort() until it returns a value not equal to port (and handle
errors each attempt), e.g. retry with a small maxRetries guard and return an
error if exceeded; update the code near the freeLocalPort() call that assigns
statusPort and reference the variables port and statusPort and the
freeLocalPort() function when implementing the retry.
In `@authbridge/cmd/abctl/edit/e2e_test.go`:
- Around line 25-30: The test currently slices fp.InnerYAML using
fp.PipelineStart:fp.PipelineEnd before validating that fp.PipelineEnd >
fp.PipelineStart, which can cause a panic; in TestE2E_FetchExistingAgent move
the check "if fp.PipelineEnd <= fp.PipelineStart { t.Fatalf(...) }" to occur
before the line "subtree := fp.InnerYAML[fp.PipelineStart:fp.PipelineEnd]" so
the test fails with t.Fatalf on an invalid range rather than panicking; retain
the subsequent strings.Contains check for "pipeline:" as-is.
In `@authbridge/cmd/abctl/edit/edit.go`:
- Around line 77-80: PollCmd currently uses the caller-provided ctx so the 120s
timeout promised by its docs can be bypassed; change PollCmd to create its own
bounded context (e.g., ctxWithTimeout via context.WithTimeout(ctx,
120*time.Second)) and defer the cancel, then call PollUntilReloaded with that
ctxWithTimeout instead of the raw ctx so the poll always times out after 120s;
keep returning PolledMsg{Result: PollUntilReloaded(...)} and ensure cancel is
invoked to avoid leaks.
In `@authbridge/cmd/abctl/edit/status.go`:
- Around line 77-79: The comparison treating rs.LastSuccessUnix >
applyTime.Unix() as success is too strict for second-granularity timestamps;
change the condition to use a non-strict comparison (rs.LastSuccessUnix >=
applyTime.Unix()) so reloads occurring in the same second as applyTime are
considered PollSuccess. Update the check in the function/method that returns
PollResult (the block referencing rs.LastSuccessUnix and applyTime) to use >=
while keeping the return PollResult{Status: PollSuccess} unchanged.
In `@authbridge/cmd/abctl/tui/app.go`:
- Around line 865-873: The openEditorCmd function currently builds a shell
string and runs exec.Command("sh", "-c", editor+" "+path), which lets the shell
interpret $EDITOR and breaks paths with spaces; instead, split the editor
variable into program+args (e.g. using strings.Fields or a shell-word parser),
append path as a separate argument, and call exec.Command with the program and
args (no "sh -c"), then pass that *exec.Cmd into tea.ExecProcess so the editor
is launched without shell interpretation; update references in openEditorCmd
(editor, path, and the exec.Command call) accordingly.
In `@authbridge/cmd/abctl/tui/keys.go`:
- Around line 411-416: When handling editPhaseError in the "r" case, currently
we always call openEditorCmd(m.editState.tempPath) which retries with an empty
path when tempPath wasn't set; change the branch to check m.editState.tempPath
and, if it's empty, invoke the same fetch/retry routine used in
authbridge/cmd/abctl/tui/app.go where the tempfile is created (the code that
sets m.editState.tempPath on success) instead of calling openEditorCmd;
otherwise (when tempPath is present) keep setting m.editState.phase =
editPhaseEditing and call openEditorCmd(m.editState.tempPath). Ensure you
reference and call the existing fetch command/function used to obtain the
tempfile rather than introducing a new fetch implementation.
- Around line 208-216: The "e" key path must be gated by cluster-backed edit
prerequisites: before starting edit.FetchCmd ensure the cluster fields are
populated (e.g. check m.selectedNamespace and m.selectedPod are non-empty) and
short-circuit (return nil) when running in --endpoint mode or when those values
are missing; update the helpView() key advertisement to hide the "e" shortcut
unless the same prerequisites (m.pane == panePipeline && m.editState.phase ==
editPhaseDone && m.selectedNamespace != "" && m.selectedPod != "") are met so
the key is not shown when cluster-backed editing is not available.
---
Nitpick comments:
In `@authbridge/cmd/abctl/edit/configmap_test.go`:
- Around line 297-305: The test currently guards the regression assertion behind
a bytes.HasSuffix check so it can pass without proving the documented "- name:
bsession:" corruption; make the regression assertion unconditional by always
checking that spliced contains the corruption marker and failing the test if it
does not (replace the outer if !bytes.HasSuffix(...) block with a direct if
!bytes.Contains(spliced, []byte("- name: bsession:")) { t.Fatalf(...) }),
keeping the explanatory comment about the TUI/Splice contract and using the
existing variables spliced and the test helper t.Fatalf to surface regressions.
In `@authbridge/cmd/abctl/edit/edit_test.go`:
- Around line 13-28: TestFetchCmd_Success currently leaves the subtree tempfile
behind and TestPollCmd_Success can hang; update TestFetchCmd_Success to remove
the generated tempfile after the assertions by deleting the file at
FetchedMsg.TempPath (use os.Remove or os.RemoveAll) and ensure it only runs when
TempPath is non-empty, and update TestPollCmd_Success to use a bounded context
(wrap context.Background() with context.WithTimeout and defer cancel) when
calling PollCmd so the test cannot block indefinitely; reference the test
functions TestFetchCmd_Success and TestPollCmd_Success and the returned
FetchedMsg.TempPath from FetchCmd/PollCmd to locate where to add cleanup and the
timeout.
In `@authbridge/cmd/abctl/edit/status_test.go`:
- Around line 30-57: Both tests create an unbounded context causing potential
hangs; replace the context.Background() calls in TestPollUntilReloaded_Success
and TestPollUntilReloaded_Failure with a bounded context (e.g., ctx, cancel :=
context.WithTimeout(context.Background(), 5*time.Second); defer cancel()) and
pass ctx into PollUntilReloaded so the loop in PollUntilReloaded can time out
and the test won't hang indefinitely.
In `@authbridge/cmd/abctl/tui/edit_overlay_test.go`:
- Around line 46-52: The test TestEditOverlayRender_Error only asserts the error
text is present; update it to also assert the recovery hint string rendered in
the error phase by checking renderEditOverlay output for the hint (e.g. the
literal "[r] re-edit [Esc] back to Pipeline" or its key fragments). Locate the
test and render function references (TestEditOverlayRender_Error,
renderEditOverlay, editState with editPhaseError) and add one or two additional
assertions that the recovery instructions appear in out to ensure the
error-phase UI shows the user options.
- Around line 8-52: Add two unit tests to cover editPhaseEditing and
editPhaseValidating: create TestEditOverlayRender_Editing that builds an
editState with phase: editPhaseEditing and tempPath set to a sample path and
assert renderEditOverlay(s, 80, 20) contains that tempPath, and create
TestEditOverlayRender_Validating that sets phase: editPhaseValidating and
asserts the output contains "Validating YAML" (use the same renderEditOverlay
call signature). Reference editState, tempPath, editPhaseEditing,
editPhaseValidating, and renderEditOverlay when locating where to add these
tests.
In `@authbridge/cmd/abctl/tui/edit_overlay.go`:
- Around line 49-84: The switch on s.phase (cases like editPhaseFetching,
editPhaseEditing, editPhaseValidating, editPhaseDiff, editPhaseApplying,
editPhaseWaiting, editPhaseError) needs a default branch to handle unexpected
values: add a default case that renders a clear diagnostic using
styleTitle/styleHint (e.g., title "Edit pipeline — unknown state") and include
the actual s.phase value (via fmt.Sprintf) and any available s.err or a short
guidance string so the overlay never renders empty when s.phase is invalid.
- Line 42: The renderEditOverlay function currently accepts a height parameter
that is never used; either remove the unused parameter from the function
signature (renderEditOverlay) and all its callsites, or explicitly use it for
vertical sizing in the overlay rendering logic (or add a concise comment inside
renderEditOverlay explaining that lipgloss auto-sizes vertically and height is
reserved for future use) so the intent is clear and the linter warning is
resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eaacedef-0313-4dae-9f22-42e839763524
⛔ Files ignored due to path filters (1)
authbridge/cmd/abctl/go.sumis excluded by!**/*.sum
📒 Files selected for processing (22)
authbridge/cmd/abctl/README.mdauthbridge/cmd/abctl/cluster/portforward.goauthbridge/cmd/abctl/cluster/portforward_test.goauthbridge/cmd/abctl/edit/configmap.goauthbridge/cmd/abctl/edit/configmap_test.goauthbridge/cmd/abctl/edit/diff.goauthbridge/cmd/abctl/edit/diff_test.goauthbridge/cmd/abctl/edit/e2e_test.goauthbridge/cmd/abctl/edit/edit.goauthbridge/cmd/abctl/edit/edit_test.goauthbridge/cmd/abctl/edit/status.goauthbridge/cmd/abctl/edit/status_test.goauthbridge/cmd/abctl/go.modauthbridge/cmd/abctl/tui/app.goauthbridge/cmd/abctl/tui/edit_e2e_test.goauthbridge/cmd/abctl/tui/edit_overlay.goauthbridge/cmd/abctl/tui/edit_overlay_test.goauthbridge/cmd/abctl/tui/keys.goauthbridge/cmd/abctl/tui/namespaces_pane.goauthbridge/cmd/abctl/tui/picker_test.goauthbridge/docs/superpowers/plans/2026-05-28-abctl-pipeline-edit.mdauthbridge/docs/superpowers/specs/2026-05-28-abctl-pipeline-edit-design.md
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: <reason>; 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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
authbridge/cmd/abctl/edit/edit.go (1)
97-100:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce the polling timeout inside
PollCmdrather than relying on callers.The function documents a 120s timeout expectation but still delegates timeout enforcement to callers by passing the raw
ctxtoPollUntilReloaded. This leaves the edit flow vulnerable to indefinite hangs if the endpoint never reaches a terminal state.🛠️ Recommended fix
func PollCmd(ctx context.Context, statusURL string, applyTime time.Time) tea.Cmd { return func() tea.Msg { - return PolledMsg{Result: PollUntilReloaded(ctx, statusURL, applyTime)} + pollCtx, cancel := context.WithTimeout(ctx, 120*time.Second) + defer cancel() + return PolledMsg{Result: PollUntilReloaded(pollCtx, statusURL, applyTime)} } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authbridge/cmd/abctl/edit/edit.go` around lines 97 - 100, PollCmd currently passes the caller's ctx straight through to PollUntilReloaded, relying on callers to enforce the documented 120s timeout; modify PollCmd to create and use its own bounded context (e.g., ctxWithTimeout, using context.WithTimeout(ctx, 120*time.Second) and defer cancel()) and pass that to PollUntilReloaded so the poll cannot hang indefinitely; keep returning PolledMsg{Result: PollUntilReloaded(ctxWithTimeout, statusURL, applyTime)} and preserve existing behavior on timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@authbridge/cmd/abctl/tui/edit_e2e_test.go`:
- Around line 257-262: The test currently ignores setup/apply errors which can
hide root causes; update the rollback flow to fail fast by checking
FetchedMsg.Err after fetching (fetchedMsg := cmd().(edit.FetchedMsg)), checking
the error returned from os.WriteFile when writing to fetchedMsg.TempPath, and
checking AppliedMsg.Err after apply (AppliedMsg from cmd().(edit.AppliedMsg));
if any of these are non-nil return or call t.Fatalf/t.Fatalf-like helper
immediately with the error so the test stops with a clear message. Ensure you
reference the TempPath on fetchedMsg and the specific cmd() cast targets
(edit.FetchedMsg, edit.AppliedMsg) when adding these checks.
---
Duplicate comments:
In `@authbridge/cmd/abctl/edit/edit.go`:
- Around line 97-100: PollCmd currently passes the caller's ctx straight through
to PollUntilReloaded, relying on callers to enforce the documented 120s timeout;
modify PollCmd to create and use its own bounded context (e.g., ctxWithTimeout,
using context.WithTimeout(ctx, 120*time.Second) and defer cancel()) and pass
that to PollUntilReloaded so the poll cannot hang indefinitely; keep returning
PolledMsg{Result: PollUntilReloaded(ctxWithTimeout, statusURL, applyTime)} and
preserve existing behavior on timeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce2716b8-d997-4efc-9fbb-111a3a4fdb25
📒 Files selected for processing (4)
authbridge/cmd/abctl/edit/edit.goauthbridge/cmd/abctl/tui/app.goauthbridge/cmd/abctl/tui/edit_e2e_test.goauthbridge/cmd/abctl/tui/edit_overlay.go
🚧 Files skipped from review as they are similar to previous changes (2)
- authbridge/cmd/abctl/tui/edit_overlay.go
- authbridge/cmd/abctl/tui/app.go
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
pdettori
approved these changes
May 29, 2026
pdettori
left a comment
pdettori
left a comment
Member
There was a problem hiding this comment.
Solid PR — well-structured state machine, clean byte-range splice that preserves comments, proper SSA with force-conflicts, auto-rollback on reload failure, and good test coverage (unit + E2E). All 21 commits signed off, CI green.
CodeRabbit already flagged the key issues (sh -c injection, port race, retry semantics). Two additional minor suggestions below — neither blocking.
| // 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. |
Member
There was a problem hiding this comment.
suggestion: Splice trusts that start and end are within [0, len(innerYAML)]. If FindPipelineRange ever returns stale offsets (e.g. the YAML was mutated between Find and Splice), this panics on the slice expression.
A one-line guard at the top would harden the public API:
if start < 0 || end > len(innerYAML) || start > end {
return nil // or return innerYAML unchanged
}Non-blocking — callers currently always validate via FindPipelineRange on the same bytes.
| md := doc.Content[i+1] | ||
| if md.Kind != yaml.MappingNode { | ||
| break | ||
| } |
Member
There was a problem hiding this comment.
nit: stripped := md.Content[:0] reuses the underlying backing array (fine since we only shrink), but a reader might wonder if it's intentional. Either a tiny comment (// reuse backing; we only drop elements) or an explicit make([]* yaml.Node, 0, len(md.Content)) would clarify intent. Very minor.
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
github-project-automation
Bot
moved this from New /:ToDo
to Done
in Kagenti Issue Prioritization
9 tasks
6 tasks
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
abctl: presseon the Pipeline pane to edit the per-agentauthbridge-config-<agent>ConfigMap'spipeline:subtree in$EDITOR, see a diff, confirm, and apply viakubectl apply --server-side. The framework's existing file-watcher hot-reloads the new config;abctlpolls:9093/reload/statusto surface success or failure.:9093alongside:9094in the picker's port-forward so reload status is reachable without a second terminal.pipeline:subtree (yaml.v3 Node tree locates the range; emit is a literal-text splice, not a re-marshal).tea.ExecProcesssuspends the TUI for$EDITOR) → Validating (YAML parse + empty-input + no-changes guards) → Diff (LCS line diff with green/red lipgloss styling,y/Nto apply) → Applying → Waiting (poll/reload/statusfor new SHA) → Done. Errors land on a dedicated phase withrto retry /Escto abort.data.config\.yamlof the named ConfigMap; bad YAML in the editor → diff phase rejects with the parser's line/col message; no-changes → flash "no changes; nothing to apply" and abort cleanly.Test Plan
go test -race ./...acrossapiclient,cluster,edit,tui(4 packages, all green)TestEditFlow_HappyPath— full state-machine drive with a stub Runner +httptest/reload/statusserverTestEditFlow_NCancelsAtDiff—Nat the diff phase aborts without calling ApplyTestEditFlow_NormalizesTrailingNewline— edited bytes without a trailing\nare normalized before spliceTestE2E_FetchExistingAgent(build-tage2e) — liveFetchagainst an IBAC cluster/reload/statusSHA flip; bad-YAML rejected with line/col; no-changes flash;Esc-from-error returns to Pipeline panegofmt -lclean across the new filesAssisted-By: Claude (Anthropic AI) noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation
Tests