Skip to content

docs(send_message): v3 closeout addendum (loose ends fixed + terminal context-bleed discovery) - #93

Closed
Kyzcreig wants to merge 4 commits into
mainfrom
docs/v3-closeout-addendum
Closed

Kyzcreig wants to merge 4 commits into
mainfrom
docs/v3-closeout-addendum

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

Closeout addendum for the v3 cron-session-latch PRD: the two pre-existing test reds were fixed (PR #92, not WONTFIX), and a latent terminal-tool context-bleed was discovered during live re-verification (logged as BACKLOG — not a v3 regression, no routing leak). Docs only.

Apollo added 4 commits June 22, 2026 11:12
…ak was the subagent seam, no separate main-turn bug
…state.db not lcm.db; agent.log fan-out is the load-bearing proof, store-absence corroborating (state.db hygiene trimmed the leak minute)
@github-actions

Copy link
Copy Markdown

🔎 Lint report: docs/v3-closeout-addendum vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11352 on HEAD, 11350 on base (🆕 +2)

🆕 New issues (2):

Rule Count
unresolved-attribute 2
First entries
run_agent.py:2940: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`

✅ Fixed issues (1):

Rule Count
invalid-assignment 1
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`

Unchanged: 5939 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@greptile-apps

greptile-apps Bot commented Jun 22, 2026

Copy link
Copy Markdown

Greptile Summary

This is a docs-only closeout addendum for the v3 cron-session-latch PRD, resolving two loose ends and logging a newly discovered latent issue.

  • v2 OQ2 resolved: The open question about whether the 00:02 leak was a separate main-turn seam (RC#2) is now closed with forensic evidence from agent.log (16-way delegate_task fan-out confirms RC#1/subagent attribution) and state.db (zero send_message calls in the retained #tdd transcript), plus a method note on recovering compacted tool-call arguments from the two stores.
  • v3 closed-out: Status updated to SHIPPED & CLOSED; the two pre-existing test-red classes (test_hardline_blocklist, TestDelegateHeartbeat) were fixed rather than WONTFIX'd (PR test: de-flake hardline (HERMES_ALLOW_REBOOT env leak) + delegate heartbeat (timing brittleness) #92), with root-cause documented.
  • BACKLOG discovery: terminal tool subprocesses can inherit a concurrently-active cron session's full HERMES_SESSION_* block (including SESSION_ID, CHAT_ID, KEY) — a pre-existing context-inheritance issue exposed (not caused) by v3. Documented as non-routing, non-v3, with a false-positive caveat for the Mechanism-4 diagnostic and a promotion trigger.

Confidence Score: 4/5

Docs-only change with no code or configuration modifications; safe to merge with minor documentation accuracy concerns.

The v2 OQ2 resolution is thorough and well-evidenced. The v3 closeout addendum has a PR cross-reference that may be inaccurate (credits PR #91 for the v2 OQ2 correction while the actual diff lives in this PR), and the BACKLOG entry's diagnostic false-positive framing may understate that the KEY bleed is itself a latent auth-boundary issue. Neither concern is blocking, but both could mislead future readers doing forensic trace-backs.

The v3 closeout addendum in PRD-send-message-origin-leak-v3-cron-session-latch.md deserves a second look: the PR #91 reference and the BACKLOG severity framing for the KEY bleed.

Important Files Changed

Filename Overview
docs/specs/PRD-send-message-origin-leak-v2.md OQ2 resolved with detailed forensic evidence (agent.log fan-out + state.db absence proving RC#1-only); method note added for recovering compacted tool-call args from the two stores. No code changes.
docs/specs/PRD-send-message-origin-leak-v3-cron-session-latch.md Status updated to SHIPPED & CLOSED; closeout addendum documents two test-red fixes (PR #92) and a new terminal context-bleed BACKLOG discovery. Contains a PR reference inconsistency (addendum credits PR #91 for the v2 OQ2 correction, but the actual OQ2 change ships in this PR #93). BACKLOG item's severity framing may understate the KEY-bleed surface.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["send_message called"] --> B{Is caller a subagent?}
    B -- No --> C["Read _SESSION_PLATFORM\n(gateway main turn)"]
    B -- Yes --> D["Read _SEND_ORIGIN_*\n(propagated from parent)"]
    C --> E["Route to current channel\n(correct ✅)"]
    D --> F{Parent origin available?}
    F -- Yes --> G["Route to parent origin channel\n(RC#1 fix — PR #83 ✅)"]
    F -- No --> H["Fall to home channel\n+ emit warning tripwire"]

    subgraph "Terminal Context-Bleed (BACKLOG)"
        I["Cron job sets\nHERMES_SESSION_* in os.environ"] --> J["Interactive turn spawns\nterminal subprocess"]
        J --> K["Subprocess inherits cron session's\nSESSION_ID / CHAT_ID / KEY\n⚠️ KEY bleeds too"]
        K --> L["send_message reads model turn context\nnot terminal env → routing still correct ✅"]
        K --> M["terminal env diagnostic gives\nfalse positive ⚠️\nCross-check with execute_code"]
    end

    subgraph "v3 Root Cause (FIXED)"
        N["HERMES_CRON_SESSION set in\nos.environ by scheduler\nnever cleared in finally"] --> O["Main-turn bare send_message\nreads process-global env\n→ thinks it is cron → routes to home ❌"]
        O --> P["Fix: migrate to ContextVar\n_VAR_MAP registration\n(PR #88 squash d12ac4895 ✅)"]
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["send_message called"] --> B{Is caller a subagent?}
    B -- No --> C["Read _SESSION_PLATFORM\n(gateway main turn)"]
    B -- Yes --> D["Read _SEND_ORIGIN_*\n(propagated from parent)"]
    C --> E["Route to current channel\n(correct ✅)"]
    D --> F{Parent origin available?}
    F -- Yes --> G["Route to parent origin channel\n(RC#1 fix — PR #83 ✅)"]
    F -- No --> H["Fall to home channel\n+ emit warning tripwire"]

    subgraph "Terminal Context-Bleed (BACKLOG)"
        I["Cron job sets\nHERMES_SESSION_* in os.environ"] --> J["Interactive turn spawns\nterminal subprocess"]
        J --> K["Subprocess inherits cron session's\nSESSION_ID / CHAT_ID / KEY\n⚠️ KEY bleeds too"]
        K --> L["send_message reads model turn context\nnot terminal env → routing still correct ✅"]
        K --> M["terminal env diagnostic gives\nfalse positive ⚠️\nCross-check with execute_code"]
    end

    subgraph "v3 Root Cause (FIXED)"
        N["HERMES_CRON_SESSION set in\nos.environ by scheduler\nnever cleared in finally"] --> O["Main-turn bare send_message\nreads process-global env\n→ thinks it is cron → routes to home ❌"]
        O --> P["Fix: migrate to ContextVar\n_VAR_MAP registration\n(PR #88 squash d12ac4895 ✅)"]
    end
Loading

Reviews (1): Last reviewed commit: "docs(send_message): v3 closeout addendum..." | Re-trigger Greptile

- The two "pre-existing red" test classes (`test_hardline_blocklist`, `TestDelegateHeartbeat`) were **investigated and fixed**, not WONTFIX'd (PR #92, merged `eb7b4651c`):
- `test_hardline_blocklist` (24 reboot/shutdown cases + yolo-bypass): the live fleet sets `HERMES_ALLOW_REBOOT=1` (legit ops opt-out, `74ed8a85c`), which leaks into pytest's inherited env and downgrades the reboot family out of the hardline floor → the default-block tests fail. Fixed with an autouse `monkeypatch.delenv("HERMES_ALLOW_REBOOT")`. Deterministic in both ambient states.
- `TestDelegateHeartbeat` (3 timing-brittle tests): asserted wall-clock-derived touch counts; the heartbeat thread's wake cadence is GIL/scheduler-bound, so on a loaded host they produced too few touches. Rewrote to assert the **behavior contract** (heartbeat still alive late in the run / ≥1 touch with pinned thresholds) instead of a count. Verified 6/6 green under a 6-thread CPU burn.
- The v2-spec OQ2 docs correction shipped (PR #91).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Cross-reference to PR #91 may be inaccurate

The addendum states "The v2-spec OQ2 docs correction shipped (PR #91)", but the OQ2 section in PRD-send-message-origin-leak-v2.md is also being changed in this PR (#93) — from the old open question to the full "RESOLVED (2026-06-22)" forensic write-up. If PR #91 already made a partial OQ2 update, the addendum should clarify what specifically shipped in #91 vs. what is being added here, so future readers tracing the change history aren't misled about which commit holds the authoritative resolution.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

- `TestDelegateHeartbeat` (3 timing-brittle tests): asserted wall-clock-derived touch counts; the heartbeat thread's wake cadence is GIL/scheduler-bound, so on a loaded host they produced too few touches. Rewrote to assert the **behavior contract** (heartbeat still alive late in the run / ≥1 touch with pinned thresholds) instead of a count. Verified 6/6 green under a 6-thread CPU burn.
- The v2-spec OQ2 docs correction shipped (PR #91).

**DISCOVERY (latent, BACKLOG — NOT a v3 regression, NO routing leak):** the `terminal` tool's subprocess can inherit a *different concurrently-active session's* contextvars (a `no_agent` cron job's `set_cron_session()` bleeds the whole `HERMES_SESSION_*` block — incl. `HERMES_CRON_SESSION=1` and that cron session's `SESSION_ID`/`CHAT_ID`/`KEY` — into an interactive turn's terminal env). It bleeds **every** session var, not just the cron flag, so it's a general context-inheritance issue, pre-existing, exposed (not caused) by adding `HERMES_CRON_SESSION` to `_VAR_MAP`. It does **not** misroute `send_message` (which reads the model's turn context — live-verified routing to the current channel). Practical impact: the Mechanism-4 `terminal env|grep HERMES_CRON_SESSION` diagnostic now gives false positives — **cross-check with the in-process `execute_code` read** (`gateway.session_context.is_cron_session()`), which is authoritative. The `agent-message-misrouting-diagnosis` skill was updated with this caveat. Promotion trigger: if it ever causes a real cross-context terminal action (a cron-spawned terminal command running with interactive creds or vice versa).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Severity of KEY bleed in BACKLOG item may be understated

The discovery section describes the terminal context-bleed as primarily a "diagnostic false-positive" issue, but the bleed includes HERMES_SESSION_KEY (a session auth key) alongside SESSION_ID and CHAT_ID. If any terminal command in an interactive turn inherits a concurrently-active cron session's KEY and uses it to authenticate outbound calls, that is a real cross-context auth boundary violation — not just a false positive in env|grep. The "Promotion trigger" covers the scenario but frames it as a future concern, when the credential cross-contamination is present in the current environment any time a cron job and an interactive terminal command overlap. Worth noting explicitly in the BACKLOG entry that the KEY bleed itself is the latent security surface, even if no routing misuse has been observed.

@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Superseded: the v3 closeout-addendum content this PR adds is already on main (shipped via #88/#91/#92, 2026-06-22 — git show main:docs/specs/PRD-send-message-origin-leak-v3-cron-session-latch.md | grep 'Closeout addendum' is present). Rebasing would resolve to a near-empty diff, so closing rather than merging busywork. Greploop flagged it arm-escalate-human (a real decision: rebase-or-close) — decision = close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant