Skip to content

Never give the connector's monitor a timeout - #1035

Merged
jeremy merged 4 commits into
mainfrom
connect-monitor-persistent
Nov 12, 2026
Merged

jeremy merged 4 commits into
mainfrom
connect-monitor-persistent

Conversation

@jeremy

@jeremy jeremy commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Context

Kitchen card: a connector session armed its request monitor with a 30-minute timeout. At each expiry the model woke, re-armed it, and said another system "is running in the background. Nothing is needed from you." That was about 48 wasted turns a day, and nothing behind the status line had checked that system, which took no work for 15 hours meanwhile.

The skill already said persistent: true. What pushed the session to a timeout is the Monitor tool itself: in current Claude Code builds (2.1.322 here) a feature gate, on by default, drops persistent from the Monitor schema, makes timeout_ms the only knob (default 5 minutes, capped at 30), and the tool's description tells the model to "set timeout_ms to the maximum and re-arm on each expiry". 30 minutes is exactly that cap. So the skill has to say what to do both where persistent exists and where it doesn't.

Change

skills/basecamp-connect/SKILL.md only:

  • What this flow needs now names a Monitor tool that takes persistent, beside Claude Code. Without one, the session goes to Without this session (--orchestrator) before starting anything, which needs no monitor and records each event once.
  • Step 1 arms the monitor with persistent: true and never a timeout_ms, in the arming sentence itself. If it expires, re-arm it once, persistent; if that one expires too (a remote session turns persistent into a 30-minute cap), tell the person. An earlier head switched to the orchestrator mode mid-run instead, which needed a stop, a wait for the lock and a drain of requests already marked handed off; the switch went rather than gaining a third patch.
  • Without this session says why a monitor that always expires can't hold the watch: each re-arm replays the whole output file (tail -n +1), and a compacted session can't tell requests it handled from new ones.
  • Keep the terminal quiet now says the session speaks for the connector alone and never says whether anything else here is working.
  • The event_id skip rule names a re-armed monitor's replay beside a restart.

To keep the per-request steps inside the 14,000-character compaction budget (TestTheConnectSkillKeepsEachRequestsStepsInsideTheCompactionCut), the opening paragraph goes: the "Three pieces" list restates it point for point. The per-request part now ends at 13,949 characters, against 13,980 before: 51 characters of headroom, so the next addition here will have to trim something.

For a decision: the Monitor in Claude Code 2.1.322 has no persistent by default (see Context), so on today's builds this sends most sessions to the orchestrator mode rather than a session watch. The alternative is a session that re-arms every 30 minutes with a cursor file the monitor writes so a replay resumes after the last line passed: about 48 turns a day, and a persisted marker to keep right. That is the trade-off to settle; this PR takes the first.

Verification

  • bin/ci green on b9fab5d (run on a 32-core host): unit and e2e tests, lint, CLI surface, and skill drift for all four skill files. On 76fd277, which changes only SKILL.md wording, the connector skill tests and make check-skill-drift pass.
  • TestTheConnectSkillKeepsEachRequestsStepsInsideTheCompactionCut passes at 13,949 characters; the first head's additions without dropping the opening paragraph measured 14,233 and failed it.
  • The Monitor schema behavior is read from the Claude Code 2.1.322 bundle: with the gate on, the tool takes no persistent and caps timeout_ms at 30 minutes; with it off, persistent: true runs with no timeout and ignores timeout_ms.
  • Not run: make -C skill-evals eval-connect (model-graded; no case covers arming the monitor).

A connector session armed its request monitor with a 30-minute timeout
and re-armed it at every expiry, about 48 turns a day, each ending in a
status line that claimed another system was working when nothing had
checked it.

Step 1 now says it in the arming sentence itself: persistent: true and
never a timeout_ms. An expired monitor is re-armed once, persistent.
Where the Monitor tool offers no persistent option (some Claude Code
builds cap every monitor at 30 minutes and tell the model to re-arm at
the maximum), give it the longest timeout and re-arm silently, and offer
the orchestrator mode for a watch meant to last days.

The quiet-terminal rule now says the session speaks for the connector
alone and never says whether anything else here is working.

A re-armed monitor replays the file from the start, so the event_id
skip rule now names that alongside a restart. The per-request steps
stay inside the 14,000-character compaction budget (13,973) by dropping
the opening paragraph, which the "Three pieces" list restates.
@jeremy
jeremy requested a review from a team as a code owner November 12, 2026 19:35
Copilot AI balanced review requested due to automatic review settings November 12, 2026 19:35
@github-actions github-actions Bot added the skills Agent skills label Nov 12, 2026
Copilot AI previously approved these changes Nov 12, 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 recorded for the Basecamp connector monitoring guidance changes.

No open issues 🎉

What changed in this PR

