Refactor claude-notify AGENTS guidance into linked docs#8
Conversation
- reduce root AGENTS.md to scope, quick reference, and critical overrides - add module structure, build/test quality gates, and maintenance docs under docs/agent-instructions - preserve existing constraints while applying progressive disclosure for agent instructions
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDocs for hooks/claude-notify were split into three focused agent-instruction files and AGENTS.md was narrowed to module-scoped guidance with links; build/test and quality-gate rules (docstring coverage, required case IDs) were added as docs; E010 e2e test now uses a fake-origin executable with stronger fallback-run verification; main.swift gains a test hook to force a pre-authorization post failure. Changes
Sequence Diagram(s)(Skipped — changes are documentation plus a test-only runtime hook and an updated test flow; no new multi-component runtime feature requiring a sequence diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
hooks/claude-notify/docs/agent-instructions/module-structure.md (1)
34-36: Vary sentence starters to avoid the three successive "Keep" openers.✏️ Suggested rewording
-## Structure Rules - -- Keep core logic in `Sources/NotifyCore`. -- Keep `Sources/ClaudeNotify/main.swift` thin and focused on wiring/entrypoint behavior. -- Keep `claude-notify.app` as a committed skeleton only; binary and code-sign outputs are build artifacts. +## Structure Rules + +- Core logic belongs in `Sources/NotifyCore`. +- `Sources/ClaudeNotify/main.swift` must stay thin and focused on wiring/entrypoint behavior. +- Treat `claude-notify.app` as a committed skeleton only; binary and code-sign outputs are build artifacts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/claude-notify/docs/agent-instructions/module-structure.md` around lines 34 - 36, The three list items use the same verb "Keep" repeatedly; reword them to vary sentence starters while preserving meaning: for the item referencing Sources/NotifyCore say something like "House core logic in Sources/NotifyCore," for main.swift use a different opener such as "Limit Sources/ClaudeNotify/main.swift to wiring/entrypoint responsibilities," and for claude-notify.app use an alternative like "Treat claude-notify.app as a committed skeleton; binary and code-sign outputs remain build artifacts." Ensure the unique references (Sources/NotifyCore, Sources/ClaudeNotify/main.swift, claude-notify.app) remain unchanged.hooks/claude-notify/AGENTS.md (1)
20-22: Consider varying sentence starters in Critical Overrides (same static-analysis flag as inmodule-structure.md).✏️ Suggested rewording
## Critical Overrides -- Keep test execution order as `unit -> integration -> e2e`. -- Keep `notify.sh` as the hook entrypoint, accepting stdin JSON payloads or message text via `$1`. -- Keep `README.md` synchronized with behavior, env var, and test changes. +- Test execution order must be `unit -> integration -> e2e`. +- `notify.sh` is the hook entrypoint; it accepts stdin JSON payloads or message text via `$1`. +- Keep `README.md` synchronized with behavior, env var, and test changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/claude-notify/AGENTS.md` around lines 20 - 22, The "Critical Overrides" section contains repetitive sentence starters; update AGENTS.md by rewording those lines to vary sentence openings and mirror the static-analysis flag usage from module-structure.md (use the same flag name and phrasing), and ensure related bullets like "Keep test execution order as `unit -> integration -> e2e`", the `notify.sh` entrypoint description, and the note about `README.md` remain accurate and consistent with the new phrasing; target the "Critical Overrides" heading and nearby sentences for these edits and keep the technical symbols (`unit -> integration -> e2e`, `notify.sh`, `README.md`, and the static-analysis flag name from module-structure.md) unchanged except for stylistic rewording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hooks/claude-notify/AGENTS.md`:
- Around line 20-22: The "Critical Overrides" section contains repetitive
sentence starters; update AGENTS.md by rewording those lines to vary sentence
openings and mirror the static-analysis flag usage from module-structure.md (use
the same flag name and phrasing), and ensure related bullets like "Keep test
execution order as `unit -> integration -> e2e`", the `notify.sh` entrypoint
description, and the note about `README.md` remain accurate and consistent with
the new phrasing; target the "Critical Overrides" heading and nearby sentences
for these edits and keep the technical symbols (`unit -> integration -> e2e`,
`notify.sh`, `README.md`, and the static-analysis flag name from
module-structure.md) unchanged except for stylistic rewording.
In `@hooks/claude-notify/docs/agent-instructions/module-structure.md`:
- Around line 34-36: The three list items use the same verb "Keep" repeatedly;
reword them to vary sentence starters while preserving meaning: for the item
referencing Sources/NotifyCore say something like "House core logic in
Sources/NotifyCore," for main.swift use a different opener such as "Limit
Sources/ClaudeNotify/main.swift to wiring/entrypoint responsibilities," and for
claude-notify.app use an alternative like "Treat claude-notify.app as a
committed skeleton; binary and code-sign outputs remain build artifacts." Ensure
the unique references (Sources/NotifyCore, Sources/ClaudeNotify/main.swift,
claude-notify.app) remain unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
hooks/claude-notify/AGENTS.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/docs/agent-instructions/module-structure.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-shell
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/README.md : Keep README.md in hooks/claude-notify/ synchronized with behavior/env-var/test changes
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Sources/**/*.swift : Core logic must belong in Sources/NotifyCore; Sources/ClaudeNotify/main.swift should stay thin
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/{Sources,Tests}/**/*.swift : Minimum docstring coverage of >= 80% on Swift declarations in Sources/ and Tests/
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/** : Tests must remain tiered and ordered: unit -> integration -> e2e
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/notify.sh : notify.sh should accept either JSON on stdin (Claude hook payload) or message text as $1 argument
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/{integration,e2e}/** : Avoid introducing OS-state flakiness into integration and e2e test suites; preserve deterministic test behavior
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Sources/**/*.swift : Core logic must belong in Sources/NotifyCore; Sources/ClaudeNotify/main.swift should stay thin
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/{Sources,Tests}/**/*.swift : Minimum docstring coverage of >= 80% on Swift declarations in Sources/ and Tests/
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/README.md : Keep README.md in hooks/claude-notify/ synchronized with behavior/env-var/test changes
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/{Sources,Tests}/**/*.swift : Every test scenario must be listed in Tests/required-cases.txt with stable case IDs
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T07:28:09.415Z
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/e2e/test-runtime.sh:238-265
Timestamp: 2026-02-23T07:28:09.415Z
Learning: Applies to hooks/claude-notify/Tests/**/*.{swift,sh}: All test scenarios must have a stable ID registered in Tests/required-cases.txt. Unit tests use U001–U020 prefix, integration Swift XCTest use I001–I005, integration shell tests use I101–I129, e2e shell tests use E001–E013.
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/** : Tests must remain tiered and ordered: unit -> integration -> e2e
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/NotifyCoreTests/**/*.swift : Integration test case IDs (Swift) must be in the range I001–I005
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/NotifyCoreTests/**/*.swift : Unit test case IDs must be in the range U001–U020
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.md
📚 Learning: 2026-02-23T07:27:26.956Z
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/unit/test-notify-activate.sh:17-97
Timestamp: 2026-02-23T07:27:26.956Z
Learning: Applies to hooks/claude-notify/Tests/**/*.{swift,sh} : All test scenarios must have a stable ID registered in Tests/required-cases.txt. The active ranges are: Unit tests use U001–U020 prefix, integration Swift XCTest use I001–I005, integration shell tests use I101–I129, e2e shell tests use E001–E013. I006–I100 remain reserved.
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T07:27:36.086Z
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/unit/test-notify-activate.sh:17-97
Timestamp: 2026-02-23T07:27:36.086Z
Learning: In hooks/claude-notify, all test scenarios must have a stable ID registered in Tests/required-cases.txt. The documented active ranges are: Unit tests use U001–U020 prefix, Integration Swift XCTest use I001–I005, Integration shell tests use I101–I129, E2E shell tests use E001–E013, and I006–I100 remain reserved.
Applied to files:
hooks/claude-notify/docs/agent-instructions/module-structure.mdhooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/{integration,e2e}/** : Avoid introducing OS-state flakiness into integration and e2e test suites; preserve deterministic test behavior
Applied to files:
hooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.mdhooks/claude-notify/AGENTS.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/e2e/** : E2E test case IDs (shell) must be in the range E001–E013
Applied to files:
hooks/claude-notify/docs/agent-instructions/build-test-quality.mdhooks/claude-notify/docs/agent-instructions/conventions-maintenance.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/notify.sh : notify.sh should accept either JSON on stdin (Claude hook payload) or message text as $1 argument
Applied to files:
hooks/claude-notify/docs/agent-instructions/conventions-maintenance.md
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/integration/** : Integration test case IDs (shell) must be in the range I101–I129
Applied to files:
hooks/claude-notify/docs/agent-instructions/conventions-maintenance.md
🪛 LanguageTool
hooks/claude-notify/docs/agent-instructions/module-structure.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ocused on wiring/entrypoint behavior. - Keep claude-notify.app as a committed skel...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
hooks/claude-notify/AGENTS.md
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ON payloads or message text via $1. - Keep README.md synchronized with behavior,...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (1)
hooks/claude-notify/docs/agent-instructions/module-structure.md (1)
9-30: Layout tree is accurate — no corrections needed.Both entries verified:
test-claude-notify.shexists athooks/claude-notify/rootSources/NotifyCore/contains exactly one Swift file (NotifyCore.swift) as shownThe tree at lines 9–30 reflects the actual filesystem correctly. No drift detected.
Likely an incorrect or invalid review comment.
- make E010 assert fallback launch via origin arg-log evidence - verify sender-mode off and fallback-run markers in launched fallback path - keep warning-text check as non-fatal diagnostic to reduce CI flake sensitivity
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh (1)
215-221: Robust file-based fallback assertion — good flakiness reduction.The shift from stderr
grepto inspecting the origin args log is a solid improvement for determinism. Theawk+grepchain correctly verifies both-sender-mode offand-fallback-runin the fake origin's logged arguments.One edge-case note: if
-sender-modeis the very last argument in the log (no subsequent line),getlinereturns 0 and$0retains its prior value, which could cause a false positive or false negative depending on that prior line's content. In practice this shouldn't happen since-sender-modetakes a value argument and more flags follow, but a safer awk pattern could match both key and value on adjacent lines explicitly:🛡️ Optional: slightly more defensive awk
- && awk '/\]=-sender-mode$/{getline; if ($0 ~ /\]=off$/) found=1} END{exit found?0:1}' "$ORIGIN_ARGS_LOG" \ + && awk '/\]=-sender-mode$/{key=1; next} key && /\]=off$/{found=1; key=0} /\]=/{key=0} END{exit found?0:1}' "$ORIGIN_ARGS_LOG" \Not blocking — the current version works for the expected argument order.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/claude-notify/Tests/shell/e2e/test-runtime.sh` around lines 215 - 221, The awk test that looks for the "-sender-mode" key uses getline but doesn't check its return value, which can leave $0 unchanged if getline hits EOF; update the awk invocation that reads ORIGIN_ARGS_LOG (used in the conditional with wait_for_file and grep) to verify getline returned a line before inspecting it (e.g., store getline into a variable like "next" and only test next ~ /\]=off$/ when getline succeeded), so the "-sender-mode" check cannot produce a false result when the key is the last line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hooks/claude-notify/Tests/shell/e2e/test-runtime.sh`:
- Around line 215-221: The awk test that looks for the "-sender-mode" key uses
getline but doesn't check its return value, which can leave $0 unchanged if
getline hits EOF; update the awk invocation that reads ORIGIN_ARGS_LOG (used in
the conditional with wait_for_file and grep) to verify getline returned a line
before inspecting it (e.g., store getline into a variable like "next" and only
test next ~ /\]=off$/ when getline succeeded), so the "-sender-mode" check
cannot produce a false result when the key is the last line.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-shell
🧰 Additional context used
📓 Path-based instructions (3)
hooks/claude-notify/Tests/shell/e2e/**
📄 CodeRabbit inference engine (hooks/claude-notify/AGENTS.md)
E2E test case IDs (shell) must be in the range E001–E013
Files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
hooks/claude-notify/Tests/**
📄 CodeRabbit inference engine (hooks/claude-notify/AGENTS.md)
Tests must remain tiered and ordered: unit -> integration -> e2e
Files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
hooks/claude-notify/Tests/shell/{integration,e2e}/**
📄 CodeRabbit inference engine (hooks/claude-notify/AGENTS.md)
Avoid introducing OS-state flakiness into integration and e2e test suites; preserve deterministic test behavior
Files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/README.md : Keep README.md in hooks/claude-notify/ synchronized with behavior/env-var/test changes
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/{integration,e2e}/** : Avoid introducing OS-state flakiness into integration and e2e test suites; preserve deterministic test behavior
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Sources/**/*.swift : Core logic must belong in Sources/NotifyCore; Sources/ClaudeNotify/main.swift should stay thin
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/** : Tests must remain tiered and ordered: unit -> integration -> e2e
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/e2e/** : E2E test case IDs (shell) must be in the range E001–E013
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/unit/test-notify-activate.sh:17-97
Timestamp: 2026-02-23T07:27:36.086Z
Learning: In hooks/claude-notify, all test scenarios must have a stable ID registered in Tests/required-cases.txt. The documented active ranges are: Unit tests use U001–U020 prefix, Integration Swift XCTest use I001–I005, Integration shell tests use I101–I129, E2E shell tests use E001–E013, and I006–I100 remain reserved.
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/{Sources,Tests}/**/*.swift : Minimum docstring coverage of >= 80% on Swift declarations in Sources/ and Tests/
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/e2e/test-runtime.sh:238-265
Timestamp: 2026-02-23T07:28:09.415Z
Learning: Applies to hooks/claude-notify/Tests/**/*.{swift,sh}: All test scenarios must have a stable ID registered in Tests/required-cases.txt. Unit tests use U001–U020 prefix, integration Swift XCTest use I001–I005, integration shell tests use I101–I129, e2e shell tests use E001–E013.
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/{Sources,Tests}/**/*.swift : Every test scenario must be listed in Tests/required-cases.txt with stable case IDs
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/notify.sh : notify.sh should accept either JSON on stdin (Claude hook payload) or message text as $1 argument
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/{integration,e2e}/** : Avoid introducing OS-state flakiness into integration and e2e test suites; preserve deterministic test behavior
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/e2e/** : E2E test case IDs (shell) must be in the range E001–E013
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/** : Tests must remain tiered and ordered: unit -> integration -> e2e
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T07:28:09.415Z
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/e2e/test-runtime.sh:238-265
Timestamp: 2026-02-23T07:28:09.415Z
Learning: In hooks/claude-notify/Tests, ensure every test scenario has a stable ID registered in Tests/required-cases.txt. Use the following ID prefixes and ranges: Unit tests U001–U020, Integration Swift XCTest I001–I005, Integration shell tests I101–I129, and End-to-End shell tests E001–E013. For any test under this path, require a corresponding ID from these ranges to ensure traceability and reproducibility.
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T07:27:26.956Z
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/unit/test-notify-activate.sh:17-97
Timestamp: 2026-02-23T07:27:26.956Z
Learning: Enforce test case IDs for all tests under hooks/claude-notify/Tests. Each test scenario must have a stable ID registered in Tests/required-cases.txt. Active ID ranges: Unit tests U001–U020, Integration Swift XCTest I001–I005, Integration shell tests I101–I129, E2E shell tests E001–E013. Reserve I006–I100. During code reviews, verify that every test file under Tests contains a corresponding ID in the required-cases.txt with the correct prefix/range, and that IDs are not reused or out of range.
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/README.md : Keep README.md in hooks/claude-notify/ synchronized with behavior/env-var/test changes
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/integration/** : Integration test case IDs (shell) must be in the range I101–I129
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T07:27:36.086Z
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/unit/test-notify-activate.sh:17-97
Timestamp: 2026-02-23T07:27:36.086Z
Learning: In hooks/claude-notify/Tests/shell/**/*.sh, ensure every test scenario has a stable ID registered in Tests/required-cases.txt. Use one of the documented ID ranges:
- Unit tests: U001–U020
- Integration Swift XCTest: I001–I005
- Integration shell: I101–I129
- E2E shell tests: E001–E013
Note: I006–I100 are reserved. Use a valid ID from the appropriate range and verify its presence in Tests/required-cases.txt to ensure consistency across test scenarios.
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
🧬 Code graph analysis (1)
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh (2)
hooks/claude-notify/Tests/shell/lib/notify-test-helpers.sh (3)
make_case_tmp_dir(13-19)write_fake_origin_exec(44-58)drain_pid_file_if_present(164-173)hooks/claude-notify/Tests/shell/lib/testlib.sh (3)
pass(6-9)fail(11-14)wait_for_file(58-68)
🔇 Additional comments (2)
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh (2)
199-207: Setup uses origin-faking flow — clean and deterministic.The temp directory creation, fake origin wiring, and environment-variable scoping all follow existing test patterns (E007, E011) and properly isolate from OS state. Good improvement over the previous stderr-based approach.
Minor observation:
ORIGIN_ISOLATE_LOGandORIGIN_ALLOW_RETRY_LOG(lines 202-203) are passed to the fake origin but never asserted upon in this test. They're harmless (written bywrite_fake_origin_execas a side effect), but you could omit them from the env if you want to keep the invocation leaner. Not blocking.
222-224: Non-fatal diagnostic — good pragmatic approach to reduce CI flakiness.Demoting the stderr warning-text check to a diagnostic-only path is the right call. The structural assertion (args log) on lines 215-217 is the authoritative check; the stderr text is a secondary signal that shouldn't gate CI. Based on learnings: "Avoid introducing OS-state flakiness into integration and e2e test suites; preserve deterministic test behavior."
- vary repeated sentence starters in AGENTS Critical Overrides - vary repeated sentence starters in module-structure rules - keep all technical constraints and references unchanged
- guard getline result before matching sender-mode off value - prevent stale awk /bin/zsh reads when key appears at EOF - preserve deterministic fallback-run assertion behavior
- document that AGENTS links are file-relative to hooks/claude-notify/AGENTS.md - keep existing docs/agent-instructions link paths unchanged - address discussion r2840383760 without changing runtime behavior
…auth callback - move CLAUDE_NOTIFY_TEST_FORCE_POST_ERROR handling before requestAuthorization - remove callback-time forced-post-error branch to avoid timeout race - preserve existing auth-denied and non-test delivery behavior
- poll E010 origin args log for fallback-run marker before assertions - retain defensive sender-mode awk check and non-fatal warning diagnostic - add docs/agent-instructions subtree to module-structure layout tree
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh (1)
215-225: Consider parenthesizinggetline nextlinefor unambiguous operator precedence.
getline nextline > 0inside anif()condition is unambiguous on macOS'sawk(one-true-awk) because>in an expression context is always a relational operator, not I/O redirection. However,>serves both as a relational and a redirection in awk, with context distinguishing between the two meanings, and it is wise to always use parentheses whenever there is an unusual combination of operators, as even experienced programmers occasionally forget the exact rules; explicit parentheses help prevent such mistakes.The conventional idiom avoids any reader confusion:
♻️ Suggested clarification
-if grep -q -- '\]=-fallback-run$' "$ORIGIN_ARGS_LOG" 2>/dev/null \ - && awk '/\]=-sender-mode$/{ if (getline nextline > 0 && nextline ~ /\]=off$/) found=1 } END{exit found?0:1}' "$ORIGIN_ARGS_LOG"; then +if grep -q -- '\]=-fallback-run$' "$ORIGIN_ARGS_LOG" 2>/dev/null \ + && awk '/\]=-sender-mode$/{ if ((getline nextline) > 0 && nextline ~ /\]=off$/) found=1 } END{exit found?0:1}' "$ORIGIN_ARGS_LOG"; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/claude-notify/Tests/shell/e2e/test-runtime.sh` around lines 215 - 225, Change the awk condition that currently uses "if (getline nextline > 0 && nextline ~ /\]=off$/) ..." to explicitly parenthesize the getline call so precedence is unambiguous (e.g. use "if ((getline nextline) > 0 && nextline ~ /\]=off$/) ..."). Update the awk invocation in the test loop (the awk pattern handling '\]=-sender-mode$' and the END exit logic) to use the parenthesized form to avoid potential misinterpretation of ">" as a redirection operator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hooks/claude-notify/Tests/shell/e2e/test-runtime.sh`:
- Around line 215-225: Change the awk condition that currently uses "if (getline
nextline > 0 && nextline ~ /\]=off$/) ..." to explicitly parenthesize the
getline call so precedence is unambiguous (e.g. use "if ((getline nextline) > 0
&& nextline ~ /\]=off$/) ..."). Update the awk invocation in the test loop (the
awk pattern handling '\]=-sender-mode$' and the END exit logic) to use the
parenthesized form to avoid potential misinterpretation of ">" as a redirection
operator.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hooks/claude-notify/Tests/shell/e2e/test-runtime.shhooks/claude-notify/docs/agent-instructions/module-structure.md
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/claude-notify/docs/agent-instructions/module-structure.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
hooks/claude-notify/Tests/shell/e2e/**
📄 CodeRabbit inference engine (hooks/claude-notify/AGENTS.md)
E2E test case IDs (shell) must be in the range E001–E013
Files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
hooks/claude-notify/Tests/**
📄 CodeRabbit inference engine (hooks/claude-notify/AGENTS.md)
Tests must remain tiered and ordered: unit -> integration -> e2e
Files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
hooks/claude-notify/Tests/shell/{integration,e2e}/**
📄 CodeRabbit inference engine (hooks/claude-notify/AGENTS.md)
Avoid introducing OS-state flakiness into integration and e2e test suites; preserve deterministic test behavior
Files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/README.md : Keep README.md in hooks/claude-notify/ synchronized with behavior/env-var/test changes
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/{Sources,Tests}/**/*.swift : Minimum docstring coverage of >= 80% on Swift declarations in Sources/ and Tests/
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/{Sources,Tests}/**/*.swift : Every test scenario must be listed in Tests/required-cases.txt with stable case IDs
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Sources/**/*.swift : Core logic must belong in Sources/NotifyCore; Sources/ClaudeNotify/main.swift should stay thin
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/e2e/test-runtime.sh:238-265
Timestamp: 2026-02-23T07:28:09.415Z
Learning: Applies to hooks/claude-notify/Tests/**/*.{swift,sh}: All test scenarios must have a stable ID registered in Tests/required-cases.txt. Unit tests use U001–U020 prefix, integration Swift XCTest use I001–I005, integration shell tests use I101–I129, e2e shell tests use E001–E013.
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/unit/test-notify-activate.sh:17-97
Timestamp: 2026-02-23T07:27:26.956Z
Learning: Applies to hooks/claude-notify/Tests/**/*.{swift,sh} : All test scenarios must have a stable ID registered in Tests/required-cases.txt. The active ranges are: Unit tests use U001–U020 prefix, integration Swift XCTest use I001–I005, integration shell tests use I101–I129, e2e shell tests use E001–E013. I006–I100 remain reserved.
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/unit/test-notify-activate.sh:17-97
Timestamp: 2026-02-23T07:27:36.086Z
Learning: In hooks/claude-notify, all test scenarios must have a stable ID registered in Tests/required-cases.txt. The documented active ranges are: Unit tests use U001–U020 prefix, Integration Swift XCTest use I001–I005, Integration shell tests use I101–I129, E2E shell tests use E001–E013, and I006–I100 remain reserved.
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/** : Tests must remain tiered and ordered: unit -> integration -> e2e
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/NotifyCoreTests/**/*.swift : Integration test case IDs (Swift) must be in the range I001–I005
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/{integration,e2e}/** : Avoid introducing OS-state flakiness into integration and e2e test suites; preserve deterministic test behavior
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/{integration,e2e}/** : Avoid introducing OS-state flakiness into integration and e2e test suites; preserve deterministic test behavior
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/e2e/** : E2E test case IDs (shell) must be in the range E001–E013
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T07:28:09.415Z
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/e2e/test-runtime.sh:238-265
Timestamp: 2026-02-23T07:28:09.415Z
Learning: In hooks/claude-notify/Tests, ensure every test scenario has a stable ID registered in Tests/required-cases.txt. Use the following ID prefixes and ranges: Unit tests U001–U020, Integration Swift XCTest I001–I005, Integration shell tests I101–I129, and End-to-End shell tests E001–E013. For any test under this path, require a corresponding ID from these ranges to ensure traceability and reproducibility.
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/** : Tests must remain tiered and ordered: unit -> integration -> e2e
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/Tests/shell/integration/** : Integration test case IDs (shell) must be in the range I101–I129
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T07:27:26.956Z
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/unit/test-notify-activate.sh:17-97
Timestamp: 2026-02-23T07:27:26.956Z
Learning: Enforce test case IDs for all tests under hooks/claude-notify/Tests. Each test scenario must have a stable ID registered in Tests/required-cases.txt. Active ID ranges: Unit tests U001–U020, Integration Swift XCTest I001–I005, Integration shell tests I101–I129, E2E shell tests E001–E013. Reserve I006–I100. During code reviews, verify that every test file under Tests contains a corresponding ID in the required-cases.txt with the correct prefix/range, and that IDs are not reused or out of range.
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T08:19:32.025Z
Learnt from: CR
Repo: giwankim/my-claude-code PR: 0
File: hooks/claude-notify/AGENTS.md:0-0
Timestamp: 2026-02-23T08:19:32.025Z
Learning: Applies to hooks/claude-notify/README.md : Keep README.md in hooks/claude-notify/ synchronized with behavior/env-var/test changes
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
📚 Learning: 2026-02-23T07:27:36.086Z
Learnt from: giwankim
Repo: giwankim/my-claude-code PR: 7
File: hooks/claude-notify/Tests/shell/unit/test-notify-activate.sh:17-97
Timestamp: 2026-02-23T07:27:36.086Z
Learning: In hooks/claude-notify/Tests/shell/**/*.sh, ensure every test scenario has a stable ID registered in Tests/required-cases.txt. Use one of the documented ID ranges:
- Unit tests: U001–U020
- Integration Swift XCTest: I001–I005
- Integration shell: I101–I129
- E2E shell tests: E001–E013
Note: I006–I100 are reserved. Use a valid ID from the appropriate range and verify its presence in Tests/required-cases.txt to ensure consistency across test scenarios.
Applied to files:
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh
🧬 Code graph analysis (1)
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh (2)
hooks/claude-notify/Tests/shell/lib/notify-test-helpers.sh (3)
make_case_tmp_dir(13-19)write_fake_origin_exec(44-58)drain_pid_file_if_present(164-173)hooks/claude-notify/Tests/shell/lib/testlib.sh (2)
pass(6-9)fail(11-14)
🔇 Additional comments (2)
hooks/claude-notify/Tests/shell/e2e/test-runtime.sh (2)
199-207: E010 setup LGTM — thewrite_fake_origin_exec/tmp-dir isolation approach cleanly replaces the old direct origin invocation and all three log env vars are correctly threaded into the$NOTIFYinvocation so the fake origin can inherit them at runtime.
226-228: Non-fatal diagnostic path LGTM — printing to stderr without incrementingFAILcorrectly surfaces warning-text regressions without breaking the E010 pass/fail gate.
- poll for activate key/value evidence before final assertions - use defensive awk getline checks to avoid stale line reads - reduce async notify_run write/read race sensitivity in shell integration
|
- poll E010 origin args log for fallback-run marker before assertions - retain defensive sender-mode awk check and non-fatal warning diagnostic - add docs/agent-instructions subtree to module-structure layout tree



Summary
hooks/claude-notify/AGENTS.mdto agile scope, quick reference, and overridesdocs/agent-instructionsTesting