feat(hooks): complete governance hook-event coverage — context injection + subagent/compact/failure/notification observability (#255)#258
Open
richard-devbot wants to merge 8 commits into
Conversation
…tack-awareness (#255) New framework-neutral context injector for UserPromptSubmit + SessionStart hooks. Reads the hook JSON on stdin, resolves the active run, and emits a small structural packet — active run id + current stage, count of pending approvals + open decisions, and a static orchestrator pointer — in Claude Code's {"hookSpecificOutput":{...,"additionalContext":"..."}} shape on stdout. Contract mirrors observe: ALWAYS exit 0 (context hooks can't deny), never throws, injects nothing (no stdout) when there is no active run, and never injects secrets — the packet is built only from ids/stage/integer counts we generate, never from prompt text, tool inputs, or decision question text, so there is no channel for a credential to reach the model. Injected string is capped at ~1KB. Wired into bin/rstack-agents.js. Closes the Pi-only "orchestrator packet injection" gap for every harness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… events (#255) Extend normalizeObservation's hook_event_name vocabulary (same exit-0 / no-op / redaction contract): - SubagentStart -> subagent_started { agent_type } - SubagentStop -> subagent_stopped { agent_type } (distinct from session_shutdown — a delegated builder/validator finishing is its own signal; SubagentStop previously collapsed to session_shutdown) - PreCompact -> context_preserved { trigger } - PostToolUseFailure -> tool_result { isError:true } (defaults to error even without an explicit is_error flag; checked before the tool_response inference so it isn't short-circuited) agent_type/trigger are secret-redacted and length-bounded like every other value. Existing tool_call/tool_result/session_shutdown mappings unchanged. The dashboard filters by known types, so the new events append to the ledger without disturbing any rollup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New framework-neutral relay for the host Notification hook: reads the payload on
stdin (Claude Code {message,title}) or --message/--title flags and forwards it to
every configured RStack channel via the existing notifications/router.js
(notifyAll — Slack/Teams/Discord/Telegram/WhatsApp).
Contract: ALWAYS exit 0, never throws, silent no-op when no channels are
configured (no parse, no network), and secret-redacted + truncated — it forwards
only the host's short message/title, never tool inputs or file contents. This is
the one hook that makes a network call; notifyAll is fire-and-forget with
per-channel error capture and a bounded timeout, so a slow/failing webhook can
never disrupt the session. Wired into bin/rstack-agents.js.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…#255) Extend the Claude Code hook set init installs (idempotent; guard + existing observe entries intact): - SessionStart: hub launcher + `rstack-agents context` (two hooks) - UserPromptSubmit: `rstack-agents context` - PostToolUseFailure / SubagentStart / SubagentStop / PreCompact: `rstack-agents observe` - Notification: `rstack-agents notify-hook` PreToolUse (guard) is the only hook that can block, unchanged. Updated the report labels + next-steps, the .claude/rstack-sdlc.md doc (context / observability / notifications sections), and the pinned-shape test to assert every event → command mapping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…#255) Register the Tau events that map to the new coverage: - before_compaction -> context_preserved { trigger } (fire-and-forget) - tool_execution_failure -> error tool_result (fire-and-forget) - before_agent_start -> context injection: fetch the RStack packet via `rstack-agents context` and prepend it to the turn's system prompt (BeforeAgentStartEventResult). Best-effort, hard-timeout-bounded, returns no override on any failure — can never block a turn. Documented the two events Tau does NOT expose: no delegated-SUBAGENT lifecycle (agent_start/end are the per-prompt loop, not spawned specialists) and no NOTIFICATION event (Tau surfaces messages via its own TUI). Verified event names and result types against the Tau hooks package. Existing tool_call/tool_result wiring unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extend the wiring checks:
- claude-code: new "context hook" (UserPromptSubmit/SessionStart → context) and
"notification hook" (Notification → notify-hook) checks. WARN not FAIL —
additive, like the observe check.
- tau: new "tau context hook" check (before_agent_start → context injection)
alongside the existing observability check.
All new checks carry a fix/hint. Added doctor tests for the full claude-code hook
set and the tau context wiring.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- docs/integrations/claude-code.md: full hook-map table (every event → command → what it does / what it never does), plus Context injection and Notifications sections; updated the settings.json block, the "Pi-only" note (context is no longer Pi-only), and the observe event vocabulary. - docs/HARNESS.md: new "Hook system" section — the four verbs, their contract (only guard blocks; all others exit 0 / no-op / redact), the observe event vocabulary, the context packet's structural-only guarantee, the notify relay, and the Tau coverage + the two events Tau lacks. - docs/integrations/tau.md: observability/context/notifications section. - README.md: guard/observe/context/notify-hook CLI rows + the Claude Code hook rows in the hooks table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
--source is still accepted on the CLI for hook-command symmetry, but the context packet describes the run, not the harness, so it isn't consumed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
📝 WalkthroughWalkthroughAdds ChangesFull hook-event coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/commands/context.js (1)
172-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
readStdinTextto a shared utility.This function and the
MAX_STDIN_BYTESconstant are duplicated identically insrc/commands/notify-hook.js. Extracting to a shared module (e.g.,src/utils/stdin.js) eliminates the copy and ensures the cap stays consistent across commands.♻️ Suggested extraction
+// src/utils/stdin.js +export const MAX_STDIN_BYTES = 1_000_000; + +export async function readStdinText(stream = process.stdin) { + if (stream.isTTY) return ''; + let data = ''; + stream.setEncoding('utf8'); + for await (const chunk of stream) { + data += chunk; + if (data.length >= MAX_STDIN_BYTES) return data.slice(0, MAX_STDIN_BYTES); + } + return data; +}Then in both
context.jsandnotify-hook.js:-import { readStdinText } from './utils/stdin.js'; +// remove the local readStdinText + MAX_STDIN_BYTES definitions🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/context.js` around lines 172 - 183, The readStdinText helper and MAX_STDIN_BYTES cap are duplicated in context.js and notify-hook.js, so move them into a shared utility module and have both commands import the same implementation. Create a reusable stdin helper (for example, a shared utility for readStdinText and the cap constant) and update the existing readStdinText references in context.js and notify-hook.js to use it so the behavior stays consistent in one place.src/integrations/init.js (1)
295-346: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting
GUARD_CMDandHUB_CMDconstants for consistency.
OBSERVE_CMD,CONTEXT_CMD, andNOTIFY_CMDare extracted as named constants, but the hub command (line 325,'npx -y rstack-agents hub') and guard command (line 331,'npx --yes rstack-agents guard --context builder') are inline strings. Extracting them would improve consistency and make future command changes a single-point edit. The-yvs--yesdifference between hub and all other commands is intentional (tested at line 181 of the test file), but consolidating to--yeswould reduce visual noise.♻️ Optional refactor
const OBSERVE_CMD = 'npx --yes rstack-agents observe --source claude-code'; const CONTEXT_CMD = 'npx --yes rstack-agents context --source claude-code'; const NOTIFY_CMD = 'npx --yes rstack-agents notify-hook --source claude-code'; +const HUB_CMD = 'npx -y rstack-agents hub'; +const GUARD_CMD = 'npx --yes rstack-agents guard --context builder'; export const CLAUDE_CODE_HOOKS = Object.freeze({ hooks: { SessionStart: [ - { hooks: [{ type: 'command', command: 'npx -y rstack-agents hub' }] }, + { hooks: [{ type: 'command', command: HUB_CMD }] }, { hooks: [{ type: 'command', command: CONTEXT_CMD }] }, ], UserPromptSubmit: [{ hooks: [{ type: 'command', command: CONTEXT_CMD }] }], PreToolUse: [{ matcher: 'Bash|Write|Edit', - hooks: [{ type: 'command', command: 'npx --yes rstack-agents guard --context builder' }], + hooks: [{ type: 'command', command: GUARD_CMD }], }],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/init.js` around lines 295 - 346, The inline hub and guard commands in CLAUDE_CODE_HOOKS are inconsistent with the existing extracted command constants, making future edits harder. Add named constants like HUB_CMD and GUARD_CMD alongside OBSERVE_CMD, CONTEXT_CMD, and NOTIFY_CMD, then replace the inline strings in SessionStart and PreToolUse with those constants. Keep the current hub invocation flag style intact unless you intend to update the related test coverage too.tests/observe-cli.test.js (1)
296-303: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a
triggerredaction test for parity withagent_type.Both
agent_typeandtriggerpass throughredactSecretsinnormalizeObservation(src/commands/observe.js:209,218), but onlyagent_typeredaction is tested end-to-end. APreCompactpayload with a secret-lookingtriggerwould close the coverage gap.🛡️ Suggested additional test
test('observe: a secret-looking agent_type / trigger is redacted', async () => { const { root, runDir } = seedProject(); await runObserve(['--source', 'claude-code', '--project', root], { input: JSON.stringify({ hook_event_name: 'SubagentStart', agent_type: 'token=AKIAIOSFODNN7EXAMPLE123' }) }); const raw = readFileSync(join(runDir, 'events.jsonl'), 'utf8'); assert.ok(!raw.includes('AKIAIOSFODNN7EXAMPLE123'), 'secret in agent_type redacted'); + + const { root: root2, runDir: runDir2 } = seedProject(); + await runObserve(['--source', 'claude-code', '--project', root2], + { input: JSON.stringify({ hook_event_name: 'PreCompact', trigger: 'token=AKIAIOSFODNN7EXAMPLE456' }) }); + const raw2 = readFileSync(join(runDir2, 'events.jsonl'), 'utf8'); + assert.ok(!raw2.includes('AKIAIOSFODNN7EXAMPLE456'), 'secret in trigger redacted'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/observe-cli.test.js` around lines 296 - 303, Add an end-to-end observe CLI test for trigger redaction to match the existing agent_type coverage. Extend the tests in observe-cli.test.js by adding a case that sends a PreCompact payload with a secret-looking trigger through runObserve, then assert the recorded events.jsonl does not contain the secret value. Use normalizeObservation and redactSecrets as the relevant behavior being exercised, and keep the new test alongside the existing secret-redaction test for agent_type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/notify-hook.js`:
- Around line 84-87: The notification payload and verbose error handling are
leaking unredacted strings outside the process. Update buildNotifyPayload and
the related catch/verbose logging paths in notify-hook.js so every outbound
string goes through the same redaction/truncation flow before being returned,
logged, or printed, including source and any raw error messages. Use the
existing notify-hook helpers and symbols around buildNotifyPayload and the catch
path to ensure no verbatim external text can escape.
- Around line 120-121: The notify-hook fan-out result currently always returns
reason as relayed even when every sender fails, which leads to misleading
output. Update the return logic in notifyAll/notify-hook so the reason reflects
success only when at least one sender succeeds, and uses a failure-specific
reason when results.some((r) => r.ok) is false. Use the existing notifyAll call
and the returned results array to determine both notified and reason
consistently.
In `@src/integrations/tau/rstack_sdlc.py`:
- Around line 424-455: The timeout handling in _fetch_context leaves the
subprocess alive when asyncio.wait_for cancels communicate(), so update the
subprocess lifecycle there to explicitly terminate or kill proc on timeout
before returning "". Use the existing async subprocess flow in _fetch_context,
and make sure any timeout or exception path cleans up the npx/node process so
repeated before_agent_start calls do not accumulate orphaned processes.
- Line 76: Use the camelCase field when constructing the
BeforeAgentStartEventResult in rstack_sdlc so the RStack context is actually
passed through; replace the current system_prompt usage with systemPrompt in the
code path that builds the result object, and verify the event payload still
includes the expected ToolCallEventResult handling.
In `@tests/notify-hook.test.js`:
- Around line 44-52: The no-channel notify-hook tests are using a shared temp
root, so stale config can leak into `hasConfiguredChannels({ projectRoot })` and
make the assertions flaky. Update the `notify-hook` test setup to create an
isolated temporary project root per test case instead of passing `tmpdir()`
directly, and use that per-test root when calling `runCli` for the no-channel
and malformed-stdin cases. Keep the tests’ expectations the same, but ensure
each test starts from a clean `projectRoot` so `hasConfiguredChannels` cannot
see unrelated config.
---
Nitpick comments:
In `@src/commands/context.js`:
- Around line 172-183: The readStdinText helper and MAX_STDIN_BYTES cap are
duplicated in context.js and notify-hook.js, so move them into a shared utility
module and have both commands import the same implementation. Create a reusable
stdin helper (for example, a shared utility for readStdinText and the cap
constant) and update the existing readStdinText references in context.js and
notify-hook.js to use it so the behavior stays consistent in one place.
In `@src/integrations/init.js`:
- Around line 295-346: The inline hub and guard commands in CLAUDE_CODE_HOOKS
are inconsistent with the existing extracted command constants, making future
edits harder. Add named constants like HUB_CMD and GUARD_CMD alongside
OBSERVE_CMD, CONTEXT_CMD, and NOTIFY_CMD, then replace the inline strings in
SessionStart and PreToolUse with those constants. Keep the current hub
invocation flag style intact unless you intend to update the related test
coverage too.
In `@tests/observe-cli.test.js`:
- Around line 296-303: Add an end-to-end observe CLI test for trigger redaction
to match the existing agent_type coverage. Extend the tests in
observe-cli.test.js by adding a case that sends a PreCompact payload with a
secret-looking trigger through runObserve, then assert the recorded events.jsonl
does not contain the secret value. Use normalizeObservation and redactSecrets as
the relevant behavior being exercised, and keep the new test alongside the
existing secret-redaction test for agent_type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a11ed15-4a70-4d4b-af48-830833fd4642
📒 Files selected for processing (16)
README.mdbin/rstack-agents.jsdocs/HARNESS.mddocs/integrations/claude-code.mddocs/integrations/tau.mdsrc/commands/context.jssrc/commands/doctor.jssrc/commands/notify-hook.jssrc/commands/observe.jssrc/integrations/init.jssrc/integrations/tau/rstack_sdlc.pytests/context-cli.test.jstests/doctor.test.jstests/integrations-init.test.jstests/notify-hook.test.jstests/observe-cli.test.js
Comment on lines
+84
to
+87
| export function buildNotifyPayload({ message, title }, source) { | ||
| const header = title ? `*RStack · ${title}*` : '*RStack notification*'; | ||
| const src = source ? ` _(${source})_` : ''; | ||
| return { text: `${header}${src}\n${message}` }; |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact every string that can leave the process.
source is relayed verbatim, and the catch path can print raw error messages under --verbose. Both bypass the redaction/truncation contract.
Proposed fix
export function buildNotifyPayload({ message, title }, source) {
- const header = title ? `*RStack · ${title}*` : '*RStack notification*';
- const src = source ? ` _(${source})_` : '';
- return { text: `${header}${src}\n${message}` };
+ const headerTitle = title ? safeText(title, 120) : null;
+ const header = headerTitle ? `*RStack · ${headerTitle}*` : '*RStack notification*';
+ const safeSource = source ? safeText(source, 120) : '';
+ const src = safeSource ? ` _(${safeSource})_` : '';
+ return { text: `${header}${src}\n${safeText(message)}` };
}
...
} catch (error) {
- return { notified: false, reason: `notify failed (ignored): ${error?.message ?? error}` };
+ return { notified: false, reason: `notify failed (ignored): ${safeText(error?.message ?? error, 200)}` };
}
}Also applies to: 122-123, 154-155
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/notify-hook.js` around lines 84 - 87, The notification payload
and verbose error handling are leaking unredacted strings outside the process.
Update buildNotifyPayload and the related catch/verbose logging paths in
notify-hook.js so every outbound string goes through the same
redaction/truncation flow before being returned, logged, or printed, including
source and any raw error messages. Use the existing notify-hook helpers and
symbols around buildNotifyPayload and the catch path to ensure no verbatim
external text can escape.
Comment on lines
+120
to
+121
| const results = await notifyAll(payload, { projectRoot, env, ...(senders ? { senders } : {}) }); | ||
| return { notified: results.some((r) => r.ok), reason: 'relayed', results }; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid reporting failed fan-out as relayed.
When every sender fails, notified is false but reason is still 'relayed', producing confusing verbose output like skipped: relayed.
Proposed fix
const payload = buildNotifyPayload(notification, source);
// notifyAll is already fire-and-forget with per-channel error capture and
// never throws; we still guard the call.
const results = await notifyAll(payload, { projectRoot, env, ...(senders ? { senders } : {}) });
- return { notified: results.some((r) => r.ok), reason: 'relayed', results };
+ const notified = results.some((r) => r.ok);
+ return {
+ notified,
+ reason: notified ? 'relayed' : 'all configured notification channels failed',
+ results,
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const results = await notifyAll(payload, { projectRoot, env, ...(senders ? { senders } : {}) }); | |
| return { notified: results.some((r) => r.ok), reason: 'relayed', results }; | |
| const results = await notifyAll(payload, { projectRoot, env, ...(senders ? { senders } : {}) }); | |
| const notified = results.some((r) => r.ok); | |
| return { | |
| notified, | |
| reason: notified ? 'relayed' : 'all configured notification channels failed', | |
| results, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/notify-hook.js` around lines 120 - 121, The notify-hook fan-out
result currently always returns reason as relayed even when every sender fails,
which leads to misleading output. Update the return logic in
notifyAll/notify-hook so the reason reflects success only when at least one
sender succeeds, and uses a failure-specific reason when results.some((r) =>
r.ok) is false. Use the existing notifyAll call and the returned results array
to determine both notified and reason consistently.
| from pydantic import BaseModel, Field | ||
|
|
||
| from tau.hooks import ToolCallEventResult | ||
| from tau.hooks import BeforeAgentStartEventResult, ToolCallEventResult |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify BeforeAgentStartEventResult is exported from tau.hooks and its constructor signature.
# Find the tau package installation
fd -t f 'hooks' $(python -c "import tau; print(tau.__path__[0])" 2>/dev/null) 2>/dev/null || \
fd -t f -i 'hooks.py' $(python -c "import tau; print(tau.__path__[0])" 2>/dev/null) 2>/dev/null
# Search for the class definition and its __init__/constructor
rg -n 'class BeforeAgentStartEventResult' $(python -c "import tau; print(tau.__path__[0])" 2>/dev/null)
rg -n 'system_prompt' $(python -c "import tau; print(tau.__path__[0])" 2>/dev/null) -g '*.py'Repository: richard-devbot/SDLC-rstack
Length of output: 231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the integration file and nearby Tau hook usage.
git ls-files src/integrations/tau/rstack_sdlc.py
wc -l src/integrations/tau/rstack_sdlc.py
cat -n src/integrations/tau/rstack_sdlc.py | sed -n '1,220p'
echo "---- search for before_agent_start / BeforeAgentStartEventResult ----"
rg -n "before_agent_start|BeforeAgentStartEventResult|ToolCallEventResult|system_prompt" src docs .Repository: richard-devbot/SDLC-rstack
Length of output: 19352
🌐 Web query:
Tau before_agent_start hook BeforeAgentStartEventResult system_prompt tau.hooks
💡 Result:
The before_agent_start hook is an event within agent-based frameworks—notably those inspired by or related to the Pi coding agent architecture (such as OpenClaw and various Pi-derived agents)—that executes after a user submits a prompt but before the agent loop begins [1][2][3][4]. Purpose and Functionality: The primary purpose of this hook is to allow plugins or extensions to inspect, append, or modify the context (including system prompts) before the LLM processes the turn [5][6][7]. Key Characteristics: - Event Timing: It triggers after user input is received and potentially expanded, but before the agent starts executing its core loop (LLM calls, tool execution) [2][8][3]. - System Prompt Interaction: Extensions can typically access the current system prompt through the event and return a replacement or modified system prompt [6][8][7]. If multiple extensions register handlers for this hook, the system prompt is often chained across them [8][9][7]. - Context Injection: Beyond modifying the system prompt, this hook is frequently used to inject persistent messages into the session history, which are often visible in terminal user interfaces (TUI) [2][6][3]. Technical Implementation: - Event Data (BeforeAgentStartEvent): Typically includes the user's prompt text, any attached images, and the current system prompt string [8][9][3]. - Result Type (BeforeAgentStartEventResult): Usually defines optional fields such as: - message: A custom message to be injected into the session context [6][8][3]. - systemPrompt: A string used to replace or update the system prompt for the current turn [6][8][7]. While some modern implementations (such as OpenClaw) encourage using more granular hooks like before_model_resolve or before_prompt_build for specific tasks, before_agent_start is often maintained for legacy compatibility or as a convenient combined-phase hook [10][1]. Note on Terminology: There are multiple distinct projects referred to as Tau (e.g., dpc's Unix-native coding agent, the Hugging Face Tau coding agent, and others) [11][4][12]. The specific terminology before_agent_start and BeforeAgentStartEventResult is most strongly associated with the Pi-derived coding agent ecosystem, which has influenced several of these projects [2][7][11].
Citations:
- 1: https://docs.openclaw.ai/concepts/agent-loop
- 2:
before_agent_starthook event, allowing to inject SystemMessage in addition to user prompt. earendil-works/pi#324 - 3: https://github.com/can1357/oh-my-pi/blob/main/packages/coding-agent/src/extensibility/hooks/types.ts
- 4: https://tau-agent.dev/
- 5: https://github.com/badlogic/pi-mono/blob/137547a4/packages/agent/docs/hooks.md
- 6: https://cdn.jsdelivr.net/npm/@josephyoung/pi-file-reference@0.1.7/docs/system-prompt-injection.md
- 7: Allow extensions to modify system prompt in before_agent_start earendil-works/pi#575
- 8: https://cdn.jsdelivr.net/npm/@mariozechner/pi-coding-agent@0.63.2/dist/core/extensions/types.d.ts
- 9: https://cdn.jsdelivr.net/npm/open-multi-agent-kit@0.80.6/dist/core/extensions/types.d.ts
- 10: https://docs2.openclaw.ai/plugins/hooks
- 11: https://github.com/tau-agent/tau
- 12: https://github.com/huggingface/tau
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- tau integration docs ----"
wc -l docs/integrations/tau.md docs/HARNESS.md
sed -n '80,120p' docs/integrations/tau.md
echo "---- harness docs around tau ----"
sed -n '136,156p' docs/HARNESS.md
echo "---- tau dependency references ----"
rg -n '"tau"|tau[<=>~ ]|BeforeAgentStartEventResult|system_prompt|systemPrompt' pyproject.toml requirements*.txt package*.json uv.lock poetry.lock .Repository: richard-devbot/SDLC-rstack
Length of output: 11965
🌐 Web query:
site:github.com/tau-agent/tau BeforeAgentStartEventResult system_prompt tau.hooks
💡 Result:
BeforeAgentStartEventResult uses systemPrompt (full replacement), not systemPromptAppend. The event also exposes systemPrompt and systemPromptOptions so handlers can inspect/chain the current prompt. [1][2]
If you want, I can also point to the exact tau.hooks file/path that defines it.
Use systemPrompt here BeforeAgentStartEventResult uses the camelCase field, so system_prompt will be ignored and the RStack context won’t reach the turn prompt.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/integrations/tau/rstack_sdlc.py` at line 76, Use the camelCase field when
constructing the BeforeAgentStartEventResult in rstack_sdlc so the RStack
context is actually passed through; replace the current system_prompt usage with
systemPrompt in the code path that builds the result object, and verify the
event payload still includes the expected ToolCallEventResult handling.
Comment on lines
+424
to
+455
| async def _fetch_context(cwd: str) -> str: | ||
| """Fetch the RStack context packet via `rstack-agents context` (#255). | ||
|
|
||
| Returns the `additionalContext` string, or "" when there is no active run, | ||
| no context, or anything goes wrong. Best-effort and hard-timeout-bounded: | ||
| this runs on the before_agent_start critical path, so it must never block a | ||
| turn — every failure path returns "" and the turn proceeds unchanged. | ||
| """ | ||
| npx = shutil.which("npx") | ||
| if npx is None: | ||
| return "" | ||
| try: | ||
| proc = await asyncio.create_subprocess_exec( | ||
| npx, "--yes", "rstack-agents", "context", "--source", "tau", "--project", cwd, | ||
| stdin=asyncio.subprocess.DEVNULL, | ||
| stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.DEVNULL, | ||
| env=os.environ, cwd=cwd, | ||
| ) | ||
| out, _ = await asyncio.wait_for(proc.communicate(), timeout=5.0) | ||
| except Exception: | ||
| return "" # context is additive — never let it break or delay a turn | ||
| text = out.decode("utf-8", "replace").strip() | ||
| if not text: | ||
| return "" | ||
| try: | ||
| data = json.loads(text) | ||
| ctx = data.get("hookSpecificOutput", {}).get("additionalContext", "") | ||
| return str(ctx) if ctx else "" | ||
| except Exception: | ||
| return "" | ||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Kill the subprocess on timeout to prevent orphaned process accumulation.
When asyncio.wait_for times out, communicate() is cancelled but proc is not killed. The subprocess lingers indefinitely. Since _fetch_context runs on the before_agent_start critical path (every turn), repeated timeouts — e.g., a slow first npx install — will accumulate orphaned npx/node processes and exhaust resources over a long session.
🔒 Proposed fix
async def _fetch_context(cwd: str) -> str:
"""Fetch the RStack context packet via `rstack-agents context` (`#255`).
Returns the `additionalContext` string, or "" when there is no active run,
no context, or anything goes wrong. Best-effort and hard-timeout-bounded:
this runs on the before_agent_start critical path, so it must never block a
turn — every failure path returns "" and the turn proceeds unchanged.
"""
npx = shutil.which("npx")
if npx is None:
return ""
+ proc = None
try:
proc = await asyncio.create_subprocess_exec(
npx, "--yes", "rstack-agents", "context", "--source", "tau", "--project", cwd,
stdin=asyncio.subprocess.DEVNULL,
stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.DEVNULL,
env=os.environ, cwd=cwd,
)
out, _ = await asyncio.wait_for(proc.communicate(), timeout=5.0)
except Exception:
+ if proc is not None:
+ try:
+ proc.kill()
+ except ProcessLookupError:
+ pass
return "" # context is additive — never let it break or delay a turn📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async def _fetch_context(cwd: str) -> str: | |
| """Fetch the RStack context packet via `rstack-agents context` (#255). | |
| Returns the `additionalContext` string, or "" when there is no active run, | |
| no context, or anything goes wrong. Best-effort and hard-timeout-bounded: | |
| this runs on the before_agent_start critical path, so it must never block a | |
| turn — every failure path returns "" and the turn proceeds unchanged. | |
| """ | |
| npx = shutil.which("npx") | |
| if npx is None: | |
| return "" | |
| try: | |
| proc = await asyncio.create_subprocess_exec( | |
| npx, "--yes", "rstack-agents", "context", "--source", "tau", "--project", cwd, | |
| stdin=asyncio.subprocess.DEVNULL, | |
| stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.DEVNULL, | |
| env=os.environ, cwd=cwd, | |
| ) | |
| out, _ = await asyncio.wait_for(proc.communicate(), timeout=5.0) | |
| except Exception: | |
| return "" # context is additive — never let it break or delay a turn | |
| text = out.decode("utf-8", "replace").strip() | |
| if not text: | |
| return "" | |
| try: | |
| data = json.loads(text) | |
| ctx = data.get("hookSpecificOutput", {}).get("additionalContext", "") | |
| return str(ctx) if ctx else "" | |
| except Exception: | |
| return "" | |
| async def _fetch_context(cwd: str) -> str: | |
| """Fetch the RStack context packet via `rstack-agents context` (`#255`). | |
| Returns the `additionalContext` string, or "" when there is no active run, | |
| no context, or anything goes wrong. Best-effort and hard-timeout-bounded: | |
| this runs on the before_agent_start critical path, so it must never block a | |
| turn — every failure path returns "" and the turn proceeds unchanged. | |
| """ | |
| npx = shutil.which("npx") | |
| if npx is None: | |
| return "" | |
| proc = None | |
| try: | |
| proc = await asyncio.create_subprocess_exec( | |
| npx, "--yes", "rstack-agents", "context", "--source", "tau", "--project", cwd, | |
| stdin=asyncio.subprocess.DEVNULL, | |
| stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.DEVNULL, | |
| env=os.environ, cwd=cwd, | |
| ) | |
| out, _ = await asyncio.wait_for(proc.communicate(), timeout=5.0) | |
| except Exception: | |
| if proc is not None: | |
| try: | |
| proc.kill() | |
| except ProcessLookupError: | |
| pass | |
| return "" # context is additive — never let it break or delay a turn | |
| text = out.decode("utf-8", "replace").strip() | |
| if not text: | |
| return "" | |
| try: | |
| data = json.loads(text) | |
| ctx = data.get("hookSpecificOutput", {}).get("additionalContext", "") | |
| return str(ctx) if ctx else "" | |
| except Exception: | |
| return "" |
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 443-443: Do not catch blind exception: Exception
(BLE001)
[warning] 452-452: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/integrations/tau/rstack_sdlc.py` around lines 424 - 455, The timeout
handling in _fetch_context leaves the subprocess alive when asyncio.wait_for
cancels communicate(), so update the subprocess lifecycle there to explicitly
terminate or kill proc on timeout before returning "". Use the existing async
subprocess flow in _fetch_context, and make sure any timeout or exception path
cleans up the npx/node process so repeated before_agent_start calls do not
accumulate orphaned processes.
Comment on lines
+44
to
+52
| test('notify-hook CLI: no channels configured is a silent no-op, exit 0', async () => { | ||
| const payload = JSON.stringify({ hook_event_name: 'Notification', message: 'Claude needs your input' }); | ||
| const { code, stdout } = await runCli(['--project', tmpdir()], { input: payload }); | ||
| assert.equal(code, 0, 'always exits 0'); | ||
| assert.equal(stdout.trim(), '', 'no output when there is nothing to relay'); | ||
| }); | ||
|
|
||
| test('notify-hook CLI: malformed stdin still exits 0 (never throws)', async () => { | ||
| const { code, stderr } = await runCli(['--project', tmpdir()], { input: 'not json }{' }); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Use isolated temp project roots for no-channel tests.
Passing tmpdir() directly means hasConfiguredChannels({ projectRoot }) can see stale config under the shared system temp directory, making no-channel assertions environment-dependent.
Proposed fix
import { spawn } from 'node:child_process';
+import { mkdtemp } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { dirname, resolve } from 'node:path';
...
const BIN = resolve(dirname(fileURLToPath(import.meta.url)), '..', 'bin', 'rstack-agents.js');
+const tempProject = () => mkdtemp(resolve(tmpdir(), 'rstack-notify-'));
...
test('notify-hook CLI: no channels configured is a silent no-op, exit 0', async () => {
const payload = JSON.stringify({ hook_event_name: 'Notification', message: 'Claude needs your input' });
- const { code, stdout } = await runCli(['--project', tmpdir()], { input: payload });
+ const { code, stdout } = await runCli(['--project', await tempProject()], { input: payload });
...
test('notify-hook CLI: malformed stdin still exits 0 (never throws)', async () => {
- const { code, stderr } = await runCli(['--project', tmpdir()], { input: 'not json }{' });
+ const { code, stderr } = await runCli(['--project', await tempProject()], { input: 'not json }{' });
...
const result = await runNotifyHook({
- stdinText: JSON.stringify({ message: 'hello' }), project: tmpdir(), env: cleanEnv(), senders,
+ stdinText: JSON.stringify({ message: 'hello' }), project: await tempProject(), env: cleanEnv(), senders,
...
const result = await runNotifyHook({
- stdinText: JSON.stringify({ message: 'hi' }), project: tmpdir(), env, senders,
+ stdinText: JSON.stringify({ message: 'hi' }), project: await tempProject(), env, senders,Also applies to: 102-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/notify-hook.test.js` around lines 44 - 52, The no-channel notify-hook
tests are using a shared temp root, so stale config can leak into
`hasConfiguredChannels({ projectRoot })` and make the assertions flaky. Update
the `notify-hook` test setup to create an isolated temporary project root per
test case instead of passing `tmpdir()` directly, and use that per-test root
when calling `runCli` for the no-channel and malformed-stdin cases. Keep the
tests’ expectations the same, but ensure each test starts from a clean
`projectRoot` so `hasConfiguredChannels` cannot see unrelated config.
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.
Closes #255. First slice of the full hook-parity wave (ref suite: 14-event production hooks). RStack now covers every governance-relevant Claude Code hook event, and Tau's real event surface, with a coherent CLI (guard / observe / context / notify-hook) instead of loose scripts.
Commits (bisected):
rstack-agents context— UserPromptSubmit + SessionStart inject a ≤1KB RStack-awareness packet (active run + stage, pending approvals/decisions counts, orchestrator pointer) viahookSpecificOutput.additionalContext. Closes the Pi-only "orchestrator packet injection" gap: any harness agent now knows the governed state. No active run → emits NOTHING (no malformed context). Built only from RStack-generated ids/counts — never echoes prompt text, tool inputs, or decision text.subagent_started/stopped, PreCompact →context_preserved, PostToolUseFailure → errortool_result. Same contract: always exit 0, no-op without a run, redaction + truncation reused.rstack-agents notify-hook— Notification events relay to configured channels vianotifyAll(no channels → pre-network no-op; fire-and-forget, bounded).CLAUDE_CODE_HOOKSmap (11 event entries; guard + existing observe intact; idempotent, never edits an existing settings.json).tool_execution_failure,before_compaction, and context injection onbefore_agent_start(timeout-bounded, "" on failure); verified against installed Tau — no subagent/notification events exist there (documented honestly).Session-safety: every new verb ALWAYS exits 0 (only guard may exit 2); double-wrapped try/catch; 1MB stdin caps; ≤1KB injection; Tau context hard-timeout 5s → "".
Gates: typecheck, 881/881 tests (+31), lint, validate (196), security-audit, diff-check, Tau AST — all green.
Adversarial review runs on this PR before merge (session-embedded surface).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation