Self-moving Kanban — cards move between columns live via SSE#1530
Merged
Conversation
7b86735 to
b30c944
Compare
… reload Move the engine.changed SSE subscription off GoalsKanbanView and into EngineGoalsClient (the shared-store owner). On engine.changed it now calls a new useGoalsStore.refreshActive() that re-fetches the first page of the active set and replaces active.goals in place — so Kanban cards slide between columns immediately, without the server re-render + page flash a bare router.refresh() caused. router.refresh() is still fired afterward to sync server-rendered metadata (bucket counts, filter options) that isn't part of active.goals. refreshActive() silently no-ops on a transient fetch failure so a blip never blanks the board. The subscription lives once in the client, not per-view. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… from 6144 to 4096
…s-kanban-view imports
…ntu-latest The single vitest worker accumulates 6+ GB of heap across 1175 test files, hitting the --max-old-space-size=6144 limit. With maxWorkers: "50%" on a 2-CPU runner that's still 1 worker for everything. Switching to pool:"forks" + a worker cap bounded by BOTH CPU count and available memory (floor(RAM / 2 GB)) distributes the heap across 2 workers on ubuntu-latest (7 GB / 2 GB = 3 budget, min(cpus-1=1, 3)=1, max(2,1)=2), halving peak memory per process. Also bump testTimeout to 30s to absorb residual contention headroom.
The non-stateful Portal mock rendered dialog content unconditionally, causing 'Found multiple elements with name Resume' when both the trigger button and the dialog action were simultaneously in DOM. The dialog is controlled (open/onOpenChange on Root), so the mock now uses React context to propagate open state and Portal renders only when open=true. Also fixes lint error: lucide-react mock now uses async factory with await import() instead of require(). vitest.config.ts: cap each fork's V8 heap at 1.5 GB via execArgv so cleanup GC stays within ubuntu-latest's 7 GB budget.
… React type refs
- Content: add role="alertdialog" so getByRole("alertdialog") resolves in tests
- lucide-react mock: add Clock export (used by goal-activity.tsx's GoalTime)
- Alert dialog mock: use React.ReactNode/ReactElement types instead of R.* to fix TS2503
1536 MB was set exactly at the ~1.5 GB peak each worker accumulates after running its ~293 test files — no GC headroom, OOM still triggered. 3072 MB gives 2× headroom so V8 can compact before hitting the limit. Peak concurrent: 1 worker teardown (3 GB) + 3 active (0.5 GB each) = 4.5 GB, within the 7 GB ubuntu-latest runner.
ce27674 to
5c73f04
Compare
The CI unit-test job OOM'd at 3067/3072 MB during GC cleanup after phases-plan-client.test.ts — hitting the execArgv ceiling exactly. Two changes: - execArgv heap raised from 3072 → 4096 MB (~30% headroom over observed peak) - memBudget formula changed from 2 GB/fork to 4 GB/fork and Math.max floor dropped from 2 → 1, so the 7 GB ubuntu-latest runner uses 1 fork instead of 2 (preventing two forks from competing for the same physical RAM)
Two shards × ~600 files each → ~2 GB peak per shard instead of ~4.1 GB for all 1191 files in one process. Keeps the same "unit tests" check name via a unit-ok aggregator job so branch protection continues to pass on the single expected check. fail-fast: false lets shard 2 keep running even if shard 1 fails, so both failure outputs are visible in one CI run.
The compaction phase of GC peaks at ~4.1 GB regardless of how many test files a shard handles (module object graph size is driven by the heaviest import, not file count). Hard caps at 3072 or 4096 MB both OOM'd reliably. Removing execArgv lets each fork inherit NODE_OPTIONS=--max-old-space-size=8192 from the npm test script. Workers won't actually use 8 GB — the ceiling just needs to be above the observed ~4.1 GB compaction peak. Sharding (2 jobs, each on its own 7 GB runner) keeps concurrent RAM usage bounded.
…hard heap cap)
The 2-way sharded unit-test job OOM'd shard 1 in a Vitest worker fork
("Reached heap limit — JavaScript heap out of memory", worker exited
unexpectedly) after ~7962 tests passed. Root cause was this branch's
vitest.config.ts, not any resumeGoalBacklog logic:
- the worker floor was lowered from Math.max(2,…) to Math.max(1,…)
- the per-fork memory divisor was raised 2 GB -> 4 GB
- the per-fork execArgv heap cap was dropped in favor of NODE_OPTIONS
inheritance, which does NOT reliably reach worker forks
On the 7 GB / 2-vCPU ubuntu-latest runner both cpuCount-1 and
memBudgetWorkers collapse to 1, so maxWorkers=1: a single fork ran the
entire ~596-file shard, accumulated module/heap state across files, and
blew the ceiling at teardown GC compaction. main survived because
Math.max(2,…) forced >=2 forks so each handled ~half the files.
Fix (belt and suspenders):
- restore Math.max(2,…) floor and the 2 GB divisor so no single fork
ever runs a whole shard (each handles ~half the files)
- add a hard per-fork heap ceiling via top-level test.execArgv
(--max-old-space-size=6144), sized above the observed ~4.1 GB GC
compaction peak, since NODE_OPTIONS does not reach forks
- reconcile the npm test script NODE_OPTIONS (8192 -> 6144) with the
ci.yml job env (6144) and the fork cap
Test-infra/CI change only — no user-facing data, tools, or workflows
change, so no PM agent config update is required.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eep OOM fix Unblocks PR #1530's sharded CI: the branch was CONFLICTING with main (pre-existing, unrelated to the OOM work), so GitHub couldn't build the merge commit and the required "unit tests (shard 1/2 & 2/2)" checks never ran. Conflict resolution — the goals surface (use-goals-view, engine-goals-client, goals-kanban-view + their tests): main independently shipped a superseding, finer-grained version of this branch's self-moving-Kanban feature. This branch refreshed the whole active set on engine.changed (refreshActive, subscription in EngineGoalsClient); main patches just the changed row (applyEngineChange / upsertGoalRow / removeGoalById, subscription in GoalsKanbanView, with reconnect handling). main's design fully satisfies "cards move between columns live via SSE" and is strictly more granular, so all five files are resolved to main's version; the superseded refreshActive path is dropped. No unique branch value in these files is lost — refreshActive was this branch's only change to them. vitest.config.ts + package.json auto-merged with the OOM fix intact (maxWorkers floor >=2, 2 GB divisor, per-fork execArgv --max-old-space-size=6144, npm test NODE_OPTIONS reconciled to 6144). Test-infra/merge only — no user-facing data, tools, or workflows change, so no PM agent config update is required. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
goals-kanban-view.tsx renders <ExternalLink> icons on the "View PR" and
"Reopen on GitHub" links, but the test file's vi.mock("lucide-react", ...)
never listed it, crashing 2 tests with "No 'ExternalLink' export is defined
on the 'lucide-react' mock".
…uct-manager into goal/4e57c48e
1 task
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
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.
Integration PR for this goal — its tasks commit onto this branch (one PR per goal). Opened automatically by the engine on first push; left as a draft until the goal completes.
Vitest sharded-worker heap OOM fix
The
unit tests (shard 1/2)+unit testsaggregator checks failed with a Vitest worker-fork heap OOM (Reached heap limit — JavaScript heap out of memory, worker exited unexpectedly) after ~7962 tests passed. Root cause was this branch'svitest.config.tschanges (worker floor dropped toMath.max(1,…), per-fork divisor raised to 4 GB), which collapsedmaxWorkersto 1 on the 7 GB / 2-vCPUubuntu-latestrunner so a single fork ran a whole ~596-file shard and blew the ceiling at teardown GC compaction — not aresumeGoalBacklogregression. Fixed by restoring the>=2worker floor + 2 GB divisor and adding a hard per-fork heap cap via top-leveltest.execArgv(--max-old-space-size=6144, sized above the ~4.1 GB compaction peak), plus reconciling NODE_OPTIONS between the job env and npm test script.Landing path taken: (a) — fix committed directly onto this goal's branch (
goal/4e57c48e). The broken config lives on this branch, so the correctedvitest.config.ts(+ci.yml/package.jsontouch) was committed here and #1530's own 2-way sharded CI self-validated the fix — run28618967714:unit tests (shard 1/2)SUCCESS (8493 passed / 35 skipped),unit tests (shard 2/2)SUCCESS (8736 passed / 10 skipped), aggregatorunit testsSUCCESS, 0 heap-limit / worker-exit markers.No PM agent config update required: this is a test-infra / CI-only change — no user-facing data, tools, or workflows change, so no
src/server/agents/configs/*orTOOL_NAMESupdate is needed (per CLAUDE.md).