Skip to content

fix: reject named_session entries referencing pool agents (#574)#654

Open
rileywhite wants to merge 1 commit intogastownhall:mainfrom
rileywhite:fix/574-named-session-pool-validation
Open

fix: reject named_session entries referencing pool agents (#574)#654
rileywhite wants to merge 1 commit intogastownhall:mainfrom
rileywhite:fix/574-named-session-pool-validation

Conversation

@rileywhite
Copy link
Copy Markdown
Contributor

@rileywhite rileywhite commented Apr 12, 2026

Summary

Adds a pool-agent check in ValidateNamedSessions (internal/config/config.go) that
rejects [[named_session]] entries where the referenced template has namepool/namepool_names
set or max_active_sessions != 1. This enforces the invariant documented in
engdocs/design/named-configured-sessions.md Section 8 at boot time.

  • max_active_sessions = 1 is explicitly allowed (singleton cap, used by gastown pack)
  • nil max (basic template, no pool fields) is accepted
  • Namepool, max > 1, max = 0, and max = -1 (unlimited) are all rejected

Closes #574
Closes #504

What was tested

  • make fmt-check — pass (0 issues)
  • make lint — pass (0 issues)
  • make vet — pass
  • go test -v -run TestValidateNamedSessions ./internal/config/ — all 8 subtests pass
  • Verified gastown pack compatibility (all 5 named-session agents have max=1, pass validation)

Review outcome

Design council (5 perspectives) identified the correct approach. Adversarial review caught a
critical bug in the first cut: max_active_sessions = 1 was incorrectly rejected as "pool",
which would have broken gastown pack configs (mayor, deacon, boot, witness, refinery all use
max=1 + named_session). What-about review confirmed the same issue from a second angle
(control-dispatcher implicit agent). Fix: changed check to allow max=1, aligning with the
runtime's isMultiSessionCfgAgent definition and the API layer's isMultiSessionAgent.

Neutral judge verdict: CLEARED.

Residual note: the codebase has three separate "is pool" definitions that don't fully agree
(config validation, runtime isMultiSessionCfgAgent, API isMultiSessionAgent). This is
intentional — config-time treats nil max as "basic template" while runtime treats it as
"unlimited = multi-session". A future cleanup could extract named predicates.

Changes

File Change
internal/config/config.go +7 lines: pool-agent guard in ValidateNamedSessions
internal/config/session_sleep_test.go +80 lines: 5 rejection + 2 acceptance test cases

No documentation changes needed — the invariant was already documented.
No breaking changes for in-tree configs.

🤖 Generated with Claude Code

@github-actions github-actions bot added the status/needs-triage Inbox — we haven't looked at it yet label Apr 12, 2026
@rileywhite rileywhite force-pushed the fix/574-named-session-pool-validation branch 2 times, most recently from 0f20e7d to c162187 Compare April 12, 2026 23:08
@rileywhite rileywhite force-pushed the fix/574-named-session-pool-validation branch from c162187 to 72ca60f Compare April 12, 2026 23:25
@rileywhite rileywhite marked this pull request as ready for review April 12, 2026 23:25
@github-actions github-actions bot removed the status/needs-triage Inbox — we haven't looked at it yet label Apr 13, 2026
@julianknutsen julianknutsen added kind/bug Broken behavior status/needs-triage Inbox — we haven't looked at it yet priority/p1 High — core workflow broken and removed status/needs-triage Inbox — we haven't looked at it yet labels Apr 13, 2026
@julianknutsen julianknutsen added this to the 1.0 milestone Apr 13, 2026
@rileywhite
Copy link
Copy Markdown
Contributor Author

Note: this PR also resolves #504 (per-agent drain policy). PR #652, which addressed #504 separately, has been closed as this is a superset of that work.

@rileywhite rileywhite force-pushed the fix/574-named-session-pool-validation branch from 72ca60f to 9cf65ac Compare April 13, 2026 11:11
…l#574)

Add pool-agent check in ValidateNamedSessions (internal/config/config.go)
that rejects named_session entries where the referenced template has
namepool/namepool_names set or max_active_sessions != 1. Enforces the
invariant from engdocs/design/named-configured-sessions.md Section 8 at
boot time.

- max_active_sessions = 1 explicitly allowed (singleton cap)
- nil max (basic template, no pool fields) accepted
- Namepool, max > 1, max = 0, max = -1 (unlimited) all rejected
- Includes per-agent drain policy (lifecycle block)

Closes gastownhall#574

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rileywhite rileywhite force-pushed the fix/574-named-session-pool-validation branch from 9cf65ac to 2df2190 Compare April 13, 2026 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Broken behavior priority/p1 High — core workflow broken session

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: config validation should reject [[named_session]] on pool agents feat: per-agent drain policy in session templates

2 participants