Updates the Basecamp connector skill to prefer a timeout-free monitor and handle tools without persistent monitoring silently, avoiding repeated status chatter.

Changes:

  • Requires persistent monitoring without a timeout when supported; otherwise uses the longest timeout and silently re-arms.
  • Clarifies replay deduplication and limits terminal updates to the connector.
  • Offers session-independent operation for multi-day watches and trims introductory text to preserve the compaction budget.

[!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 Refines monitor lifecycle, replay handling, terminal messaging, and long-running watch guidance.

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 issue found across 1 file

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="skills/basecamp-connect/SKILL.md">

<violation number="1" location="skills/basecamp-connect/SKILL.md:82">
P1: The no-`persistent` fallback replays the whole output file at every expiry, but the skill stores handled `event_id`s only in session context. After compaction, old requests can be dispatched again, causing duplicate work and Basecamp replies; persist a replay cursor or handled-event set, or route multi-day watches through orchestrator mode.</violation>
</file>

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

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

Comment thread skills/basecamp-connect/SKILL.md Resolved
cubic found that the no-persistent fallback re-armed every 30 minutes
replaying the whole output file, so a compacted session would take
requests it had already handled as new ones. Rather than keep a cursor
for a watch the tool can't hold, the skill now names a Monitor tool
that takes persistent as part of what this flow needs, and sends a
session without one to the orchestrator mode, which records each event
once. A persistent monitor that expires anyway is re-armed once; if
that one expires too, the same handoff applies.
Copilot AI balanced review requested due to automatic review settings November 12, 2026 19:41
Copilot AI dismissed their stale review, a newer Copilot review was requested November 12, 2026 19:41

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.

🟡 Changes recommended

The monitor-expiry fallback leaves the existing connector running, so its instance lock prevents the replacement orchestrator from starting; the connector must be stopped and its exit awaited first.

1 open issue

Review effort: Balanced

Comment thread skills/basecamp-connect/SKILL.md Resolved

@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.

All reported issues were addressed across 1 file (changes from recent commits).

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

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

Comment thread skills/basecamp-connect/SKILL.md Resolved
The plain connector started in step 1 holds the profile's lock, so an
orchestrator started beside it refuses. When the one persistent re-arm
expires too, the skill now stops the connector and waits for it to exit
before following Without this session.
Copilot AI balanced review requested due to automatic review settings November 12, 2026 19:45

@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.

All reported issues were addressed across 1 file (changes from recent commits).

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

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

Comment thread skills/basecamp-connect/SKILL.md Resolved

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.

🔵 Needs a closer look

The connector-to-orchestrator switch can lose requests emitted after monitoring ends because they are marked handed off but never drained from the old output.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

High severity Drain connector output before switching to avoid losing requests

skills/​basecamp-connect/​SKILL.md:84

A request printed after the second monitor expires but before the connector exits can be lost during this switch. The plain connector marks each printed request handed_off immediately (internal/connector/handoff.go:287–295), so the new orchestrator will neither receive it from the ledger nor replay the old output file. After stopping and waiting for exit, drain that output and handle unseen event_ids before starting the orchestrator.

1 resolved since last review

Review effort: Balanced

Switching a running session to the orchestrator mode mid-run took a
stop, a wait for the lock, and a drain of requests the plain connector
had already marked handed off, each found in a review round. The switch
goes instead: whether a session can hold the watch is decided before it
starts (a Monitor tool that takes persistent), and a persistent monitor
that expires after its one re-arm is a failure to tell the person about.
Copilot AI balanced review requested due to automatic review settings November 12, 2026 19:53

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.

🔵 Needs a closer look

Routing most current Claude Code sessions to detached orchestrators with unattended full permissions changes the operating model and requires human confirmation of the trade-off.

No open issues 🎉

Review effort: Balanced

@jeremy

jeremy commented Nov 12, 2026

Copy link
Copy Markdown
Member Author

Review threads: 4 resolved (4 fixed, 0 declined).
Review bodies: 1 finding answered (1 fixed).
Fixed from a review body:

  • Drain the connector's output before switching: the mid-run switch to the orchestrator mode is gone (76fd277), so nothing is stopped and no handed-off request is dropped.
    Open for a decision:
  • Copilot on 76fd277, for @jeremy: Claude Code 2.1.322's Monitor offers no persistent by default, so this sends most current sessions to the detached orchestrator mode instead of a session watch. The alternative, a session re-arming every 30 minutes with a cursor file so replays resume where they stopped, costs about 48 turns a day and a persisted marker. The PR body's "For a decision" has both.

@jeremy
jeremy merged commit 7727131 into main Nov 12, 2026
29 checks passed
@jeremy
jeremy deleted the connect-monitor-persistent branch November 12, 2026 20:33
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

skills Agent skills

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants