[FIRE-1932] feat(logs): add surface (i.e. Claude CLI, Claude Desktop, etc) on every hook log line - #36
Open
amos-qualifire wants to merge 3 commits into
Conversation
There is one log file per agent family per machine, and every surface of that
family appends to it: Claude Code from the CLI, from the Desktop app and a Cowork
session all write the same claude.log, and nothing on the line said which one.
The heartbeat knows the surface, but it describes ONE session, while a log holds
lines from many sessions across many surfaces - so it has to be stamped as the
line is written.
2026-08-13T07:02:05Z provider=claude surface=cli event=PreToolUse outcome=allow
Directly after provider=, before event=. Lowercase closed-vocabulary slug, no
space and no `=`, so a reader still finds every value by scanning to the next
`key=`.
OPTIONAL, and that is load-bearing. An undetermined surface omits the WHOLE
token: `surface=` with nothing after it, or `surface=unknown`, would both be
indistinguishable from a real value to that same reader. Lines written by earlier
versions have no token and are never rewritten. In practice it is absent only
there and on an antigravity event whose payload carries no transcriptPath -
including outcome=unconfigured, which is logged before stdin is read.
ONE resolution per plugin, from the signal its heartbeat already uses:
- plugins/rogue/scripts/surface.{sh,ps1} is a new shared table. hook takes the
slug, heartbeat takes the display label, from the same `case` - previously
duplicated in both. Claude's dispatchers exit before their first log line
when CLAUDE_CODE_ENTRYPOINT is unset, so the slug is always resolvable there.
- plugins/codex/scripts/surface.{sh,ps1} reads ROGUE_CODEX_SURFACE once and
VALIDATES it against the closed list, feeding the log token, the
x-rogue-agent header and the heartbeat. The validation is new: the value
comes from an env file, so an arbitrary string with a space or an `=` in it
would otherwise have reached both a log line and a roster row.
- antigravity already resolved its surface from the payload's transcriptPath
and passed it to the heartbeat; that one value now also stamps the line and
drives the IDE-only enrichment branch. Its PowerShell half resolved this
AFTER the heartbeat, which would have left a few lines untagged where the sh
half tagged them - Resolve-Surface moved up to match.
- cursor, copilot and gemini have exactly one surface each, so a constant -
still emitted through the same conditional as everyone else, so all six
dispatchers share one emit shape.
A second, independent way to decide the surface would be worse than no token: a
line and the roster row for the same session could name different surfaces.
No new failure modes. Every resolution is guarded on both sides; a missing
surface.sh, an unreadable file or a throwing resolver yields an empty slug, which
yields no token. Nothing here can block a session, change an allow/deny outcome,
or write to stderr. The upload envelope is untouched.
Tests: the slug matrix per plugin, the emit position, and that a placeholder is
never written, in all three languages. The claude table is asserted against the
heartbeat's own labels so the two cannot drift. tests/log_probe.ps1 needed a
snapshot of its -Surface argument before dot-sourcing the dispatcher: PowerShell
variable names are case-insensitive, so the dispatcher's own $script:surface
silently overwrote the parameter - the same trap the probe already documents for
$Creds.
Versions: rogue 1.0.24, codex 1.0.1, cursor 1.1.1, copilot 1.2.1, antigravity
1.0.24, gemini 1.0.25. Vocabulary, per-plugin detection and the absent-token
rules: docs/hook-log-format.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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 |
…ument
The new "omits the token" case passed `-Surface ''` to the probe. 5.1 drops an
empty native-command argument, so the child saw a bare `-Surface` and refused to
bind it:
log_probe.ps1 : Missing an argument for parameter 'Surface'.
Which is the trap the sibling arguments in this harness are already conditional
for, and which pwsh 7 does not reproduce - so it only surfaced on the
windows-latest job. A '<none>' sentinel carries "force it empty" instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…st a local API
live_session.sh installs `git archive HEAD` and asserts against a fake node
receiver, then tears everything down. Neither half fits the case this adds:
exercising UNCOMMITTED plugin changes, interactively, against a backend the
developer is running themselves.
local_api.sh installs the working tree as a `rogue-localdev` marketplace and
leaves it installed, so ordinary sessions drive it. Three details are the
reason it is a script rather than a paragraph of instructions:
* The install COPIES the tree, so editing plugins/rogue changes nothing
until the copy is refreshed - verified, not assumed. `sync` refreshes the
installed copy in place, which keeps the install record, version and
enabled state intact and takes effect on the next hook invocation.
* Two enabled rogue plugins answer every event twice. `up` disables
rogue@rogue-marketplace and records that it did, so `down` re-enables
only what it actually disabled.
* Credentials have to go in ~/.rogue-env, not the process environment:
hook.sh sources the env files after the process environment is already
set, with no save/restore, so the file's ROGUE_API_KEY wins. The original
is backed up, and a second `up` will not clobber that backup.
`check` reads the log a real session wrote and asserts this branch's contract
on it: the token present, in position, from the closed list, never `unknown`,
never empty. Two of those assertions were written the weak way first and
fixed after a deliberately malformed log failed to trip them - the position
check passed as soon as ONE line was right, and the empty-token check anchored
on a trailing space so it missed an empty value at end of line.
A non-local --url is refused: this file is read by all six plugins, so a typo
there sends every agent's prompts and tool calls somewhere unintended. The
IPv6 arm escapes its brackets, since `[::1]` is a glob bracket expression and
the unescaped form never matches the address it names.
tests/manual/*.sh now joins the shell-parse job. Nothing else in CI reads
those files, so a parse error surfaces only when someone reaches for the
harness - exactly when they can least afford to debug it.
amos-qualifire
changed the title
drorIvry
approved these changes
Aug 16, 2026
drorIvry
left a comment
drorIvry
left a comment
Contributor
There was a problem hiding this comment.
Super important to test windows as well
Comment on lines
-652
to
+658
| _surface=$(surface_from_transcript "$(json_field transcriptPath "$BODY")") | ||
| _surface="$SURFACE" |
Comment on lines
+1
to
+21
| # Codex's SURFACE - the PowerShell half of surface.sh, kept in lockstep with it. | ||
| # | ||
| # hook.ps1 sends it as the x-rogue-agent header AND stamps it on each log line | ||
| # heartbeat.ps1 sends it as the roster agent | ||
| # | ||
| # Codex exposes no app/cli entrypoint variable of its own, so the installer pins | ||
| # ROGUE_CODEX_SURFACE per surface and everything reads that one value. | ||
| # | ||
| # slug | when | ||
| # -----------|------------------------------------------------------- | ||
| # codex_app | ROGUE_CODEX_SURFACE=codex_app | ||
| # codex_cli | ROGUE_CODEX_SURFACE=codex_cli, unset, or ANYTHING ELSE | ||
| # | ||
| # THE CLOSED LIST IS ENFORCED HERE. The value comes from an env file, so it is | ||
| # whatever someone wrote there: a space or an '=' would break the log line's | ||
| # key=value shape, and arbitrary text is exactly what this token must never carry. | ||
| # | ||
| # Takes the resolved credential map rather than reading $env: directly - the | ||
| # variable normally lives in ~/.rogue-env, which the process environment has not | ||
| # seen. Dot-sourced; Windows PowerShell 5.1 compatible. | ||
|
|
Comment on lines
+1
to
+24
| #!/bin/sh | ||
| # Codex's SURFACE — one table, three consumers. | ||
| # | ||
| # hook.sh sends it as the x-rogue-agent header AND stamps it on each log line | ||
| # heartbeat.sh sends it as the roster agent | ||
| # | ||
| # Codex exposes no app/cli entrypoint variable of its own, so the installer pins | ||
| # ROGUE_CODEX_SURFACE per surface and everything reads that one value. Keeping the | ||
| # read in one place is what stops a log line and the roster row for the same | ||
| # session from naming different surfaces. | ||
| # | ||
| # slug | when | ||
| # -----------|------------------------------------------------------- | ||
| # codex_app | ROGUE_CODEX_SURFACE=codex_app | ||
| # codex_cli | ROGUE_CODEX_SURFACE=codex_cli, unset, or ANYTHING ELSE | ||
| # | ||
| # THE CLOSED LIST IS ENFORCED HERE, and that is the point of the function rather | ||
| # than a bare `${ROGUE_CODEX_SURFACE:-codex_cli}`. The variable comes from an env | ||
| # file, so its value is whatever someone wrote there: a value with a space or an | ||
| # `=` would break the log line's `key=value` shape, and an arbitrary string on a | ||
| # line or in a roster row is exactly the kind of uncontrolled content this token | ||
| # must never carry. Anything unrecognised is treated as unset. | ||
| # | ||
| # Sourced, not executed. |
Comment on lines
+88
to
+90
| # The one surface this plugin has. A closed-vocabulary slug, lowercase, no space | ||
| # and no '=', so a reader finds the value by scanning to the next 'key=' token. It | ||
| # matches what heartbeat reports as the roster agent for this plugin. |
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.
Stacked on #30 (which is stacked on #29), because the line format it extends lands there.
What
Every dispatcher now writes an optional
surface=token, directly afterprovider=and beforeevent=:There is one log file per agent family per machine and every surface of that family appends to it, so until now nothing on a line said whether the CLI, the Desktop app or a Cowork session wrote it. The heartbeat knows the surface, but it describes one session, while a log file holds lines from many.
Vocabulary
Closed list, lowercase, no space and no
=. Full table with per-plugin detection: docs/hook-log-format.md.claudecli,desktop,coworkcodexcodex_cli,codex_appantigravityantigravity,antigravity_ide,antigravity_clicursor/copilot/geminicursor/github_copilot/gemini_cliOptional means optional
An undetermined surface omits the whole token — never
surface=, neversurface=unknown, since a reader scanningkey=tokens cannot tell either from a real value. Absent on every line written by an earlier version (never rewritten), and on an antigravity event whose payload has notranscriptPath.One resolution per plugin
Each plugin resolves the surface once, from the signal its heartbeat already uses:
plugins/rogue/scripts/surface.{sh,ps1}— new shared table; hook takes the slug, heartbeat takes the display label, from the samecase(previously duplicated in both).plugins/codex/scripts/surface.{sh,ps1}— one read ofROGUE_CODEX_SURFACE, now validated against the closed list. That validation is new: the value comes from an env file, so an arbitrary string could previously reach a log line and a roster row.transcriptPathresolution it already passed to its heartbeat. Its PowerShell half resolved this after the heartbeat, which would have left a few lines untagged where the sh half tagged them — moved up to match.A second, independent detection path would be worse than no token: a line and the roster row for one session could name different surfaces.
Safety
Every resolution is guarded on both sides. A missing
surface.sh, an unreadable file or a throwing resolver yields an empty slug, which yields no token. Nothing here can block a session, change an allow/deny outcome, or write to stderr. The upload envelope is unchanged — this is a per-line change only.Versions
rogue 1.0.24 · codex 1.0.1 · cursor 1.1.1 · copilot 1.2.1 · antigravity 1.0.24 · gemini 1.0.25
Tests
Slug matrix per plugin, emit position, and "never a placeholder", in all three languages. The claude table is asserted against the heartbeat's own labels so the two cannot drift.
🤖 Generated with Claude Code