fix(hooks): pass phase argument from hook ID to observe.sh by Millectable · Pull Request #1042 · affaan-m/ECC

Millectable · 2026-03-31T08:38:47Z

Summary

Fixes run-with-flags-shell.sh to extract the phase prefix from the hook ID (e.g., "pre:observe" → "pre", "post:observe" → "post") and pass it as $1 to the invoked script
Without this fix, observe.sh always defaults to "post", causing all observations to be recorded as tool_complete events — no tool_start events are ever captured

Root Cause

In hooks.json, the PreToolUse observe hook is registered with hook ID "pre:observe" and PostToolUse with "post:observe". The shell wrapper receives this ID as $1 but never extracts the phase and forwards it to the script. The JS wrapper (run-with-flags.js) doesn't have this issue because the JS hook scripts receive the hook ID directly.

Fix

Added phase extraction using ${HOOK_ID%%:*} (bash parameter expansion to get everything before the first colon) and passes it as an argument to the script.

Test plan

All 1695/1696 tests pass (1 pre-existing failure in codex-hooks.test.js unrelated to this change)
Verify observations now capture both tool_start and tool_complete events

Fixes #1018

🤖 Generated with Claude Code

Summary by cubic

Fix the shell hook runner to pass the phase ("pre"/"post") from the hook ID to observe.sh. This restores recording of both tool_start and tool_complete events.

Bug Fixes
- Extract phase from HOOK_ID with ${HOOK_ID%%:*} and pass it as $1 to the script.
- Previously, observe.sh defaulted to "post", recording only tool_complete events.

^{Written for commit 749a11d. Summary will update on new commits.}

Summary by CodeRabbit

Chores
- Enhanced hook processing to improve phase handling during script execution.

The shell wrapper run-with-flags-shell.sh was not extracting the phase prefix from the hook ID (e.g., "pre:observe" -> "pre") and passing it as $1 to the invoked script. This caused observe.sh to always default to "post", recording all observations as tool_complete events with no tool_start events captured. Fixes affaan-m#1018 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-03-31T08:39:03Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c16c2dbc-5941-47fa-9115-42380b2f8403

📥 Commits

Reviewing files that changed from the base of the PR and between f7f91d9 and 749a11d.

📒 Files selected for processing (1)

scripts/hooks/run-with-flags-shell.sh

📝 Walkthrough

Walkthrough

The hook runner now extracts the phase prefix from the HOOK_ID parameter (e.g., pre:observe → pre) and passes it as a command-line argument to the invoked script, fixing a bug where phase information was not forwarded to observe.sh.

Changes

Cohort / File(s)	Summary
Hook Phase Argument Forwarding `scripts/hooks/run-with-flags-shell.sh`	Extracts the phase prefix from `HOOK_ID` and passes it as an additional positional argument to the script, replacing the previous call that only forwarded stdin.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A phase was lost in the midnight call,
Pre and post confused by it all,
Now the colon speaks truth: extract and send,
The phase flies forward, bug on the mend! 🔧

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(hooks): pass phase argument from hook ID to observe.sh' accurately and specifically describes the main change—extracting and passing the phase argument to the observe.sh script.
Linked Issues check	✅ Passed	The pull request fully implements Option B from issue `#1018`: extracting HOOK_PHASE from HOOK_ID using parameter expansion and passing it as $1 to the script, which restores correct phase propagation for both pre and post hook phases.
Out of Scope Changes check	✅ Passed	All changes are scoped to the stated objective: only run-with-flags-shell.sh was modified to extract and forward the phase argument; no unrelated alterations are present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

cubic-dev-ai

No issues found across 1 file

greptile-apps · 2026-03-31T08:40:50Z

Greptile Summary

This PR fixes a one-line bug in scripts/hooks/run-with-flags-shell.sh where the hook phase ("pre" vs "post") was never extracted from the hook ID and forwarded to the invoked script, causing observe.sh to always default to "post" and record every observation as a tool_complete event — silently dropping all tool_start data.

Key changes:

