Skip to content

Lift the connector's cap on workers, and let a start set one - #1040

Merged
robzolkos merged 3 commits into
mainfrom
connect-max-workers
Nov 13, 2026
Merged

robzolkos merged 3 commits into
mainfrom
connect-max-workers

Conversation

@robzolkos

@robzolkos robzolkos commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

What this does

A connector doing the work itself no longer caps how many workers run at once. Every request gets a worker as it arrives.

basecamp connect --max-workers N sets a cap for the operator who wants one: at most N workers run at once, and a request past the cap waits its turn.

  • Kept for the profile. Like the orchestrator, the worker and the permissions, a cap a start names is kept in runner.json (as max_workers), so --run starts with it.
  • Taking it off. --max-workers 0 removes a kept cap. A start that does not name the flag leaves the kept one alone, including one written into runner.json by hand.
  • Seeing it. basecamp connect workers list says "at most N at once" for a running connector that has a cap, and carries it as max_workers in its JSON.

Why

The runner ran at most four workers at once. Nobody chose that number, and the only way to change it was a hand edit to runner.json and a restart. The cap is now a control the operator reaches for, not a default everyone gets.

Behavior changes to know about

  • Existing connectors go from four at once to no cap when they update, unless their runner.json already has max_workers. The first start afterwards continues every worker a stop interrupted at once, not four at a time.
  • max_workers above 64 is accepted. It was refused before. With no cap as the default, a ceiling on the cap has nothing to protect.
  • A connector that is stopping starts no worker turn. A worker reaching its turn as the run ended could be started only to be stopped. Waiting for a place under the cap avoided that about half the time; with no cap it would have happened every time.

Worth a reviewer's attention

  • The cap is read once, at a start. I tried re-reading it as the connector runs, the way the rules and the permissions are, and dropped it: waiting workers lost their arrival order, and a file caught mid-save could lift the cap. For an optional setting that was not worth the machinery. The help, the skill and AGENTS.md say it is read at a start.
  • A flag given at the start wins again at basecamp connect restart, because a restart re-runs the original command line. A connector started with --max-workers 3 keeps 3 through a restart even if runner.json was edited in between. --permissions already behaves this way; the help text now says so for the cap.
  • The --shadow refusal named only the orchestrator and worker flags. It now names every flag that asks for the runner, this one included.

Verified

  • bin/ci on this branch.
  • Unit tests for: no cap by default (more than four workers at once), a cap from runner.json, a cap from the start over the file's, a start that takes the cap off, an edit during a run not changing the running cap, and the flag being kept, left alone and removed across starts.
  • The built binary refuses a value that is not a number, a negative one, and the flag with --shadow.

Not verified

  • A live connector started with the flag. The one profile available had its connector running throughout, and a profile runs one connector at a time.
  • macOS.
  • The skill's model evals. The skill gains one bullet.

Summary by cubic

Lifts the connector's built-in four-worker cap: every request now gets a worker as it arrives unless a start sets one with --max-workers N. The cap is kept in runner.json as max_workers and shown by basecamp connect workers list.

Behavior changes

  • Existing connectors update to no cap unless their runner.json already has max_workers.
  • A cap is read once at start, so editing runner.json while the connector runs applies at the next start.
  • --max-workers 0 removes a kept cap; a start without the flag leaves the kept cap alone.
  • Caps above 64 are accepted, and negative values are refused.
  • A connector that is stopping starts no worker turn, and a worker whose run is ending asks nothing of the hold.
  • basecamp connect restart re-runs the original flags, so a cap set at start survives a restart.

Written for commit 25940cc. Summary will update on new commits.

View guided diff Turn on auto-fix

RetriggerView in GreptileConfidence Score: 5/5 Tier: plus

This PR appears safe to merge; no actionable issues were found.

Summary

Removes the default four-worker cap and adds --max-workers. The chosen cap is saved in runner.json, applied at startup, and shown by workers list.

  • The built-in runner starts workers without a default cap.
  • Workers stop starting new turns as the connector shuts down.

