This is a clean, focused removal of two dead pieces: the top-level Configuration.Concurrency/AutoConcurrency fields (superseded by per-service concurrency) and the set CLI command that was the only consumer of those fields.
What I checked:
HasConcurrency, SetConcurrency, AutoConcurrency, or commands.Set anywhere in the codebase — the search came back empty across all 400 files scanned.rpc.yml is handled correctly: indices 1 and 2 are left as a gap (with an explanatory comment) rather than recycled, so old clients decoding a new payload won't misread a future field through a stale index. That's the right call for a binary-serialised protocol.rpc.gen.go matches the schema change exactly — the AutoConcurrency type and the two configurationData fields drop together cleanly.app_status.go removes both the human-readable and JSON output paths for the old concurrency field, and correctly tightens the output.Configuration guard from cfg.Concurrency > 0 || len(cfg.EnvVars) > 0 to just len(cfg.EnvVars) > 0.ServiceConfig.ConcurrencyMode, ServiceConcurrency in core_v1alpha, the appconfig per-service concurrency block) is untouched — this change is precisely scoped to the legacy top-level fields.One minor cosmetic note: removing the three concurrency lines from app_status.go around line 106 left a blank line immediately after ctx.Printf("\n%s\n", ...). It's harmless but mildly untidy — not worth blocking on.
Overall the change is safe, well-scoped, and the protocol-safety reasoning in the comment is sound.
Verdict: ready
{
"owner": "mirendev",
"repo": "runtime",
"number": 1093,
"verdict": "ready",
"event": "approve",
"summary": "This is a clean, focused removal of two dead pieces: the top-level `Configuration.Concurrency`/`AutoConcurrency` fields (superseded by per-service concurrency) and the `set` CLI command that was the only consumer of those fields.\n\n**What I checked:**\n- No surviving callers of `HasConcurrency`, `SetConcurrency`, `AutoConcurrency`, or `commands.Set` anywhere in the codebase — the search came back empty across all 400 files scanned.\n- The CBOR/JSON index reservation in `rpc.yml` is handled correctly: indices 1 and 2 are left as a gap (with an explanatory comment) rather than recycled, so old clients decoding a new payload won't misread a future field through a stale index. That's the right call for a binary-serialised protocol.\n- The generated `rpc.gen.go` matches the schema change exactly — the `AutoConcurrency` type and the two `configurationData` fields drop together cleanly.\n- `app_status.go` removes both the human-readable and JSON output paths for the old concurrency field, and correctly tightens the `output.Configuration` guard from `cfg.Concurrency \u003e 0 || len(cfg.EnvVars) \u003e 0` to just `len(cfg.EnvVars) \u003e 0`.\n- The remaining concurrency machinery (`ServiceConfig.ConcurrencyMode`, `ServiceConcurrency` in `core_v1alpha`, the `appconfig` per-service concurrency block) is untouched — this change is precisely scoped to the legacy top-level fields.\n\nOne minor cosmetic note: removing the three concurrency lines from `app_status.go` around line 106 left a blank line immediately after `ctx.Printf(\"\\n%s\\n\", ...)`. It's harmless but mildly untidy — not worth blocking on.\n\nOverall the change is safe, well-scoped, and the protocol-safety reasoning in the comment is sound.",
"posted_to_pr": true
}