Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions context/knowledge/gotchas/ci-gates.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
- **`lint-pr`** — uses `--new-from-rev=origin/master`, so it only flags issues your diff introduced (incl. `staticcheck` deprecations like `SA1019` on new lines). Fix them; no blanket `//nolint`.
- **`vuln`** — CI runs govulncheck with `continue-on-error: true`, so Go stdlib-only CVEs (`Found in: <pkg>@go1.x.y`, "Standard library") never block CI (fixable only by bumping the toolchain). Confirm the failure is toolchain-only (fails on a clean `origin/master` tree too), note it in the PR, run the remaining gates individually. Module-level findings in bumpable deps still must be fixed.
- **`test-cover-gate` flakes in `internal/agent` under full-suite `-race` on a loaded macOS dev machine** — real-PTY tests panic with `session.go: invalid memory address or nil pointer dereference` after a setup error `device not configured` (macOS PTY-device exhaustion when many packages' subprocess/PTY tests run concurrently). `go test ./internal/agent/...` in isolation passes cleanly; `go test -p 1 ./...` (fully serialized across packages) also passes with normal coverage. Confirm via that isolation/serialization split before suspecting a real regression — do NOT weaken or skip the affected tests. Not observed to reproduce on CI (Linux runners, different PTY limits).
- **The same class of flake also hits `internal/tui/terminal` (many `x/vt.SafeEmulator` goroutines each backed by an `io.Pipe`, same resource-contention shape as a PTY) — and, less often, `internal/tui` itself** — a per-package 120s timeout under full-suite `-race`, surfaced as a goroutine dump with everything parked in `io.(*pipe).read`/`SafeEmulator.Read` (`internal/tui/terminal`) or mid-`db.OpenInMemory()`/`sql.Open` (`internal/tui`), never a real panic. `go test ./internal/tui/terminal/...` in isolation passes in ~90s; confirmed reproducing with a fully clean tree (no diff at all) via `git stash`, so it is NOT a signal that a given change caused it — it's whichever package happens to be mid-flight when the loaded machine's scheduler starves it past 120s. Same diagnosis recipe as the `internal/agent` case above: isolate or serialize (`-p 1`) before suspecting a regression.
7 changes: 7 additions & 0 deletions context/knowledge/gotchas/hera-view.md
Original file line number Diff line number Diff line change
Expand Up @@ -280,3 +280,10 @@ M6a scaffolds the native Hera view: a `HeraPage` (rail | coordinator pane | agen
- **`Rail.MouseHandler` (mirrors `gitpanel.FilePanel` and `taskview.TaskListView`) is the FIX for the routing test's own stale comment: the rail previously had NO `MouseHandler`, so a wheel event that `page.go`'s `regionAt` misrouted to the rail column silently hit `Box`'s default (which only handles `MouseLeftDown`) and was never consumed — a genuinely broken scroll, not just an untested one.** `page.go`'s `MouseHandler` already gates dispatch on `regionAt(x) == FocusRail` before calling `p.rail.MouseHandler()`, so the new handler's own `InRect` check is a second, redundant-in-practice guard for direct callers (tests) rather than the thing doing the real gating.
- **Wheel up/down call the SAME `CursorUp`/`CursorDown` (→ `step()`) the `k`/`j`/arrow keys already use — no separate scroll-offset path.** `step()` already handles selectable-row skipping, kanban-group boundary crossing, and persistence (`setCursor`'s `persist()` call), so a wheel notch is indistinguishable from a keyboard nav step; `adjustOffset` (Draw-time) keeps the cursor in view exactly as it does for keyboard nav. This holds regardless of `filterInput` — `MouseHandler` calls `CursorUp`/`CursorDown` directly, bypassing the `InputHandler`'s filter-vs-nav key routing entirely, but both paths bottom out in the same `step()`.
- **`tview.MouseScrollUp` maps to `CursorDown` and `MouseScrollDown` maps to `CursorUp` — INVERTED relative to `gitpanel.FilePanel`'s pane-scroll convention, deliberately (dogfood feedback, same day as ship).** `FilePanel` (and `terminalpane`/`dagview`/`modal.HelpModal`/`SettingsView`) scroll a content PANE, where `ScrollUp` conventionally reveals earlier/upper content. This widget scrolls the CURSOR itself — the fingers are "dragging the cursor," not "dragging the pane" — so the intuitive mapping is the cursor moving in the SAME direction as the trackpad gesture, which on a Mac with natural scrolling reaches this widget as the OPPOSITE tview event from what a pane-scroll would want. Do not "fix" this to match `FilePanel` without re-confirming on an actual trackpad — the first shipped version used the pane-scroll direction and Aaron reported it backwards on his first test.

## BUG-076: Hera-pane size-drift kick never auto-restarts (false "Session not running" after ordinary rail nav)

- **Root cause was NOT a liveness misclassification — the session really did get stopped, by a legitimate-looking size-drift kick (`heraKickRerender`, BUG-074) whose auto-restart then unconditionally skipped itself because `handleSessionExitUI`'s "is the user still watching" gate (`stillViewing`) checked ONLY `a.mode == modeAgent` — the classic fullscreen agent view's mode. The native Hera view never sets `a.mode` to `modeAgent`; it stays `modeTaskList` with `header.ActiveTab()==TabHera` regardless of which pane the rail cursor is on. So EVERY kick fired from a Hera pane — and BUG-074 made that nearly every task's first Hera-view bind, since Hera panes are narrower than the main agent view and almost any previously-wider-attached (or never-attached) task exceeds `RerenderMargin` — took the "user navigated away, settle at InReview" branch verbatim, even though the operator was staring right at the pane through the Hera tab.** The kick's own gating (`ShouldKickRerender`) requires the session to be idle, so it doesn't fire mid-stream — it fires in the gap between turns, which is exactly the "moment" a rail j/k glance-away-and-back window covers: the operator sees it fine, looks away, and by the time they look back the async kick (RPC round trip + `QueueUpdateDraw` dispatch, see `maybeKickRerenderAtWidth`) has already stopped the session and skipped the restart. The task settles at InReview with a genuinely dead session; the pane's next resolve returns nil and `terminalpane.go`'s `sess == nil && !tp.HasContent()` branch paints "Session not running - press Enter to start" — literally true at that point, just for the wrong reason. Reproduces on BOTH the coordinator and agent/worker Hera panes (both route through `heraKickRerender`); the same `stillViewing` gate is also reachable from the classic agent view if the operator switches tasks inside the kick's async window, so the bug family isn't Hera-exclusive even though Hera nav is what makes it "very frequent."
- **Fix: widen the restart gate to recognize BOTH ways of "watching" — `App.isViewingTaskSession(taskID)` returns true for the classic `a.mode==modeAgent` case OR for `a.mode==modeTaskList && ActiveTab()==TabHera && heraPage.IsBoundToTask(taskID)`.** `HeraPage.IsBoundToTask` (`internal/tui/hera/panes.go`) is a pure read of `coordBound`/`agentBound` — true if EITHER pane currently shows taskID, independent of which pane has keyboard focus (the coordinator pane stays bound the whole time any worker under it is selected, so it must count as "viewed" even while the rail cursor sits on a worker row). `handleSessionExitUI`'s `stillViewing` now calls `isViewingTaskSession` instead of the inline `modeAgent`-only check; `startSession`'s own `a.mode==modeAgent` pane-attach guard is untouched (still correctly a no-op from the Hera branch — the classic `a.agentPane` isn't shown then, and the ALREADY-RUNNING Hera reconcile tick (`reconcileOne`'s late-bind case) picks up the freshly-resumed session on its own).
- **Do not "fix" this by making the kick less aggressive (e.g. skipping it from Hera panes) — the kick itself is correct and necessary** (BUG-074's whole point: a Hera pane's narrower width needs the same kill+resume repair the main agent view gets, or scrollback stays corrupted). The bug was purely in the EXIT-TIME restart decision, not the kick decision.
- Regression coverage: `internal/tui/hera/panes_test.go`'s `TestPanes_IsBoundToTask` (pure `IsBoundToTask` semantics — coordinator bound the whole time a worker is selected, unrelated/empty task rejected); `internal/tui/app_test.go`'s `TestApp_IsViewingTaskSession` (all four `isViewingTaskSession` branches, including "Hera-bound but a different tab is active"); `internal/tui/heraactions_test.go`'s `TestHandleSessionExitUI_RerenderRestartsWhenViewedViaHeraPane` (end-to-end through a real Hera tab selection + `handleSessionExitUI`, confirmed to fail with `status=in_review` against the old `modeAgent`-only gate before asserting the fixed behavior).
Loading
Loading