Reviewed the source and related SDK references. No runtime tests were run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["Start connector"] --> B{"--max-workers given?"}
  B -->|Yes| C["Save the supplied cap"]
  B -->|No| D["Keep the saved cap"]
  C --> E["Choose the cap once"]
  D --> E
  E --> F{"Cap is zero?"}
  F -->|Yes| G["Workers start without waiting for a slot"]
  F -->|No| H["Workers wait for one of N slots"]
Loading

Reviews (1) · Last reviewed commit: "Lift the connector's cap on workers, and..." · Reviewed by Greptile

The runner ran at most four workers at once: a number nobody chose,
changed only by a hand edit to runner.json and a restart. Now every
request gets a worker as it arrives, and a cap is something the
operator sets when they want that control.

`basecamp connect --max-workers N` caps how many workers run at once,
so a request past the cap waits its turn. Like the rest of what a start
names it is kept for the profile, as max_workers in runner.json, so
--run starts with it. `--max-workers 0` takes a kept cap off, and a
start that does not name it leaves the kept one alone.

A cap is read once, at a start. `basecamp connect workers list` says
what the running connector's is.

A connector with no max_workers of its own goes from four at once to
no cap. So that a stop does not start a turn only to end it, which the
cap's wait used to prevent half the time, a run that is ending starts
no worker turn.
@robzolkos
robzolkos requested a review from a team as a code owner November 13, 2026 16:41
Copilot AI balanced review requested due to automatic review settings November 13, 2026 16:41
@github-actions github-actions Bot added commands CLI command implementations tests Tests (unit and e2e) skills Agent skills docs labels Nov 13, 2026
Copilot AI previously approved these changes Nov 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

No active review issues block approval of the worker-concurrency configuration changes.

No open issues 🎉

What changed in this PR

Removes the connector runner’s default four-worker limit and adds an optional, profile-persisted --max-workers cap.

Changes:

  • Defaults to uncapped worker concurrency; explicit caps are saved in runner.json, and 0 removes them.
  • Reports the active cap in worker status and adds an early shutdown check before worker turns.
  • Adds tests and documents persistence, startup-only configuration, and restart behavior.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File Description
skills/​basecamp-connect/​SKILL.md Explains setting, removing, and changing worker caps.
internal/​connector/​runner/​workers.go Makes slot waiting optional and checks for shutdown early.
internal/​connector/​runner/​spec.go Validates and persists optional caps without an upper ceiling.
internal/​connector/​runner/​spec_test.go Tests cap validation, preservation, and removal.
internal/​connector/​runner/​socket.go Exposes the active cap in status responses.
internal/​connector/​runner/​runner.go Removes the default cap and applies startup overrides.
internal/​connector/​runner/​runner_test.go Tests uncapped execution, cap precedence, and startup-only behavior.
internal/​commands/​connect.go Documents cap controls and restart semantics in help.
internal/​commands/​connect_runner.go Registers the flag, saves its value, and displays the cap.
internal/​commands/​connect_runner_test.go Tests flag parsing and persistence across starts.
internal/​commands/​connect_run.go Passes startup settings through and expands the shadow-mode error.
AGENTS.md Documents optional caps and when they take effect.

Review effort: Balanced

greptile-apps[bot]
greptile-apps Bot previously approved these changes Nov 13, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 12 files

You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/commands/connect_run.go">

