[phinze]

Listing sandbox pools takes about 10 seconds on a cluster with a few hundred app versions. Most of that is the released CLI, which resolves every app version's config from the client just to fill in the MODE column. Main already fixed the big part by moving the work behind the SandboxPools RPC, so the next release gets it for free. That path still made three store calls per pool in sequence, though, which takes it to about 0.8s against the same cluster.

This trims the server side. Each version is now resolved once, no matter how many pools share it. Scaling mode now comes from the stored config, since addon bindings only add variables and disks. That drops a round trip per pool, and inventory no longer loads addon credentials it was never going to show.

debug sandbox-pool list also gets --app and --service filters, matching sandbox list, plus an APP column. Addon pools have no owner field, only an app label, so they used to show up blank. The RPC now reports that label as their app and its app filter matches the reported name, so --app shop includes shop's database pool. The label is read after authorization and grants nothing. --service filters client-side.

Adding flags broke the pool CLI tests even though nothing they check changed, because they restated each command's option struct, tags and all. They now go through RunCommand with real flags. MIR-2193 tracks the same cleanup in the other test files.

Closes MIR-2191
Closes MIR-1361

[evanphx]

Nice find

[miren-code-agent]

🍪 biscuit: ✅ ready to merge — auto-review, non-blocking

Approval recommended, not submitted: this repository is not enabled for autonomous approvals. This review is a comment, not a GitHub approval.

Nothing of substance has changed since my last review, and I still recommend approval. The branch is now a single commit, f381862, with the same content I reviewed at bd6087f. In servers/sandbox/pools.go:

• filterInventoryApp still runs against each pool's real owner before the app label is read.
• The label is used only to display ownerless pools and to match them against the app filter.
• poolVersion reads concurrency through coreutil.ResolveConfig. That's safe for MODE because ResolveAddonConfig only touches Variables and service Disks.
• TestPoolInventoryAddonPoolApp still pins the deny case for app-bound callers, and TestSandboxPoolListFilters still runs an addon pool through the real CLI flags.

There are no new comments or threads, and evanphx has approved this head.

Who sees addon pools under --app shop. Only cluster admins see shop's database pool there. A caller who owns shop but isn't a cluster admin still won't see it, because ownerless pools need admin rights. That matches what this code did before, so it isn't a regression. I'm mentioning it in case someone reads the description as "app owners now see their addon pools."

Older servers. On servers without the RPC, the legacy fallback filters --app client-side against pool.App(). That path never reads the label, so addon pools there still show - and drop out of a filtered listing. This only affects servers that predate the RPC, so I don't think it needs to block.

Why I'd skip another human pass: the only security-relevant change is moving authorization from a single upfront check to a per-pool check, and that check still runs before any label is read. A test covers the case where an app-bound caller must be denied. Everything else is a contained CLI and inventory change, and each new behavior has its own test.

---

🍪 full review note · reviewed at f381862