[FIRE-1932] feat(heartbeat): fire on Stop as well as SessionStart, throttled - #37
Merged
amos-qualifire merged 8 commits intoAug 18, 2026
Conversation
…ssions report
SessionStart was the only trigger, so a session left open for days produced
exactly ONE beacon and ONE log upload for its entire lifetime: the roster row
went stale and the hook log sat on disk unshipped. Stop fires once per
assistant turn - verified 1:1 against UserPromptSubmit on a real session, and
NOT once per tool call - which makes it the frequent trigger this needed.
Registering heartbeat.sh on Stop rather than adding a ship-only call path is
what keeps this small: the shipper already rides inside heartbeat.*, so one
new hooks.json group per event replaces what would have been a second
invocation path in each plugin. The Stop entry is a verbatim copy of the
SessionStart one plus the trigger argument, so the two command strings cannot
drift apart in quoting, flags or timeout.
Because Stop is per-turn, the beacon POST is throttled
(ROGUE_HEARTBEAT_MIN_INTERVAL, default 900s, stamp under ~/.rogue/beacon/).
The shipper needs no such gate - it already throttles itself, and a run with
nothing new makes no request at all.
Five properties both dispatchers share, each pinned by a test:
* SessionStart is NEVER throttled. It fires once per session and that is
exactly when the roster wants the update. The trigger argument defaults to
SessionStart, so an older hooks.json behaves precisely as it does today.
* Numeric zero disables the throttle; a non-numeric value falls back to the
default - the rule ROGUE_LOG_MAX_BYTES and ROGUE_SHIP_MIN_INTERVAL already
use. I had proposed flooring zero and was wrong: ship-logs parses its twin
with allow-zero explicit, and both test harnesses set 0 deliberately.
* A value too wide for int64 falls back to the default. dash answers `-lt`
on an oversized value with "Illegal number" and a FALSE, which reads as
"not throttled" and would loose a beacon on every turn.
* Every unreadable, corrupt, empty or future-dated stamp answers "not
throttled". A stamp we cannot trust must never silence presence reporting;
on the roster that is indistinguishable from an uninstalled plugin.
* The stamp is written before the request - a crash-loop guard as much as a
rate limit - 0700/0600 on sh, and BOM-less on PowerShell, since 5.1's
Add-Content -Encoding UTF8 would prepend three bytes that fail every
TryParse reading it back, disabling the throttle silently.
heartbeat.ps1 gains the ROGUE_PS_LIB_ONLY seam the other dispatchers have, so
the throttle is testable on Linux CI and under Windows PowerShell 5.1.
Three bugs found while writing the tests were all in the tests, and each is
now commented where it bit: a `${x:+VAR=$x}` prefix is not an assignment under
zsh, PowerShell's Remove-Item will not delete a dot-prefixed file without
-Force, and dot-sourcing heartbeat.ps1 overwrites a same-named $Trigger in the
caller's scope - the trap tests/log_probe.ps1 already documents, which cost
four cases a false pass.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
…y env file heartbeat.ps1 read $env:ROGUE_HEARTBEAT_MIN_INTERVAL at file scope, which runs before the credential files are parsed. heartbeat.sh sources them first and then reads the knob, so ROGUE_HEARTBEAT_MIN_INTERVAL in ~/.rogue-env, in a bundled env, or in an MDM C:\ProgramData\rogue\env took effect on macOS and Linux and did nothing at all on Windows: every Windows machine stayed on the 900s default. Nor could anyone work around it, because this script is spawned detached by hook.ps1 and its process environment comes from Claude Code rather than from whoever configured the box. Resolve it from the credential map in Initialize-BeaconThrottle, called after the map is built - the same shape, and the same reason, as Initialize-Logging for the log knobs. The variable joins the process-env override list so process env still wins over the files. Also fix the epoch arithmetic in the same file. [datetime]'1970-01-01T00:00:00Z' reads the Z as UTC and then converts to local, so its Kind is Local; subtracting a Local from a Utc does naive tick arithmetic and returned epoch minus the machine's UTC offset. The throttle still behaved correctly, since one function both writes and reads the stamp and the delta was therefore right, but the number on disk was hours off from the true epoch heartbeat.sh writes to the same path with `date -u +%s` and from ship-logs.ps1's Get-EpochSeconds, which already constructs it with DateTimeKind::Utc. Copy that construction. The tests grew an independent reference clock, because a suite that shares the implementation's epoch helper agrees with the bug. Six new assertions cover both defects, and all six fail against the reverted code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng on shipper lines tests/manual/local_api.ps1 is the Windows sibling of local_api.sh: same up/sync/status/check/probe/down flow, pointing a native-Windows Claude Code at the plugin working tree and a local API. It is not a convenience port. The sh harness cannot run here even under Git Bash, because install_path reads a C:\Users\... path out of installed_plugins.json and `cp -R "$IP/"` eats the backslashes as escapes - a sync would appear to succeed and copy nothing. It is also the only thing anywhere that drives hook.ps1 and heartbeat.ps1 through real Claude Code; e2e_ship_logs.ps1 covers the shipper on windows-latest but never the dispatcher. Its check adds the two assertions only a real Windows run can make: no UTF-8 BOM and no CR bytes in the log. Sanitize strips \x00-\x1f from every server-controlled string before logging, so a CR can only come from a writer using Add-Content or Out-File instead of AppendAllText with an explicit `n. It also parses the beacon stamp exactly as Test-BeaconThrottled does, since an unparseable stamp reads as "not throttled" and would beacon on every turn. Both harnesses now count DISPATCHER lines only. `event=ShipLogs` lines legitimately carry no surface= - ship-logs.* is the byte-identical shared script, it takes the slug as an argument and has no surface signal - so counting them made every surface assertion fail as soon as one landed in the tail, which is exactly when shipping is enabled and the feature is working. Both gained a beacon-interval flag, written to the env file. Without it the 900s default means a test session shorter than fifteen minutes sees exactly one beacon, indistinguishable from the old SessionStart-only behaviour. The localhost-only URL guard is carried over deliberately and documented: `up` rewrites ~/.rogue-env, which all six plugins share. To reach an API on another machine, forward the port rather than widening the guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nt success as nothing Both local-API harnesses gain `ship [--reset|-Reset]`: run the shipper now rather than waiting on a Stop and guessing. --reset clears the offset state so the whole log re-ships from byte 0, which is the only way to get a repeatable, observable POST at a local API. It also corrects what `check` claimed. A SUCCESSFUL SHIP WRITES NOTHING - ship-logs.* logs only a fail, a skip or a stall, and deliberately never the happy path, because a line per run would mean every run has new bytes to ship and would destroy the "an idle machine makes no HTTP request at all" property the throttle rests on. `check` presented the event=ShipLogs count as "the number you came to read", so a perfectly working install read as zero progress while a broken one looked busy. Those lines are a failure feed; the durable evidence of success is `offset=` advancing, which happens on a 2xx only. Both harnesses now report the offset against the log size and say how many bytes are still pending. `ship` runs with ROGUE_DEBUG=1, since without it a working run prints nothing at all and cannot be told from one that did nothing. When the offset does not move it now names the two causes and how to tell them apart, instead of listing both and leaving the reader to guess. The PowerShell copy spawns the shipper as an -EncodedCommand child (in-process, its `exit 0` would end the caller and its $script: writes would land on the caller's variables) and refuses outright on non-Windows, where ship-logs.ps1 stands down - otherwise the caller misreports a correct stand-down as "nothing new on disk", which it did on the first run. Verified end to end against tests/e2e_receiver.mjs in a sandboxed HOME: first ship 400 bytes, `cmp` wire against disk identical; an idle re-run makes no request and holds the offset; an incremental ship of 154 bytes stays identical; --reset re-ships all 554 from byte 0; and a forced 500 leaves the offset exactly where it was while check reports the pending bytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…point anywhere local_api.ps1 was Claude-only and loopback-only. Both were wrong for the actual job: the Cursor plugin's PowerShell dispatcher has no end-to-end coverage at all, and a local API is not always local. -Agent claude|cursor now selects the plugin. Everything that differs lives in one explicit table rather than being derived, because almost nothing is shared. Claude installs through `claude plugin install` from a staged marketplace; Cursor has no plugin CLI and installs by copying the directory into %USERPROFILE%\.cursor\plugins\local\rogue, the same asymmetry install.ps1 encodes. The surface vocabularies differ (cli|desktop|cowork against the single constant `cursor`), so `check` now discriminates per agent - a `cli` slug in cursor.log and a `cursor` slug in claude.log both fail. Claude beacons from heartbeat.ps1 on SessionStart and Stop with a throttle stamp; Cursor beacons inline from hook.ps1 at sessionStart only, so it has no stamp, `check` reads its `heartbeat=` log lines instead, and -BeaconInterval is called out as meaningless there. `probe` is claude only - there is no headless Cursor CLI. The shipper's family stays a separate field from the slug, not derived from it: codex ships codex.log under family `openai`, so inferring one from the other could never grow a third agent. Two things that would have destroyed state. Cursor's install target is the same path the one-line installer uses, so `up` moves an existing copy to <dest>.localdev-backup and `down` puts it back. And since ~/.rogue-env is shared by all six plugins, `down` now restores it only when no other agent is still up, tracked by one state-<slug> file per agent. Fixed while testing: `ship` read ~/.rogue/ship/<slug>.state, but the shipper derives that key from the LOG FILE'S BASENAME (Get-StateKeyForPath). The two coincide only because the log is named <slug>.log, so under ROGUE_LOG_FILE the subcommand would read a state file that never exists and report a successful upload as "no offset recorded". -AllowRemoteUrl replaces the flat loopback refusal. The guard exists to catch a TYPO, since `up` rewrites the shared env file and a wrong host there redirects every agent on the machine at once - but pointing at a real staging host is a legitimate thing to want, so it is now explicit rather than impossible. Verified in a sandboxed HOME: the cursor copy install reads 1.1.1 from .cursor-plugin/plugin.json; a pretend real install is moved aside and restored byte-for-byte by `down`; check passes on a realistic cursor.log and fails on a leaked foreign slug in both directions; the shipper run with slug=cursor family=cursor produces an envelope carrying agent_family=cursor shipper=cursor shipper_version=1.1.1 log_file=cursor.log with the reassembled bytes identical to disk; and tearing down one agent leaves the shared env file alone while the last one restores it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…not per session
The Stop-triggered heartbeat landed for Claude Code only. The other five
plugins still fired theirs once per session, so a session left open for days
produced exactly ONE beacon and ONE log upload for its whole lifetime: the
roster row went stale and the hook log sat on disk unshipped.
All six now run a session trigger AND a per-turn one. Which event, and where
it is fired from, differs per plugin and the second half is load-bearing:
claude SessionStart / Stop second hooks.json group
codex SessionStart / Stop hook.{sh,ps1}
copilot sessionStart / agentStop hook.{sh,ps1}
cursor sessionStart / stop hook.{sh,ps1}, inline
gemini SessionStart / AfterAgent hook.mjs
antigravity new conversation / Stop hook.{sh,ps1}
Only claude gets a new hooks.json entry, because only claude can afford one.
Codex, Copilot and Gemini fingerprint the whole hook definition and skip
untrusted command hooks until the user reviews them via /hooks, so a new entry
would have silently disabled every Rogue hook on every existing install until
each user re-approved. Their dispatchers already run on the turn-end event, so
firing from there leaves the command strings byte-identical and trust intact.
The throttle moves to scripts/shared/beacon.{sh,ps1}, synced byte-identically
into the five sh/ps plugins exactly like ship-logs (Gemini inlines it, same
rule as ship-logs.mjs). Extracted rather than copied because the semantics
carry at least six individually-silent traps, and every drift fails one of two
ways: no beacon ever again (a roster row indistinguishable from an uninstalled
plugin) or a beacon on every turn (a fleet-wide request storm). Per-plugin
differences are arguments - the stamp slug, and whether this trigger is the
session one. Deciding and stamping are one call, so a caller cannot leave the
window permanently open by forgetting the second half, nor stamp on a skip and
push the window forward forever.
Two bugs found on the way:
Antigravity's existing gate was not per-session at all. invocationNum resets
to 0 on every new prompt, so a 10-prompt session sent 10 unthrottled beacons,
and PreInvocation runs BEFORE the turn's transcript rows exist, so the shipper
riding along only ever uploaded turns 1..N-1. A session start is now
invocationNum 0 AND initialNumSteps <= 1; a continuation fires nothing and
lets its Stop cover it. initialNumSteps must be extracted and compared
numerically, never glob matched: *"initialNumSteps":1* also matches 18, so
every continuation of a long conversation read as fresh. The test caught it.
A PowerShell dot-source runs in the CALLER's scope, so loading the beacon
library inside a helper defined it in that helper's scope and it vanished on
return, leaving the consumer with no Request-RogueBeaconSlot. Antigravity's
main-and-functions heartbeat now returns the scriptblock and Invoke-Main
dot-invokes it; function lookup is dynamic, so its callees see it. Verified
both halves of that behaviour before relying on it.
Cursor is the one whose beacon runs inside the synchronous dispatcher, and
whose PowerShell half is a sync POST, so the throttle matters more there than
anywhere else - unthrottled, a per-turn beacon would add its latency to every
turn. It is also the only plugin that logs the decision (heartbeat=throttled),
since it is the only one where the decision happens where the log is written.
Copilot skips a subagent's agentStop: a subagent's stop is not a user turn, and
one turn using three subagents would queue four beacons for the throttle.
tests/test_heartbeat_{sh.sh,ps1.ps1} now exercise the real shared library
through the real dispatcher, assert the five copies are byte-identical, and pin
each plugin's wiring - the library load, the claim slug, the creds-map
initialisation and the per-turn call site. Each assertion was checked to fail
against a deliberate break. All six versions bumped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
yuval-qf
approved these changes
Aug 17, 2026
Nine conflicts. Most are two features landing in one spot and both belong: codex/copilot/cursor hook.ps1 each gained a per-turn heartbeat spawn from this branch and an install-identity block from #38, so both sides are kept. The rest are the per-turn trigger meeting the single-resolution rule: - antigravity: keep this branch's trigger logic (Stop, plus a new conversation as invocationNum 0 AND initialNumSteps <= 1, extracted and compared numerically) but hand the heartbeat ROGUE_INSTALL_AGENT rather than SURFACE, in both dispatchers. They differ only when a payload is unattributable: the log token stays empty, where an empty -Agent makes heartbeat.{sh,ps1} sniff the filesystem and pick antigravity_cli whenever `agy` is installed - one install, two roster rows. heartbeat.sh also drops its own `hostname` read, since install-id.sh sets HOST. tests/test_hook_sh_antigravity.sh asserts the stub's `<agent>|<trigger>` marker accordingly. - cursor: keep the inline two-trigger beacon and its throttle, on the host and version resolved once at the top of the dispatcher (they also ride the per-event headers now), instead of the block's own HB_VER/HB_HOST read. - gemini heartbeat.mjs: keep the inlined beacon throttle and take the version from installId(), dropping the now-dead readVersion(). Restores `import fs`, which the merge removed along with readVersion: the throttle's three fs calls all sit inside try/catch, so a ReferenceError there was swallowed and claimBeaconSlot() returned true every time - an unthrottled beacon on every turn, fleet-wide, with no symptom. Verified after the fix: three per-turn runs make one POST, ROGUE_HEARTBEAT_MIN_INTERVAL=0 makes one each, SessionStart is never throttled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e behind
/api/v1/hooks/logs is deployed, so the opt-in that existed only because it was not
is gone. Probed against production on 2026-08-18: an empty body is answered 422
{"type":"validation","on":"body"} - body validation runs ahead of auth, and the
route knows this shipper's schema - where /api/v1/hooks/nonexistent-route still
answers a bare 404 NOT_FOUND. A configured install now uploads its hook log, and the
only things that stop a run are the ones that always could: no API key, no resolvable
actor, the self-throttle, or no new bytes on disk.
REMOVED rather than flipped, in all three implementations plus the five synced
copies: flag_is_enabled / Test-FlagEnabled / flagIsEnabled, value_is_zero /
Test-ValueIsZero / valueIsZero, the SHIP_DISABLED_BY_FILE plumbing, and
ROGUE_SHIP_LOGS from every env-var list.
That also removes the =0 kill switch, which was the one deliberate inversion of
"process env beats the files", so THERE IS NO MACHINE-LEVEL OPT-OUT ANY MORE. A
value left behind on an upgraded machine - inline or in an MDM /etc/rogue/env - is
ignored, and all three test layers assert exactly that: an upgrade must not leave a
fleet silently half-off with no knob left to explain it. If an opt-out is wanted
again it is a new control with a name that says what it does, not a resurrection of
this one.
Tests keep the coverage rather than dropping it, inverted: test_ship_logs.sh asserts
a configured install ships with nothing set (first, from a clean case, because every
other assertion in the file would pass vacuously against a shipper that does
nothing) and that a stale 0 - inline, in a file, zero-padded, sh and Node - no
longer disables; test_ship_logs.ps1 asserts the ABSENCE of the gate (no flag in
SHIP_ENV_VARS, no helpers left, nothing in Invoke-Main that returns before the
API-key check); both e2e suites assert the same against a real receiver. The support
one-liners in all six /rogue:status documents lose the flag but keep
ROGUE_SHIP_MIN_INTERVAL=0 and ROGUE_DEBUG=1, since what they do is make one run
happen now and visibly. --ship / -Ship in the manual harnesses now mean "waive the
throttle so every turn uploads", which is all they ever did beyond setting the flag.
Worth stating plainly because it is now the product's behaviour: the hook log quotes
the content that tripped a rule - a secret, a command, a slice of a prompt - which is
why it is 0600 on disk, and every configured install now sends it. The server-side
requirement is unchanged and load-bearing: 2xx only after a durable write, since the
offset advances only on 2xx and the client forgets those bytes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
SessionStartwas the only trigger for the presence beacon and the log shipper. A session left open for days therefore produced exactly one beacon and one log upload for its whole lifetime — the roster row went stale, and the hook log sat on disk unshipped.What
Stopnow also fires the detached heartbeat. It fires once per assistant turn — verified 1:1 againstUserPromptSubmiton a real session, and not once per tool call.Registering
heartbeat.shonStop, rather than adding a ship-only call path, is what keeps this small: the shipper already rides insideheartbeat.*, so this is one newhooks.jsongroup instead of a second invocation path. TheStopentry is a verbatim copy of theSessionStartone plus the trigger argument, so the two command strings cannot drift in quoting, flags or timeout.Because
Stopis per-turn, the beacon POST is throttled (ROGUE_HEARTBEAT_MIN_INTERVAL, default 900s; stamp at~/.rogue/beacon/.last-claude). The shipper needs no such gate — it already throttles itself, and a run with nothing new makes no HTTP request at all.Properties, each pinned by a test
SessionStartis never throttled. It fires once per session and that is exactly when the roster wants the update. The trigger argument defaults toSessionStart, so an olderhooks.jsonbehaves precisely as it does today.ROGUE_LOG_MAX_BYTESandROGUE_SHIP_MIN_INTERVALalready use. I had proposed flooring zero and was wrong:ship-logsparses its twin with allow-zero explicit, and both test harnesses set0deliberately.-lton an oversized value withIllegal numberand a FALSE — which reads as "not throttled" and would loose a beacon on every turn.Add-Content -Encoding UTF8would prepend three bytes that fail everyTryParsereading it back, disabling the throttle silently.Tests
New
tests/test_heartbeat_sh.sh(dash + bash) andtests/test_heartbeat_ps1.ps1, both wired intovalidate.ymlincluding the Windows PowerShell 5.1 job.heartbeat.ps1gains theROGUE_PS_LIB_ONLYseam the other dispatchers already have, so the throttle is testable on Linux CI.All 16 local suites pass, plus the shipper e2e.
Scope
Claude only. The other five plugins have the identical one-shot trigger and should follow once this shape is settled — each has a different stop-equivalent event across three languages, and only this one is testable end-to-end against a local API today.
Stacked on #36.