feat: drain ACP queued nudges in reconciler tick#2
Open
tesdal wants to merge 339 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an in-process “ACP queued nudge drain” path to ensure deferred nudges targeting ACP sessions are delivered during the supervisor’s beadReconcileTick, since gc nudge poll intentionally skips ACP sessions.
Changes:
- Add
drainACPQueuedNudgesto claim and deliver due queued nudges to ACP sessions via the in-processruntime.Provider. - Add
buildACPSessionMapto derive ACP agent-key → session-name routing fromDesiredStateResult. - Wire the drain into
CityRuntime.beadReconcileTickand add tests covering drain behavior and the reconcile→drain integration path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| cmd/gc/nudge_acp_drain.go | Implements ACP queued nudge draining and ACP session routing map construction. |
| cmd/gc/nudge_acp_drain_test.go | Adds unit + integration tests for ACP drain and reconcile tick wiring. |
| cmd/gc/city_runtime.go | Calls ACP drain after dispatchReadyWaitNudges during reconcile tick. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+51
to
+78
| if !sp.IsRunning(sessName) { | ||
| _ = nudgequeue.WithState(cityPath, func(s *nudgequeue.State) error { | ||
| item.ClaimedAt = time.Time{} | ||
| item.Attempts++ | ||
| item.LastError = "session not running" | ||
| item.LastAttemptAt = now | ||
| s.Pending = append(s.Pending, item) | ||
| return nil | ||
| }) | ||
| continue | ||
| } | ||
| err := sp.Nudge(sessName, runtime.TextContent(item.Message)) | ||
| if err != nil { | ||
| _ = nudgequeue.WithState(cityPath, func(s *nudgequeue.State) error { | ||
| item.ClaimedAt = time.Time{} | ||
| item.Attempts++ | ||
| item.LastError = err.Error() | ||
| item.LastAttemptAt = now | ||
| if item.Attempts >= 5 { | ||
| item.DeadAt = now | ||
| s.Dead = append(s.Dead, item) | ||
| } else { | ||
| item.DeliverAfter = now.Add(15 * time.Second) | ||
| s.Pending = append(s.Pending, item) | ||
| } | ||
| return nil | ||
| }) | ||
| continue |
Owner
Author
There was a problem hiding this comment.
Fixed in 1ad38cf — rewritten to use ackQueuedNudges and recordQueuedNudgeFailure, which propagate errors.
Owner
Author
There was a problem hiding this comment.
Addressed in the rewrite (61661f9) — now uses claimDueQueuedNudgesForTarget, ackQueuedNudges, and recordQueuedNudgeFailure which handle all state transitions atomically. No manual WithState calls remain.
Comment on lines
+50
to
+60
| sessName := acpSessions[item.Agent] | ||
| if !sp.IsRunning(sessName) { | ||
| _ = nudgequeue.WithState(cityPath, func(s *nudgequeue.State) error { | ||
| item.ClaimedAt = time.Time{} | ||
| item.Attempts++ | ||
| item.LastError = "session not running" | ||
| item.LastAttemptAt = now | ||
| s.Pending = append(s.Pending, item) | ||
| return nil | ||
| }) | ||
| continue |
Owner
Author
There was a problem hiding this comment.
Fixed in 1ad38cf — failure path now goes through recordQueuedNudgeFailure → failedQueuedNudge, which applies retry delay and max attempts.
Owner
Author
There was a problem hiding this comment.
Addressed in the rewrite — sessions that aren't running are now skipped entirely (line 37). No requeue, no retry loop. Only running sessions get their nudges claimed.
| kept = append(kept, item) | ||
| continue | ||
| } | ||
| if !item.ExpiresAt.IsZero() && item.ExpiresAt.Before(now) { |
Owner
Author
There was a problem hiding this comment.
Fixed in 1ad38cf — expiry is now handled by claimDueQueuedNudgesMatching which calls pruneExpiredQueuedNudges with the standard !ExpiresAt.After(now) check.
Owner
Author
There was a problem hiding this comment.
Addressed in the rewrite — no custom expiry check remains. Expiry is now handled by claimDueQueuedNudgesForTarget which uses the same boundary logic as the poller.
Comment on lines
+86
to
+99
| func buildACPSessionMap(result DesiredStateResult) map[string]string { | ||
| m := make(map[string]string) | ||
| for _, tp := range result.State { | ||
| if !tp.IsACP || tp.SessionName == "" { | ||
| continue | ||
| } | ||
| key := tp.Alias | ||
| if key == "" { | ||
| key = tp.SessionName | ||
| } | ||
| if _, exists := m[key]; !exists { | ||
| m[key] = tp.SessionName | ||
| } | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in 1ad38cf — buildACPSessionMap now picks the lexicographically smallest session name when aliases collide, making routing deterministic regardless of map iteration order.
Owner
Author
There was a problem hiding this comment.
Addressed in the rewrite — buildACPSessionMap no longer exists. Replaced by buildACPNudgeTargets which builds one target per ACP session (keyed by SessionName which is unique). No alias-based deduplication or map iteration order issues.
Comment on lines
+229
to
+233
| t.Fatalf("map size = %d, want 1 (one agent key, first wins)", len(m)) | ||
| } | ||
| if _, ok := m["hermes/polecat"]; !ok { | ||
| t.Error("hermes/polecat should be in ACP map") | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in 1ad38cf — test now asserts the specific winner (hermes--polecat < s-ci-f1co lexicographically).
Owner
Author
There was a problem hiding this comment.
Addressed — this test and the buildACPSessionMap function it tested no longer exist in the rewrite.
Comment on lines
+18
to
+39
| var claimed []nudgequeue.Item | ||
| err := nudgequeue.WithState(cityPath, func(s *nudgequeue.State) error { | ||
| var kept []nudgequeue.Item | ||
| for _, item := range s.Pending { | ||
| if _, isACP := acpSessions[item.Agent]; !isACP { | ||
| kept = append(kept, item) | ||
| continue | ||
| } | ||
| if item.DeliverAfter.After(now) { | ||
| kept = append(kept, item) | ||
| continue | ||
| } | ||
| if !item.ExpiresAt.IsZero() && item.ExpiresAt.Before(now) { | ||
| item.DeadAt = now | ||
| s.Dead = append(s.Dead, item) | ||
| continue | ||
| } | ||
| item.ClaimedAt = now | ||
| claimed = append(claimed, item) | ||
| } | ||
| s.Pending = kept | ||
| return nil |
Owner
Author
There was a problem hiding this comment.
Fixed in 1ad38cf — rewritten to use claimDueQueuedNudgesMatching (Pending→InFlight with LeaseUntil), ackQueuedNudges (success), and recordQueuedNudgeFailure (failure with retry/dead-letter). No custom queue state management.
Owner
Author
There was a problem hiding this comment.
Addressed in the rewrite — claiming is now done by claimDueQueuedNudgesForTarget which manages InFlight state internally, matching the poller behavior exactly.
tesdal
force-pushed
the
fix/acp-nudge-reconciler-drain
branch
from
c325df4 to
1ad38cf
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds an in-process drain path for queued nudges targeting ACP sessions, running during the reconciler tick so queued nudges can be delivered via the supervisor’s live ACP connections (since gc nudge poll intentionally skips ACP).
Changes:
- Added
drainACPQueuedNudgesto claim and deliver due queued nudges via the in-processruntime.Provider. - Added
buildACPSessionMapto route agent keys to ACP session names fromDesiredStateResult. - Wired the ACP drain into
CityRuntime.beadReconcileTick, plus added unit/integration tests for the drain path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| cmd/gc/nudge_acp_drain.go | Implements ACP queued-nudge claiming/routing and delivery via in-process provider. |
| cmd/gc/nudge_acp_drain_test.go | Adds unit tests + an integration-style test covering beadReconcileTick → ACP drain. |
| cmd/gc/city_runtime.go | Invokes ACP queued-nudge drain during reconciler tick after wait nudge dispatch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+30
to
+53
| claimed, err := claimDueQueuedNudgesMatching(cityPath, now, func(item queuedNudge) bool { | ||
| _, isACP := acpSessions[item.Agent] | ||
| return isACP | ||
| }) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("claiming ACP nudges: %w", err) | ||
| } | ||
| if len(claimed) == 0 { | ||
| return 0, nil | ||
| } | ||
|
|
||
| delivered := 0 | ||
| var failed []string | ||
| var succeeded []string | ||
| for _, item := range claimed { | ||
| sessName := acpSessions[item.Agent] | ||
| if !sp.IsRunning(sessName) { | ||
| failed = append(failed, item.ID) | ||
| continue | ||
| } | ||
| if err := sp.Nudge(sessName, runtime.TextContent(item.Message)); err != nil { | ||
| failed = append(failed, item.ID) | ||
| continue | ||
| } |
Owner
Author
There was a problem hiding this comment.
Addressed in the rewrite (61661f9) — now uses claimDueQueuedNudgesForTarget with proper nudgeTarget (SessionID, ContinuationEpoch, aliasHistory populated from session bead metadata via buildACPNudgeTargets). Then splitQueuedNudgesForTarget for fence-mismatch rejection, splitQueuedNudgesForDelivery for wait-bead blocking. Full poller pipeline.
Comment on lines
+41
to
+56
| delivered := 0 | ||
| var failed []string | ||
| var succeeded []string | ||
| for _, item := range claimed { | ||
| sessName := acpSessions[item.Agent] | ||
| if !sp.IsRunning(sessName) { | ||
| failed = append(failed, item.ID) | ||
| continue | ||
| } | ||
| if err := sp.Nudge(sessName, runtime.TextContent(item.Message)); err != nil { | ||
| failed = append(failed, item.ID) | ||
| continue | ||
| } | ||
| succeeded = append(succeeded, item.ID) | ||
| delivered++ | ||
| } |
Owner
Author
There was a problem hiding this comment.
Addressed in the rewrite — line 76 uses formatNudgeRuntimeMessage(items) to batch all items into one message, then a single sp.Nudge() call per session (line 78).
Comment on lines
+63
to
+66
| if len(failed) > 0 { | ||
| if err := recordQueuedNudgeFailure(cityPath, failed, fmt.Errorf("ACP in-process delivery failed"), now); err != nil { | ||
| return delivered, fmt.Errorf("recording ACP nudge failures: %w", err) | ||
| } |
Owner
Author
There was a problem hiding this comment.
In the rewrite, recordQueuedNudgeFailure is called with the actual err from Nudge() (line 79), not a generic string. All items in a batch share the same error because they were sent as one message — this is correct behavior, matching the poller.
Comment on lines
+1307
to
1321
| // Drain queued nudges for ACP sessions in-process. The nudge | ||
| // poller (gc nudge poll) skips ACP sessions because it spawns a | ||
| // separate process without in-process ACP connections. The | ||
| // reconciler runs inside the supervisor and holds the live | ||
| // provider, so it can deliver directly. | ||
| acpMap := buildACPSessionMap(result) | ||
| if len(acpMap) > 0 { | ||
| if _, err := drainACPQueuedNudges(cr.cityPath, cr.sp, acpMap, time.Now()); err != nil { | ||
| fmt.Fprintf(cr.stderr, "%s: draining ACP nudges: %v\n", cr.logPrefix, err) //nolint:errcheck | ||
| } | ||
| } | ||
|
|
||
| // Idle recovery: detect pool sessions stuck at the prompt after | ||
| } | ||
|
|
Owner
Author
There was a problem hiding this comment.
Known limitation, documented in the commit. This is the same behavior as the poller — Nudge() blocks via waitIdle with nudgeBusyTimeout(). The reconciler tick is not latency-sensitive (it runs on a timer), so a brief stall is acceptable. A future improvement could add a context timeout, but that's out of scope for parity with the poller.
| acpSessions := map[string]string{ | ||
| "hermes/polecat": "hermes--polecat", | ||
| } | ||
| delivered, _ := drainACPQueuedNudges(cityPath, sp, acpSessions, now) |
Owner
Author
There was a problem hiding this comment.
Fair point — will fix. Tests should assert err == nil.
Comment on lines
+73
to
+85
| _ = nudgequeue.WithState(cityPath, func(s *nudgequeue.State) error { | ||
| s.Pending = append(s.Pending, nudgequeue.Item{ | ||
| ID: "nudge-future", | ||
| Agent: "hermes/polecat", | ||
| Message: "not yet", | ||
| Source: "session", | ||
| CreatedAt: now, | ||
| DeliverAfter: now.Add(1 * time.Hour), | ||
| ExpiresAt: now.Add(24 * time.Hour), | ||
| }) | ||
| return nil | ||
| }) | ||
|
|
Owner
Author
There was a problem hiding this comment.
Fair point — will fix test setup to assert WithState errors.
Comment on lines
+30
to
+53
| claimed, err := claimDueQueuedNudgesMatching(cityPath, now, func(item queuedNudge) bool { | ||
| _, isACP := acpSessions[item.Agent] | ||
| return isACP | ||
| }) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("claiming ACP nudges: %w", err) | ||
| } | ||
| if len(claimed) == 0 { | ||
| return 0, nil | ||
| } | ||
|
|
||
| delivered := 0 | ||
| var failed []string | ||
| var succeeded []string | ||
| for _, item := range claimed { | ||
| sessName := acpSessions[item.Agent] | ||
| if !sp.IsRunning(sessName) { | ||
| failed = append(failed, item.ID) | ||
| continue | ||
| } | ||
| if err := sp.Nudge(sessName, runtime.TextContent(item.Message)); err != nil { | ||
| failed = append(failed, item.ID) | ||
| continue | ||
| } |
Owner
Author
There was a problem hiding this comment.
Addressed in the rewrite — tests now cover session fencing (TestDrainACPQueuedNudges_SessionFenceMismatch), buildACPNudgeTargets with/without session beads, and the full pipeline integration through beadReconcileTick. Wait-bead coverage relies on the existing splitQueuedNudgesForDelivery tests since we call it directly.
tesdal
force-pushed
the
fix/acp-nudge-reconciler-drain
branch
from
1ad38cf to
61661f9
Compare
There was a problem hiding this comment.
Pull request overview
This PR ensures ACP sessions can receive queued nudges by draining and delivering them from inside the supervisor/controller reconciler tick (since the external gc nudge poll process can’t access in-process ACP connections).
Changes:
- Added an in-process ACP queued-nudge drain (
drainACPQueuedNudges) invoked duringbeadReconcileTick. - Added ACP target construction (
buildACPNudgeTargets) using desired state + session bead fencing metadata. - Added unit + integration tests covering due delivery, batching, fencing, and the
beadReconcileTickdrain path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/gc/nudge_acp_drain.go | Implements ACP queued-nudge draining and ACP target construction for reconciler-driven delivery |
| cmd/gc/nudge_acp_drain_test.go | Adds tests for due delivery, batching, fence behavior, and a full tick→drain integration path |
| cmd/gc/city_runtime.go | Wires the ACP drain into beadReconcileTick after wait-nudge dispatch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+85
to
+89
| if err := ackQueuedNudges(cityPath, queuedNudgeIDs(items)); err != nil { | ||
| return totalDelivered, fmt.Errorf("acking ACP nudges: %w", err) | ||
| } | ||
| totalDelivered += len(items) | ||
| } |
Owner
Author
There was a problem hiding this comment.
Valid concern. However, this is inherent to the poller path too — Nudge() is documented as best-effort. If a session stops between IsRunning and Nudge(), the nudge was already claimed and acked. The poller has the same race window. Adding explicit detection here would diverge from poller semantics. The nudge will be lost, but that's the documented contract for best-effort delivery. A future improvement could check the Nudge() return and record a failure if it silently no-ops, but that would require changes to the Provider.Nudge interface (currently returns nil for non-existent sessions).
There was a problem hiding this comment.
Pull request overview
This PR adds an in-process drain path for queued nudges targeting ACP sessions by running the drain during CityRuntime.beadReconcileTick, ensuring ACP sessions receive queued nudges even though gc nudge poll intentionally skips ACP.
Changes:
- Add
drainACPQueuedNudgesto claim/deliver due queued nudges for ACP sessions via the in-process runtime provider. - Add
buildACPNudgeTargetsto derive per-session routing + fencing data from desired state and session bead metadata. - Wire ACP drain execution into
beadReconcileTickand add unit/integration tests covering the drain path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/gc/nudge_acp_drain.go | Implements ACP queued-nudge draining + ACP nudge target construction. |
| cmd/gc/nudge_acp_drain_test.go | Adds unit + integration tests for ACP drain and target building. |
| cmd/gc/city_runtime.go | Invokes ACP drain during the reconciler tick after wait-nudge dispatch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| continue | ||
| } | ||
|
|
Owner
Author
There was a problem hiding this comment.
Acknowledged — this is the same race the poller has (e.g., tryDeliverQueuedNudgesByPoller at line 326 of cmd_nudge.go does IsRunning then Nudge with the same gap). Changing the behavior here would diverge from the established contract. Filed as a future improvement — would require Provider.Nudge to distinguish 'delivered' from 'session gone' in its return value.
Comment on lines
+59
to
+63
| // Filter out nudges blocked by wait-bead state (canceled, | ||
| // expired, etc.). | ||
| store := openNudgeBeadStore(cityPath) | ||
| items, blocked, err := splitQueuedNudgesForDelivery(store, items) | ||
| if err != nil { |
Owner
Author
There was a problem hiding this comment.
Good observation. However, openNudgeBeadStore may return a different store than cr.cityBeadStore() (standalone vs city store — see nudge_beads.go). The poller itself calls openNudgeBeadStore per-target throughout (lines 326, 429, 476, 554, 609, 738, 1064, 1099 of cmd_nudge.go). Matching that pattern keeps the code consistent and avoids subtle store-mismatch bugs. The cost is negligible (it's a file-backed store open, not a network call).
There was a problem hiding this comment.
Pull request overview
This PR adds an in-process drain path for queued nudges targeting ACP sessions, since the existing gc nudge poll mechanism runs out-of-process and can’t use the supervisor’s in-process ACP connections.
Changes:
- Add
drainACPQueuedNudgesto claim due queued nudges for ACP sessions and deliver them via the in-process runtime provider duringbeadReconcileTick. - Add
buildACPNudgeTargetsto construct per-sessionnudgeTargets (including fencing metadata from session beads) for ACP sessions in desired state. - Add unit + integration tests covering the drain behavior and the reconciler tick → drain path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cmd/gc/nudge_acp_drain.go | Implements ACP queued-nudge draining and ACP target construction from desired state + session beads. |
| cmd/gc/nudge_acp_drain_test.go | Adds tests for due/undue nudges, non-ACP agents, session-not-running behavior, batching, and tick integration. |
| cmd/gc/city_runtime.go | Wires ACP drain invocation into beadReconcileTick after wait-nudge dispatch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+59
to
+63
| // Filter out nudges blocked by wait-bead state (canceled, | ||
| // expired, etc.). | ||
| store := openNudgeBeadStore(cityPath) | ||
| items, blocked, err := splitQueuedNudgesForDelivery(store, items) | ||
| if err != nil { |
Owner
Author
There was a problem hiding this comment.
Previously addressed — see reply to comment #3117015236. The poller uses openNudgeBeadStore per-call throughout cmd_nudge.go (10+ call sites). This is the established pattern. openNudgeBeadStore and cr.cityBeadStore() may return different store types (standalone vs city).
| // provider, so it can deliver directly. | ||
| acpTargets := buildACPNudgeTargets(cr.cityPath, cr.cfg, result, sessionBeads) | ||
| if len(acpTargets) > 0 { | ||
| if _, err := drainACPQueuedNudges(cr.cityPath, cr.cfg, cr.sp, acpTargets, time.Now()); err != nil { |
Owner
Author
There was a problem hiding this comment.
Same as above — matching the poller's established pattern. See reply to #3117015236.
Comment on lines
+94
to
+106
| // buildACPNudgeTargets builds nudgeTarget values for all active ACP | ||
| // sessions, using session bead metadata for fencing (SessionID, | ||
| // ContinuationEpoch) and DesiredStateResult for ACP routing. | ||
| // | ||
| // This mirrors how resolveNudgeTargetFromSessionBead builds targets for | ||
| // the poller, but sources data from the reconciler's in-memory state | ||
| // instead of a fresh store query. | ||
| func buildACPNudgeTargets( | ||
| cityPath string, | ||
| cfg *config.City, | ||
| result DesiredStateResult, | ||
| sessionBeads *sessionBeadSnapshot, | ||
| ) []nudgeTarget { |
Owner
Author
There was a problem hiding this comment.
Good catch — updated the PR description to reflect buildACPNudgeTargets instead of buildACPSessionMap.
Comment on lines
+75
to
+87
| // Batch into one message per session (matches poller behavior). | ||
| msg := formatNudgeRuntimeMessage(items) | ||
|
|
||
| if err := sp.Nudge(target.sessionName, runtime.TextContent(msg)); err != nil { | ||
| if recErr := recordQueuedNudgeFailure(cityPath, queuedNudgeIDs(items), err, now); recErr != nil { | ||
| return totalDelivered, fmt.Errorf("recording ACP nudge failure: %w", recErr) | ||
| } | ||
| continue | ||
| } | ||
|
|
||
| if err := ackQueuedNudges(cityPath, queuedNudgeIDs(items)); err != nil { | ||
| return totalDelivered, fmt.Errorf("acking ACP nudges: %w", err) | ||
| } |
Owner
Author
There was a problem hiding this comment.
Previously addressed — see replies to #3116325668 and #3117015199. This is the same race the poller has. Nudge() is best-effort by contract.
julianknutsen
force-pushed
the
fix/opencode-acp-command
branch
7 times, most recently
from
eb9ecff to
0d2e185
Compare
tesdal
force-pushed
the
fix/acp-nudge-reconciler-drain
branch
6 times, most recently
from
72b602e to
ea259b6
Compare
…entity-stability-complete-v2 fix: preserve pool slot identity for live sessions
## Summary - avoid expanding an empty preserved-env array in the Go test sharder, which can fail under older macOS bash - add a regression fixture proving the sharder runs when no provider env needs preserving - replace grep-based maintenance database filters with portable shell case filtering for RC macOS ## Verification - bash -n scripts/test-go-test-shard examples/gastown/packs/maintenance/assets/scripts/reaper.sh examples/gastown/packs/maintenance/assets/scripts/jsonl-export.sh - go test ./scripts -run 'TestGoTestShard' -count=1 - go test ./examples/gastown -run TestMaintenanceDoltScriptsSkipTestPatternDatabases -count=1 - go test ./scripts ./examples/gastown -count=1 <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1748"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer -->
## Summary - serialize the RC real-inference lane behind CI parity to avoid overlapping Synthetic Claude demand - limit RC acceptance/tutorial matrices so they do not burst 429 the shared Synthetic account - add a stdlib workflow policy test for the RC throttling contract ## RC evidence Iteration 2 failed with multiple Synthetic Claude 429 responses while acceptance C, tutorial goldens, and CI parity real-inference jobs ran concurrently. The macOS sharder and maintenance DB filter fixes from gastownhall#1748 held: mac fast jobs no longer failed immediately. ## Verification - python3 .github/workflows/scripts/test_rc_gate_policy.py - python3 .github/workflows/scripts/test_runner_policy.py <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1750"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer -->
…eaper-schema fix(maintenance): stop reaper/jsonl-export from storming Dolt
## Summary - Refetch the spawned session bead before asserting persisted launch command metadata in the fresh-init Tier C test. - Avoid asserting against the stale bead snapshot captured before session metadata is fully committed. ## Test plan - go test -tags acceptance_c -run 'TestTierCEnvAuthDoesNotMirrorAuthTokenIntoAPIKey' -count=1 ./test/acceptance/tier_c - pre-commit fast unit loop <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1757"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer -->
Release v1.1.0
…acceptance ci: route Claude acceptance tests through Ollama Cloud
… (boot) (gastownhall#1769) ## Summary - Replace the broken pipeline `gc mail inbox --address=deacon --json 2>/dev/null | jq length` with `gc mail count deacon 2>/dev/null` in `examples/gastown/packs/gastown/agents/boot/prompt.template.md` line 55. - `gc mail inbox` accepts `[session]` as a positional and has no `--json` flag — the original line failed at runtime with two flag errors (`--address` not recognized, `--json` not recognized). The intent (count deacon's unread mail to triage idle state) is exactly what `gc mail count [session]` reports directly. - No new API surface — `gc mail count` is already part of the documented `gc mail` command set; this is a swap from the broken inbox-pipeline pattern to the count subcommand that exists for this purpose. Verified via `--help`: ``` $ gc mail count --help Show total and unread message counts for a session alias or human. The recipient defaults to $GC_SESSION_ID, $GC_ALIAS, $GC_AGENT, or "human". Usage: gc mail count [session] [flags] ``` ## Testing - [x] `~/cities/test-series/bin/lint-pack ~/wrk/gascity/examples/gastown/ --check command-syntax` — boot:55's two violations (one for each invalid flag) are removed by this change. Remaining errors on main are covered by other open PRs (gastownhall#1743 fixes `gc agent peek/list/drain` at boot:39/49 and mayor:205; gastownhall#1768 fixes the witness `gc session peek` positional at witness:188). - [x] `make lint` — 0 issues. - [x] `make vet` — clean. - [x] `go test ./examples/gastown/...` — only the pre-existing `TestReaperScopesIssueAutoCloseToCityBeadsDir` failure (documented in the EXPECT-FAIL list below; not caused by this change — fixed by open PR gastownhall#1759). - [ ] `make check` — full suite not run. Prose-only one-line edit to a pack prompt template; no Go code is exercised by the diff. lint-pack is the static check covering this surface. - [ ] `make check-docs` — not applicable. - [ ] `make test-integration` — not run (per cities CLAUDE.md, deferred while the suite times out without saving partial output). **Pre-existing macOS test failures** (per `bd list --label=expect-fail --status=open,in_progress` in cities): - `TestBuiltinDoltDoctorBoundsVersionProbe` — fixed by gastownhall#1729 - `TestBuiltinDoltDoctorReportsTimedOutVersionProbe` — fixed by gastownhall#1729 - `TestExecCommandRunnerTimeoutKillsChildProcess` — fixed by gastownhall#1729 - `TestReaperScopesIssueAutoCloseToCityBeadsDir` — fixed by gastownhall#1759 - `TestCityRuntimeManualReloadPanicAfterReloadKeepsReloadReplyAndClears` — fixed by gastownhall#1741 - `TestStartLongSocketPathUsesShortSocketName` — no fix-PR yet (flakes under make check parallel load on macOS) Committed with `--no-verify` because the macOS pre-commit hook is currently blocked by these pre-existing failures until gastownhall#1729 lands. ## Checklist - [x] Linked an issue, or explained why one is not needed — internal bead `stg-xin` (cities) - [ ] Added or updated tests for behavior changes — not applicable (one-line prompt-template fix; no Go behavior changed; the static lint-pack check catches this bug class) - [x] Updated docs for user-facing changes — the prompt template *is* the user-facing artifact - [ ] Called out breaking changes or migration notes — none (the broken syntax was already failing at runtime) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1769"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hall#1758) ## Summary Each `workerObserveNudgeTarget` call (the supervisor's nudge poll loop) loaded the same session bead five times — `bd show <id>` CLI forks on real Dolt-backed cities, so the cost lands directly in idle-city CPU and Dolt connection rate. Four surgical fixes collapse the cascade so that one Observe issues at most two store fetches (resolve + a freshness re-load in `LiveObservation`); deeper consolidation needs a handle-level info cache with a staleness contract, which is out of scope for this change. The cascade was: 1. `ResolveSessionIDByExactID` → `store.Get(target)` 2. `factory.SessionByID` → `manager.Get(id)` → `store.Get(id)` 3. `factory.SessionByID` → `f.store.Get(id)` (for `Metadata`) 4. `worker.ObserveHandle` → `LiveObservation` → `manager.Get(id)` → `store.Get(id)` 5. `manager.ObserveRuntime(id, …)` → `manager.Get(id)` → `store.Get(id)` The four commits, smallest first: - **perf(nudge): reuse poll-loop observation in idle check** — the poll loop already observed the target once per iteration; threading that observation into `tryDeliverQueuedNudgesByPoller` removes a second full cascade per iteration on the running-and-delivering path. - **perf(session): split `ObserveRuntime` so callers can reuse loaded Info** — `LiveObservation` already loaded `Info` immediately before calling `ObserveRuntime`, which loaded it again. New `ObserveRuntimeForInfo(info, processNames)` takes the pre-loaded Info; `ObserveRuntime` had no other callers in the repo, so it goes. - **perf(worker): fold `factory.SessionByID`'s redundant `store.Get`** — `Manager.GetWithBead` returns Info and the loaded bead in a single fetch; `SessionByID` reads metadata from the bead it already has. - **perf(session): pass loaded bead through resolve+factory on observe path** — `ResolveSessionBeadByExactID` returns the loaded bead; `factory.SessionByLoadedBead` builds the handle from it via `Manager.SessionInfoFromBead`. The gc-CLI exact-ID branch in `workerHandleForSessionTargetWithRuntimeHintsWithConfig` uses both. Followed by a regression test that pins the dedup invariant (≤ 2 fetches per Observe). Combined with the bd-side compat-migrations sentinel (separately tracked), this should drop idle-city steady-state Dolt connection rate from ~96/sec into the 10–25/sec range. The before/after numbers below are from the targeted-package tests; a live before/after capture on a fresh dolt-prof city is still pending. ## Testing - [x] `make lint` — clean - [x] `make vet` — clean - [ ] `make check` — `make test` blocked by three pre-existing macOS failures unrelated to this change, all reproducible on `origin/main`: - `TestBuiltinDoltDoctorBoundsVersionProbe` - `TestBuiltinDoltDoctorReportsTimedOutVersionProbe` - `TestExecCommandRunnerTimeoutKillsChildProcess` - `TestReaperScopesIssueAutoCloseToCityBeadsDir` (added in 6dd911f earlier today; fails on macOS due to `/var` ↔ `/private/var` symlink resolution in `t.TempDir()` vs shell `pwd`). The first three are tracked by gastownhall#1729 (open). The fourth is new. Targeted package runs of `cmd/gc`, `internal/worker`, `internal/session`, `internal/api` all pass; the new `TestWorkerObserveSessionTargetWithConfigDoesNotFetchSessionBeadMoreThanTwice` pins the dedup invariant. - [ ] `make check-docs` — N/A (no docs touched) - [ ] `make test-integration` — not run (timeout pattern documented in maintenance-fixer stance). ## Checklist - [x] Linked an issue, or explained why one is not needed — no public issue; tracked in our local bead system as parent stg-373 and task stg-1bs (with originating observation stg-w8r). - [x] Added or updated tests for behavior changes — `TestWorkerObserveSessionTargetWithConfigDoesNotFetchSessionBeadMoreThanTwice` plus existing `TestPollerSessionIdleEnough*` and `TestTryDeliverQueuedNudgesByPoller*` updated for the new observation-passing signatures. - [ ] Updated docs for user-facing changes — no user-facing change. - [ ] Called out breaking changes or migration notes — `Manager`'s `ObserveRuntime` is removed (had no callers in the repo); `ResolveSessionIDByExactID` keeps its signature and is now a shim over `ResolveSessionBeadByExactID`. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1758"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer -->
…ts (gastownhall#1632) ## Summary `internal/extmsg/types.go::PublishRequest` was missing JSON struct tags. When `HTTPAdapter.Publish` marshaled it to send to an out-of-process adapter, Go's `encoding/json` fell back to PascalCase. Adapters parse with explicit snake_case tags, and Go's case-insensitive matching does NOT bridge the underscore boundary, so `ReplyToMessageID`, `IdempotencyKey`, and `Metadata` were silently dropped on the wire. A parallel sibling bug existed on the receipt path: `PublishReceipt` was also untagged, and `HTTPAdapter.Publish` decoded adapter responses into it directly. Adapter responses use snake_case keys, so `MessageID` and `FailureKind` were silently zeroed on receive — which broke threaded replies because gc had no provider message ID to chain on. Discovered live during the Slack pack oversight-rig PL room reply-threading session: the supervisor sent `ReplyToMessageID:"…"`, the adapter received `reply_to_message_id:""`. ## Why a wire shim instead of just tagging the domain types Initial pass tagged `PublishReceipt` and `AdapterCapabilities` directly. A second-opinion review caught that **both types are exposed via the Huma API**: - `AdapterCapabilities` is the body of `POST /extmsg/adapters` - `PublishReceipt` is `OutboundResult.Receipt`, returned by `POST /extmsg/outbound` The checked-in `internal/api/openapi.json`, `docs/schema/openapi.json`, generated Go genclient (`internal/api/genclient/client_gen.go`), and dashboard TS types (`cmd/gc/dashboard/web/src/generated/{schema.d.ts,types.gen.ts}`) all advertise PascalCase keys for these types. Tagging the domain types would silently flip the live API contract. This PR keeps the Huma API contract unchanged and bridges the gc↔adapter HTTP wire via an explicit shim — aligning with AGENTS.md's *"Keep Serialization/Deserialization At The Edges"*. ## What changed - **`internal/extmsg/types.go`** - `PublishRequest`: tagged with snake_case (no Huma surface — verified, only test refs in `internal/api/handler_extmsg_test.go`). - `PublishReceipt`: kept untagged with an explanatory doc comment. The gc↔adapter wire crossing is bridged elsewhere. - `AdapterCapabilities`: kept untagged with an explanatory doc comment. Does not cross the adapter callback wire. - **`internal/extmsg/http_adapter.go`** - New unexported `wirePublishReceipt` with explicit snake_case tags. - `Publish()` now decodes the adapter response into the wire type, then converts via `wire.toPublishReceipt()` to the domain type. - **`internal/extmsg/types_wire_test.go`** (new) - `TestPublishRequestSnakeCaseWire` — pins outbound marshal of `PublishRequest`. Asserts each underscore-bearing key is present and bans PascalCase. - `TestWirePublishReceiptDecodesSnakeCase` — pins decode of an adapter response via the wire shim. Uses non-zero distinguishable values for every previously-silent-drop field (`message_id`, `failure_kind: "rate_limited"`, `retry_after: 5s`, `metadata`), so a regressed tag fails the test instead of silently round-tripping the same zero. - `TestPublishReceiptStaysPascalCaseOnAPISurface` — guards the Huma API contract: `json.Marshal(PublishReceipt{...})` must still produce `"MessageID"`, `"FailureKind"`, `"RetryAfter"`. Protects against a future *"fix the bug at the type level"* attempt that would silently break every API client. ## Sibling-type audit | Type | Status | |---|---| | `PublishRequest` | was BROKEN → FIXED with tags | | `PublishReceipt` | was BROKEN → FIXED via wire shim | | `AdapterCapabilities` | Huma-surfaced, intentionally untagged | | `EnsureChildConversation` | safe — uses explicit `map[string]any` with snake_case keys; decodes into already-tagged `ConversationRef` | | `ExternalInboundMessage` | already correctly tagged | ## Test plan - [x] `go test ./internal/extmsg/... -race` clean - [x] Stash-revert verification: tests build-fail without `wirePublishReceipt`, confirming the shim is load-bearing - [x] `go vet ./...`, `make lint`, `go build ./...`, `make check`, `make check-docs` all clean - [x] `make dashboard-check` clean — **no OpenAPI / TS client drift**, by design - [x] `make generate` produces no working-tree drift ## Out of scope - Migrating the Huma API surface from PascalCase to snake_case. That's a coordinated API change with regenerated clients, not a side effect of this fix. Co-authored-by: sjarmak <sjarmak@users.noreply.github.com>
…hall#1741) ## Summary `TestCityRuntimeManualReloadPanicAfterReloadKeepsReloadReplyAndClears` flakes ~10% on macOS with: ``` testing.go:1464: TempDir RemoveAll cleanup: unlinkat .../001/.gc: directory not empty gc: order exec ... failed: signal: killed (×N) ``` Root cause: `cr.tick(context.Background(), ...)` dispatches order subprocesses as goroutines under a non-cancellable ctx. The test returns; `t.Cleanup(cr.shutdown)` runs; shutdown's drain only **waits** for in-flight goroutines (1s budget) — it does not cancel them. When drain times out, shutdown returns; `t.TempDir`'s `RemoveAll` then races subprocesses still holding `.gc/beads.json.lock` and materialised pack scripts open. The late SIGKILL produces the `signal: killed` lines. The bug is test-environment-specific: production has no equivalent RemoveAll race, and the existing design intent (per the doc comment on `memoryOrderDispatcher.drain`) explicitly allows orders to outlive drain — orphaned tracking beads are reaped on the next boot by `sweepOrphanedOrderTrackingRetry`. This change preserves production semantics. ## What changed `memoryOrderDispatcher` gains two private fields (`dispatchCtx`, `dispatchCancel`) initialised in the existing factory functions, plus two private methods: `launchDispatchOne` (wraps each goroutine's ctx via `context.AfterFunc` so the dispatcher's internal ctx propagates cancellation alongside the caller's tick ctx) and `cancel`. The `orderDispatcher` interface and production shutdown path are **unchanged** — `cancel()` is reachable only by type-asserting to the concrete dispatcher, which `newTestCityRuntime`'s cleanup hook does before invoking `cr.shutdown`. Test fakes have no in-flight goroutines and don't need to model cancellation. Also removes a redundant `t.Cleanup(cr.shutdown)` in `TestCityRuntimeManualReloadReplyWaitsForTickCompletion` that consumed `cr.shutdownOnce` (LIFO) before the new wrapper's cancel could fire. ## Testing - [x] Target test 100/100 pass after the fix (was 2/20 ~10% on `origin/main`) - [x] Three sibling tests in the same family — 100/100 pass each - [x] `TestOrderDispatcherCancelTerminatesInFlight` regression — 10/10 pass under `-race` - [x] `go test ./cmd/gc/...` clean - [x] `go vet ./...` clean - [ ] `make check` — pre-commit hook reports 4 pre-existing lint issues on `origin/main` (`cmd/gc/cmd_status.go:97-98`, `cmd/gc/city_status_snapshot_test.go:205`, `cmd/gc/providers_test.go:729`); commit pushed with `--no-verify` for that reason — same set blocking gastownhall#1729 - [x] `make check-docs` - [ ] `make test-integration` ## Pre-existing issues observed (not introduced) Running `go test -race ./cmd/gc/...` against the affected tests surfaces a pre-existing data race in the dispatch path — parallel `dispatchOne` goroutines write to shared `m.stderr` without synchronization. Reproduces identically on bare `origin/main`. Not addressed here; out of scope for this fix. `TestBuiltinDoltDoctor*VersionProbe`, `TestExecCommandRunnerTimeoutKillsChildProcess`, and `TestMaintenanceDoltScriptsSkipTestPatternDatabases` fail on bare main on macOS (gtimeout/Homebrew PATH leak) — addressed by gastownhall#1729 / gastownhall#1706. ## Checklist - [x] Linked an issue, or explained why one is not needed — fixes the flake described above - [x] Added or updated tests for behavior changes — `TestOrderDispatcherCancelTerminatesInFlight` - [ ] Updated docs for user-facing changes — N/A (test-only fix; no public API changes) - [ ] Called out breaking changes or migration notes — N/A (interface unchanged) <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1741"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ownhall#1753) ## Summary - Replace `gc session peek <target> 50` with `gc session peek <target> --lines 50` in `examples/gastown/packs/maintenance/agents/dog/prompt.template.md` line 79. - Same bug class as the boot/deacon fixes in gastownhall#1743 / gastownhall#1744: the line-count is now a flag, not a positional. Without `--lines`, runtime fails with `accepts 1 arg(s), received 2`. - Dog agents peek at target sessions to assess state during warrant processing and shutdown dances; every dog warrant currently burns a tool call on this error before falling back to `--help` discovery. - Filed separately from gastownhall#1743 / gastownhall#1744 because this prompt lives in a different pack file (`packs/maintenance/...` vs `packs/gastown/...`) and bundling would have stretched those PRs' titles. Verified the new syntax against `gc session peek --help`: ``` Flags: -h, --help help for peek --lines int number of lines to capture (default 50) ``` ## Testing - [x] `make check` — pre-existing macOS test failures (none caused by this change; verified two passing on main with this edit stashed): - `TestBuiltinDoltDoctorBoundsVersionProbe` (documented in cities CLAUDE.md) - `TestBuiltinDoltDoctorReportsTimedOutVersionProbe` (documented in cities CLAUDE.md) - `TestExecCommandRunnerTimeoutKillsChildProcess` (documented in cities CLAUDE.md) - `TestCityRuntimeManualReloadPanicAfterReloadKeepsReloadReplyAndClears` — ~10% macOS flake fixed by my open gastownhall#1741 (commit 564cb96, *cancel dispatched orders before t.TempDir cleanup*) - `TestReaperScopesIssueAutoCloseToCityBeadsDir` — macOS `/var → /private/var` path-resolution flake; pre-existing, not yet documented (will file follow-up) - [ ] `make check-docs` — not applicable (no docs/ changes) - [ ] `make test-integration` — not run (per cities CLAUDE.md, deferred while the suite times out without saving partial output) ## Checklist - [x] Linked an issue, or explained why one is not needed — internal bead stg-7n9 (cities) - [ ] Added or updated tests for behavior changes — not applicable (one-line prompt-template fix, same class as gastownhall#1743 / gastownhall#1744 which also added no tests) - [x] Updated docs for user-facing changes — the prompt template *is* the user-facing artifact - [ ] Called out breaking changes or migration notes — none (the broken syntax was already failing at runtime) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1753"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…astownhall#1768) ## Summary - Replace `gc session peek <target> 50` with `gc session peek <target> --lines 50` in `examples/gastown/packs/gastown/agents/witness/prompt.template.md` line 188. - Same bug class as the boot/deacon/dog fixes in gastownhall#1743 / gastownhall#1744 / gastownhall#1753: the line-count is now a flag, not a positional. Without `--lines`, runtime fails with `accepts 1 arg(s), received 2`. - The witness agent runs `gc session peek` to inspect polecat output during patrol; every witness invocation currently burns a tool call on this error before falling back to `--help` discovery. - Filed separately because the witness prompt was missed when the boot/deacon/dog fixes went out — neither bundled into those PRs nor pre-emptively listed anywhere; surfaced when the new pack-aware command-syntax linter (cities `test-series/lint_pack/`) ran against `examples/gastown/` on main. Verified the new syntax against `gc session peek --help`: ``` Flags: -h, --help help for peek --lines int number of lines to capture (default 50) ``` ## Testing - [x] `~/cities/test-series/bin/lint-pack ~/wrk/gascity/examples/gastown/ --check command-syntax` — 6 errors before fix, 5 after; the witness:188 violation is the one removed. Remaining 5 are covered by other open PRs (gastownhall#1743 fixes the `gc agent peek/list/drain` errors at boot:39/49 and mayor:205) or other open beads (the boot:55 `gc mail inbox --address/--json` flag errors are tracked separately). - [x] `make lint` — 0 issues. - [x] `make vet` — clean. - [x] `go test ./examples/gastown/...` — only the pre-existing `TestReaperScopesIssueAutoCloseToCityBeadsDir` failure (documented in the EXPECT-FAIL list below; not caused by this change — it's a `/var → /private/var` symlink resolution issue fixed by open PR gastownhall#1759). - [ ] `make check` — full suite not run. The change is a prose-only one-line edit to a pack prompt template; no Go code is exercised by the diff. lint-pack is the static check that covers the surface of this change, and `go test ./examples/gastown/...` exercises the embedding/loading path. - [ ] `make check-docs` — not applicable (no `docs/` changes). - [ ] `make test-integration` — not run (per cities CLAUDE.md, deferred while the suite times out without saving partial output). **Pre-existing macOS test failures** (per `bd list --label=expect-fail --status=open,in_progress` in cities): - `TestBuiltinDoltDoctorBoundsVersionProbe` — fixed by gastownhall#1729 - `TestBuiltinDoltDoctorReportsTimedOutVersionProbe` — fixed by gastownhall#1729 - `TestExecCommandRunnerTimeoutKillsChildProcess` — fixed by gastownhall#1729 - `TestReaperScopesIssueAutoCloseToCityBeadsDir` — fixed by gastownhall#1759 - `TestCityRuntimeManualReloadPanicAfterReloadKeepsReloadReplyAndClears` — fixed by gastownhall#1741 - `TestStartLongSocketPathUsesShortSocketName` — no fix-PR yet (flakes under make check parallel load on macOS) Committed with `--no-verify` because the macOS pre-commit hook is currently blocked by these pre-existing failures until gastownhall#1729 lands. ## Checklist - [x] Linked an issue, or explained why one is not needed — internal bead `stg-lls` (cities) - [ ] Added or updated tests for behavior changes — not applicable (one-line prompt-template fix, same class as gastownhall#1743 / gastownhall#1744 / gastownhall#1753 which also added no tests; the static lint-pack check catches this bug class) - [x] Updated docs for user-facing changes — the prompt template *is* the user-facing artifact - [ ] Called out breaking changes or migration notes — none (the broken syntax was already failing at runtime) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1768"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…(ga-2z81) (gastownhall#1738) ## Summary The `loads older transcript pages without losing the drawer loading sentinel` test in `cmd/gc/dashboard/web/src/panels/crew.test.ts` fails intermittently on Blacksmith's `Dashboard SPA` job — about half of recent runs fail, half pass, on the same code. ## Root cause The local `waitFor` helper in this test polls for up to **1000 ms**. On Blacksmith CI the entire `openLogDrawer` → `loadTranscript` chain has been observed to take **~1.3 s** on slow runs (vs ~100 ms on fast ones — same VM class, same code). When the runner is in the slow tail, the chain ultimately completes — the CI log shows the `[crew] Transcript loaded` debug message firing **after** the assertion has already timed out — but the test has already given up. The bead's original hypothesis (`PR gastownhall#1698 introduced a regression in crew code`) is not the cause: PR gastownhall#1698 only touched auto-generated dashboard SDK files (`schema.d.ts`, `sdk.gen.ts`); it did not modify `crew.ts` or its helpers. ## Evidence - Failing run: [job 74521385425](https://github.com/gastownhall/gascity/actions/runs/25407163608/job/74521385425) — crew.test.ts took **1339 ms**, second test failed at the waitFor. - Passing run on the same workflow ~50 minutes later: [job 74525787419](https://github.com/gastownhall/gascity/actions/runs/25408509735/job/74525787419) — crew.test.ts took **107 ms**. - Local: 60+ runs (single-test, full-suite, with and without isolation, under CPU load) — all pass, ~50ms each. ## Fix Bump the local `waitFor` ceiling from 1000 ms → 5000 ms with a comment explaining the observed CI variance. The new ceiling absorbs the slow tail without changing the test contract, the mocked API, or any production code. Locally the cost stays in the tens of milliseconds because waitFor still returns on the first successful poll. ## Test plan - [x] `npm test` (vitest 22/22 passes locally) - [x] `npm run typecheck` (clean) - [ ] Confirm the Blacksmith `Dashboard SPA` job is green on this PR - [ ] Re-run a few times (the failure is flaky — one passing run is necessary, several are reassuring) <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1738"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> Co-authored-by: Claude Code (gascity/builder) <jim@wordelman.name>
…stownhall#1743) ## Summary The `gc agent` subcommand tree dropped peek/list/kill/etc when those runtime operations moved to `gc session` and `gc runtime` (commit d3210cd). Several pack prompts still reference the old names, so fresh agent sessions burn 3–5 tool calls per tick rediscovering the CLI shape via `--help` — measured at 21+ "unknown subcommand" errors per 10 minutes of normal boot ticks in real city transcripts. This patch sweeps the remaining stale references in pack prompts: - `gc agent peek <name>` → `gc session peek <name>` (boot prompt, 3 sites) - `gc agent list` → `gc session list` (boot + mayor prompts, 2 sites) - `gc agent drain <name>` → `gc runtime drain <name>` (mayor + crew templates, 2 sites). The deprecation shim in `cmd/gc/cmd_agent.go` points users at `gc runtime drain`, which preserves the original graceful-drain semantics. - swarm/deacon: drop the "via `gc agent restart`" parenthetical. That command never existed; the reconciler auto-restarts dead sessions, so the bullet now reads `Note crashed agents — the reconciler auto-restarts dead sessions.` Pure template-prose edits; no code changes. ## Testing - [x] `make check` clean for the changes in this PR. Pre-commit hook (`go test ./test/docsync`) passed. Full `go test ./...` shows the documented pre-existing macOS failures (`TestBuiltinDoltDoctor*`, `TestExecCommandRunnerTimeoutKillsChildProcess`, `TestMaintenanceDoltScriptsSkipTestPatternDatabases`) which exist on plain `origin/main` and are unrelated to this prose-only change. - [ ] `make check-docs` (no docs/navigation/link changes — prompt prose only) - [ ] `make test-integration` (no runtime/controller/workflow behavior changes; deferred — past attempts have timed out on macOS without saving partial output) ## Checklist - [ ] Linked an issue, or explained why one is not needed - Tracked locally; symptom + repro recorded in transcript scans (21+ unknown-subcommand errors/10min). - [ ] Added or updated tests for behavior changes (no behavior change — prose-only) - [ ] Updated docs for user-facing changes (these *are* the docs being updated) - [x] Called out breaking changes or migration notes — none; the deprecation shims in `cmd/gc/cmd_agent.go` already cover existing references.
…hall#1526) (gastownhall#1688) ## Summary `gc mail thread <id>` returned `No messages in thread` for both the parent and reply IDs even immediately after a successful `gc mail reply` between two known beads — exactly the reproducer in gastownhall#1526. Closes gastownhall#1526. ## Root cause `Send` and `Reply` correctly persist a `thread:<uuid>` label on every message bead, and `Reply` additionally writes `reply-to:<parentID>`. The bug is purely in how `Provider.Thread` interprets its argument. `cmd/gc/cmd_mail.go:1624` passes the user's CLI argument straight through: \`\`\`go threadID := args[0] msgs, err := mp.Thread(threadID) \`\`\` But `<id>` is a **message bead-ID** (e.g., `mg-wisp-pn9`), not a thread-id. `Provider.Thread` was querying `Label: \"thread:\" + msgID`, while the bead's actual label is `thread:<generated-uuid>` — so the query always returned the empty list. ## Fix `internal/mail/beadmail/beadmail.go` — resolve the input via `store.Get` and use the bead's `thread:` label as the query key. Backward-compatible: callers that already know the thread-id (e.g., the existing `TestThread`, internal callers) hit the Get-not-found fallback and use the input directly. \`\`\`go func (p *Provider) Thread(id string) ([]mail.Message, error) { threadID := id if msgBead, err := p.store.Get(id); err == nil { if t := extractLabel(msgBead.Labels, \"thread:\"); t != \"\" { threadID = t } } bs, err := p.store.List(beads.ListQuery{ Label: \"thread:\" + threadID, Type: \"message\", Sort: beads.SortCreatedAsc, }) ... } \`\`\` The fix is intentionally scoped to the data layer — no overlap with the in-flight gastownhall#1149 read-path routing rewrite (which modifies \`cmd/gc/cmd_mail.go\` and \`internal/api/decode_mail.go\`). When gastownhall#1149 lands and reroutes \`gc mail thread\` through the supervisor API, the same correct \`Provider.Thread\` will be invoked behind the new transport. ## Tests \`internal/mail/beadmail/beadmail_test.go\` — two new tests reproducing the issue, plus pre-existing tests preserved as regression cover: | Test | What it locks in | |---|---| | **NEW** \`TestThreadAcceptsMessageIDOfOriginal\` | \`Send\` → \`Reply\` → \`Thread(parent.ID)\` returns [parent, reply] | | **NEW** \`TestThreadAcceptsMessageIDOfReply\` | Same, but \`Thread(reply.ID)\` — symmetric path | | existing \`TestThread\` | \`Thread(sent.ThreadID)\` (actual thread-id) still works — backward-compat regression cover | | existing \`TestThreadEmpty\` | \`Thread(\"nonexistent\")\` returns empty without error | ## Test plan - [x] \`go test ./internal/mail/...\` — green - [x] \`go vet ./internal/mail/...\` — clean - [x] \`make check\` (fmt-check + lint + vet + test) — green - [ ] CI green - [ ] Manual reproducer per gastownhall#1526 acceptance criteria: 1. \`gc mail send mayor -s \"T\" -m \"ping\"\` → note id A 2. \`gc mail reply A -s \"Re: T\" -m \"pong\"\` → note id B 3. \`gc mail thread A\` returns both A and B in chronological order 4. \`gc mail thread B\` same <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1688"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> --------- Co-authored-by: sjarmak <sjarmak@users.noreply.github.com> Co-authored-by: Julian Knutsen <julianknutsen@users.noreply.github.com>
## Summary - keep active sessions with concrete assigned work out of new pool-demand capacity accounting - skip assigned session beads when materializing new-tier pool demand - add regression coverage for assigned sessions plus fresh ready demand producing total desired capacity ## Tests - go test ./cmd/gc -run 'TestSelectOrCreatePoolSessionBead_SkipsAssignedForNewTier|TestComputePoolDesiredStates_AssignedSessionsDoNotConsumeNewDemand|TestScaled_NewDemandDoesNotUseActiveAssignedSessions|TestComputePoolDesiredStates_InFlightNewSessionsConsumeScaleDemand|TestSelectOrCreatePoolSessionBead_ReusesAvailableForNewTier'\n- go test ./cmd/gc -run 'TestComputePoolDesiredStates_|TestScaled_|TestSelectOrCreatePoolSessionBead_'\n- pre-commit hook: GC_FAST_UNIT=1 scripts/go-test-observable test -- -p=4 -count=1 ./...\n <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1777"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer -->
…cross-rig-deps (gastownhall#1708) ## Summary Both `wisp-compact.sh` and `cross-rig-deps.sh` piped `jq` output through `|` into a `while read` loop. The pipe puts the loop body in a subshell, so `PROMOTED`/`DELETED`/`SKIPPED` (wisp-compact) and `RESOLVED` (cross-rig-deps) were lost when the subshell exited. The summary `echo` at the end of each script is gated on those counters being non-zero, so under any normal load both scripts ran silently — silent broken telemetry on every maintenance run, even though the side-effect `bd` calls inside the loop (`delete`, `--persistent`, `dep remove`/`add`) still fired correctly. `cross-rig-deps.sh` had the anti-pattern at both the outer loop (over closed beads) and the nested inner loop (over external deps); both needed fixing because the inner pipe also discarded `RESOLVED`. Follow-up to the body-coverage review on gastownhall#1662 — that PR added tests for these two scripts but didn't assert on counter values, which is why the bug went undetected. ## Approach Capture `jq` output into a variable first, then loop in the parent shell via a here-string: ```bash LINES=$(producer) # set -e + pipefail still catches jq failure while ... done <<< "\$LINES" ``` This both: 1. Preserves the original fail-loud-on-jq-failure behavior under `set -euo pipefail`. Process substitution (`< <(producer)`) was considered first but silently swallows producer exit codes, which would have been a regression vs. the original pipeline form where `pipefail` propagated `jq`'s non-zero exit. 2. Keeps the loop body in the parent shell so the counters survive to the summary echo. The `cross-rig-deps` inner loop's `select(...)` filter may legitimately yield zero matches (closed beads with only-internal deps). `<<< ""` would otherwise produce one bogus iteration with `dep_id=""`, so an explicit `if [ -z "\$EXTERNAL_DEPS" ]; then continue; fi` guards that case. ## Test coverage New body-level tests in `examples/gastown/maintenance_scripts_test.go`: - `TestWispCompactReportsNonZeroCounters` — drives the script with 2 closed-past-TTL + 1 open-past-TTL + 1 within-TTL ephemerals; asserts the exact summary line `wisp-compact: promoted=1 deleted=2 skipped=1`. - `TestCrossRigDepsReportsNonZeroCounter` — drives the script with 2 closed beads × 2 external deps each (`RESOLVED`=4) plus 1 closed bead with only-internal deps (`RESOLVED` unchanged); asserts the exact summary `cross-rig-deps: resolved 4 cross-rig dependencies` AND the absence of any `bd dep remove ""` call (locks in the empty-filter guard). Both tests fail on the pre-fix scripts (side effects observed in the bd stub log, summary echo missing) and pass after the fix. ## Test plan - [x] `make build` — clean - [x] `make lint` (golangci-lint with revive) — 0 issues - [x] `make fmt-check` — clean - [x] `go vet ./...` — clean - [x] `make test` — 0 failures across all packages - [x] Verified RED→GREEN: reverting the script changes makes both new tests fail with the targeted assertion message - [x] Verified empty-filter guard: removing `if [ -z "\$EXTERNAL_DEPS" ]` makes the test fail with `dep remove ""` observed in the bd log - [x] Sibling sweep across all 9 maintenance scripts: only `wisp-compact.sh` and `cross-rig-deps.sh` had the buggy `echo | jq | while + counter` pattern; others use `<<<` here-strings, `for`, or `\$()` capture which are all parent-shell-safe <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1708"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> Co-authored-by: sjarmak <sjarmak@users.noreply.github.com>
…ll#1728) ## Summary Closes the regression-coverage Done-when item for **ga-t0pm**. The spec'd snapshot/wait/doctor changes (Changes 1A, 1B, 2) already landed in main: - 1A + 1B in `d4046d75` (perf(reconciler): avoid broad tick-time store scans) - 2 in gastownhall#1672 (`a0323e50`, Fix gc status and doctor hangs) This PR adds the missing benchmark to lock in the perf invariant. ## What `cmd/gc/session_bead_snapshot_test.go`: - `BenchmarkLoadSessionBeadSnapshot_LargeStore` — 50 open / 5000 closed. - `BenchmarkLoadSessionBeadSnapshot_OpenOnlyBaseline` — 50 open / 0 closed (control). Both should report identical alloc shape (181 allocs, ~105KB) since the snapshot only retains open beads. A future re-introduction of `IncludeClosed: true` would balloon both ns/op and alloc count at LargeStore (snapshot would be built over 5050 beads instead of 50). Local results on AMD Ryzen AI MAX+ 395: ``` BenchmarkLoadSessionBeadSnapshot_LargeStore-32 ~93us/op 105KB 181 allocs BenchmarkLoadSessionBeadSnapshot_OpenOnlyBaseline-32 ~46us/op 105KB 181 allocs ``` The ~2x ns/op delta is MemStore's O(N) iteration overhead. Production dolt filters via the labels+status indexes, so real-world LargeStore should be closer to baseline. ## Test plan - [x] `go vet ./cmd/gc/...` clean - [x] `go test -run 'Test.*Snapshot|Test.*WaitWake|Test.*SessionModel|TestCloseBead' ./cmd/gc/` passes - [x] `go test -bench='BenchmarkLoadSessionBeadSnapshot' -benchmem ./cmd/gc/` runs both benchmarks Bead: ga-t0pm Source design: ga-7pjf 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1728"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> Co-authored-by: Claude Code (gascity/builder) <jim@wordelman.name> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary - prevent supervisor activity SSE from replaying old event backlog from partial history cursors - hide/reset city-scoped panels when a stopped or unknown city is selected - separate terminal attachment state from the selected city scope label ## Verification - PASS: npm run test -- --reporter=verbose - PASS: make dashboard-check - PASS: go vet ./... - PASS: make test - PASS: targeted rerun of packages/tests that failed under the over-parallel sharded runner - PASS: browser automation checked supervisor, running city, bead detail, stopped city, pop-up errors, and web console errors ## Note - make test-fast-parallel failed under high local parallelism with subprocess timeout/starvation symptoms; the same failing packages/tests passed on serial targeted rerun, and the official capped make test baseline passed. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/gastownhall/codesmith/gascity/pr/1616"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer --> --------- Co-authored-by: Julian Knutsen <julianknutsen@users.noreply.github.com>
Use workerHandleForNudgeTarget with NudgeWakeLiveOnly for delivery confirmation, matching the poller path. Only ack nudges when result.Delivered is true to prevent silent drops when a session stops between the IsRunning pre-check and the actual delivery. Prefer InstanceName over TemplateName for identity resolution, matching the poller's concrete-identity-first ordering. Record telemetry via telemetry.RecordNudge on both success and failure, matching the poller's observability instrumentation.
- workerHandleForNudgeTarget: record failure + continue instead of returning early, so remaining targets still get processed and claimed nudges are not left in limbo - DeliversDueNudge: assert Nudge call was made with correct message - SkipsNotYetDue: assert queue state unchanged and no Nudge calls - NoTargets: seed a due nudge to prove queue is untouched - SessionNotRunning: assert no Nudge calls made - NoSessionBead: assert identity fields (sessionName, alias, transport) - ResolveAgentForNudge: add alias-fallback positive case - SessionStopsBeforeDelivery: assert stoppingProvider triggered
…WatchEvent Wait for a reconcile tick after config reload before asserting lastAgentNames, so buildFn has run with the new config.
tesdal
force-pushed
the
fix/acp-nudge-reconciler-drain
branch
from
ac4ad48 to
600bc45
Compare
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
![https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source](https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source){kind=link}
![https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img](https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img){kind=link}
![https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a](https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a){kind=link}
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
gc nudge poll) explicitly skips ACP sessions because it spawns a separate process without in-process ACP connectionsdrainACPQueuedNudgesto the reconciler tick, mirroring the full poller pipeline:claimDueQueuedNudgesForTarget(session-fenced claiming),splitQueuedNudgesForTarget(fence-mismatch rejection),splitQueuedNudgesForDelivery(wait-bead blocking),formatNudgeRuntimeMessage(batched delivery),ackQueuedNudges/recordQueuedNudgeFailure(lifecycle)buildACPNudgeTargetsto construct properly fencednudgeTargetvalues from reconciler state (DesiredStateResult + session bead metadata)Stacked on
PR #1 (
fix/opencode-acp-command) — ACP session startup fixesFiles
cmd/gc/nudge_acp_drain.go—drainACPQueuedNudges,buildACPNudgeTargets,resolveAgentForNudge,aliasHistoryFromBeadcmd/gc/nudge_acp_drain_test.go— 10 testscmd/gc/city_runtime.go— wired drain call afterdispatchReadyWaitNudgesinbeadReconcileTick