Extracts the phase prefix from HOOK_ID using the POSIX-compatible ${HOOK_ID%%:*} expansion (e.g. "pre:observe" → "pre", "post:observe" → "post")
Passes the extracted phase as positional argument $1 to the script, which observe.sh already consumed via HOOK_PHASE="${1:-post}" — the receiving end was ready, only the sender was missing the argument
The fix is narrowly scoped: run-with-flags-shell.sh is currently only invoked for observe.sh (two entries in hooks/hooks.json), so no other scripts are affected

No issues found. The fix is correct, minimal, and well-commented.

Confidence Score: 5/5

Safe to merge — single-line targeted bug fix with no regressions.

The change is minimal and correct: it fixes a missing argument pass-through using standard POSIX shell parameter expansion. observe.sh already had the ${1:-post} default in place waiting for this argument. The only call sites of run-with-flags-shell.sh in the entire repo are the two observe.sh registrations in hooks/hooks.json, so no other scripts are impacted.

No files require special attention.

Important Files Changed

Filename	Overview
scripts/hooks/run-with-flags-shell.sh	Adds HOOK_PHASE extraction from HOOK_ID using ${HOOK_ID%%:*} and passes it as $1 to the invoked script, fixing the bug where observe.sh always defaulted to 'post' and never captured tool_start events.

Sequence Diagram

sequenceDiagram
    participant CC as Claude Code
    participant RWF as run-with-flags-shell.sh
    participant OBS as observe.sh

    Note over CC,OBS: PreToolUse (before fix)
    CC->>RWF: $1="pre:observe", stdin=JSON
    RWF->>OBS: (no args), stdin=JSON
    OBS->>OBS: HOOK_PHASE="${1:-post}" → "post"
    OBS-->>CC: records tool_complete ❌

    Note over CC,OBS: PreToolUse (after fix)
    CC->>RWF: $1="pre:observe", stdin=JSON
    RWF->>RWF: HOOK_PHASE="${HOOK_ID%%:*}" → "pre"
    RWF->>OBS: $1="pre", stdin=JSON
    OBS->>OBS: HOOK_PHASE="${1:-post}" → "pre"
    OBS-->>CC: records tool_start ✅

    Note over CC,OBS: PostToolUse (unchanged behavior)
    CC->>RWF: $1="post:observe", stdin=JSON
    RWF->>RWF: HOOK_PHASE="${HOOK_ID%%:*}" → "post"
    RWF->>OBS: $1="post", stdin=JSON
    OBS->>OBS: HOOK_PHASE="${1:-post}" → "post"
    OBS-->>CC: records tool_complete ✅

_{Reviews (1): Last reviewed commit: "fix(hooks): pass phase argument from hoo..." | Re-trigger Greptile}

…1042) The shell wrapper run-with-flags-shell.sh was not extracting the phase prefix from the hook ID (e.g., "pre:observe" -> "pre") and passing it as $1 to the invoked script. This caused observe.sh to always default to "post", recording all observations as tool_complete events with no tool_start events captured. Fixes affaan-m#1018 Co-authored-by: Millectable <noreply@github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cubic-dev-ai Bot reviewed Mar 31, 2026

View reviewed changes

affaan-m merged commit 5596159 into affaan-m:main Mar 31, 2026
4 checks passed

coderabbitai Bot mentioned this pull request Apr 24, 2026

fix: derive clv2 hook phase from event env #1568

Closed

coderabbitai Bot mentioned this pull request May 11, 2026

fix: salvage remaining stale queue fixes #1754

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(hooks): pass phase argument from hook ID to observe.sh#1042

fix(hooks): pass phase argument from hook ID to observe.sh#1042
affaan-m merged 1 commit into
affaan-m:mainfrom
Millectable:fix/observe-phase-argument

Millectable commented Mar 31, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 31, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

greptile-apps Bot commented Mar 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Millectable commented Mar 31, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Test plan

Summary by cubic

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Mar 31, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Millectable commented Mar 31, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 31, 2026 •

edited

Loading