<violation number="1" location="internal/commands/connect_run.go:231">
P3: The refusal omits `--orchestrator-model`, `--orchestrator-effort`, `--worker-model`, and `--worker-effort`, even though each triggers this branch. List them so the error tells users which runner flags to drop.</violation>
</file>

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread internal/connector/runner/spec.go Resolved
Comment thread internal/commands/connect_runner.go Resolved
Comment thread internal/connector/runner/workers.go Resolved
Comment thread internal/commands/connect_run.go Outdated
if f.runner.requested() {
if f.shadow {
return output.ErrUsage("A shadow run hands nothing off, so it has no orchestrator: drop --shadow, or the orchestrator and worker flags")
return output.ErrUsage("A shadow run hands nothing off, so it has no orchestrator: drop --shadow, or the flags that have the connector do the work (--orchestrator, --worker, --permissions, --max-workers, --run)")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The refusal omits --orchestrator-model, --orchestrator-effort, --worker-model, and --worker-effort, even though each triggers this branch. List them so the error tells users which runner flags to drop.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At internal/commands/connect_run.go, line 231:

<comment>The refusal omits `--orchestrator-model`, `--orchestrator-effort`, `--worker-model`, and `--worker-effort`, even though each triggers this branch. List them so the error tells users which runner flags to drop.</comment>

<file context>
@@ -228,7 +228,7 @@ func runConnect(cmd *cobra.Command, f *connectRunFlags) error {
 	if f.runner.requested() {
 		if f.shadow {
-			return output.ErrUsage("A shadow run hands nothing off, so it has no orchestrator: drop --shadow, or the orchestrator and worker flags")
+			return output.ErrUsage("A shadow run hands nothing off, so it has no orchestrator: drop --shadow, or the flags that have the connector do the work (--orchestrator, --worker, --permissions, --max-workers, --run)")
 		}
 		if start, err = connectRunnerSettle(f.runner, connectRunnerConfigPath(path)); err != nil {
</file context>
Suggested change
return output.ErrUsage("A shadow run hands nothing off, so it has no orchestrator: drop --shadow, or the flags that have the connector do the work (--orchestrator, --worker, --permissions, --max-workers, --run)")
return output.ErrUsage("A shadow run hands nothing off, so it has no orchestrator: drop --shadow, or the flags that have the connector do the work (--orchestrator, --orchestrator-model, --orchestrator-effort, --worker, --worker-model, --worker-effort, --permissions, --max-workers, --run)")

Comment thread internal/connector/runner/spec.go Resolved
- A run that is ending is checked for where a turn is about to be
  started, on every turn and not only a worker's first.
- The --shadow refusal names the model and effort flags too.
- `workers list` says in its help that it reports the cap.
- Comments on runner.json and on the cap's check say what is so.
- The capped test's subtests fail as themselves.
Copilot AI balanced review requested due to automatic review settings November 13, 2026 16:57
@greptile-apps
greptile-apps Bot dismissed their stale review November 13, 2026 16:57

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

Copilot AI dismissed their stale review, a newer Copilot review was requested November 13, 2026 16:57
Copilot AI previously approved these changes Nov 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

No active review issues are reported for the optional worker-cap change.

No open issues 🎉

Review effort: Balanced

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 existing issue remains and no new issues found across 5 files (changes from recent commits).

You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread internal/connector/runner/workers.go Resolved
A worker reaching its turn as the connector stopped asked after the
hold with a context already ended, and logged that the hold stands.
Copilot AI balanced review requested due to automatic review settings November 13, 2026 17:09
Copilot AI dismissed their stale review, a newer Copilot review was requested November 13, 2026 17:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

No active review issues are reported for the configurable worker-concurrency change.

No open issues 🎉

Review effort: Balanced

@robzolkos
robzolkos merged commit c1675a0 into main Nov 13, 2026
32 checks passed
@robzolkos
robzolkos deleted the connect-max-workers branch November 13, 2026 17:19
jeremy added a commit that referenced this pull request Nov 14, 2026
* commit 'ef2b350fa4662d0060c2b5e90bcbe69e32e76dd0':
  Stop telling an agent to send a Markdown fold as HTML (#1030)
  Update nix flake and plugin version for v0.30.0 (#1042)
  Watch a connector in a terminal, and talk to its orchestrator (#1039)
  Lift the connector's cap on workers, and let a start set one (#1040)
  Never give the connector's monitor a timeout (#1035)
  Update nix flake and plugin version for v0.29.0 (#1037)
  Let assignments notify people with --notify (#1034)
  Update nix flake and plugin version for v0.28.1
  Bind local config trust to approved settings
  Update nix flake and plugin version for v0.28.0 (#1029)
  Let the connector do the work itself, on the coding agent you choose (#1026)
  Build with Go 1.26.11 (#1027)
  Open a release PR for the version bump instead of pushing it to main (#1025)
  Update nix flake and plugin version for v0.27.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands CLI command implementations docs skills Agent skills tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants