fix(observer): improve Windows compatibility for temp files and Haiku prompt#972
Conversation
… prompt Address remaining issues from affaan-m#842 after PR affaan-m#903 moved temp files to PROJECT_DIR/.observer-tmp: Bug A (path resolution): Use relative paths (.observer-tmp/filename) in the prompt instead of absolute paths from mktemp. On Windows Git Bash/MSYS2, absolute paths use MSYS-style prefixes (/c/Users/...) that the spawned Claude subprocess may fail to resolve. Bug B (asks for permission): Add explicit IMPORTANT instruction block at the prompt start telling the Haiku agent it is in non-interactive --print mode and must use the Write tool directly without asking for confirmation. Additional improvements: - Pass prompt via -p flag instead of stdin redirect for Windows compat - Add .observer-tmp/ to .gitignore to prevent accidental commits Fixes affaan-m#842 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
|
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:
📝 WalkthroughWalkthroughCompute a project-relative analysis path under Changes
Sequence Diagram(s)sequenceDiagram
participant Script as Script (observer-loop.sh)
participant FS as Filesystem (.observer-tmp/)
participant Claude as Claude CLI
Script->>FS: write analysis content to `.observer-tmp/<basename>` (analysis_relpath)
Script->>Script: cd "$PROJECT_DIR" (set CWD)
Script->>Claude: invoke `claude -p "<prompt referencing analysis_relpath>"`
Claude->>FS: use Write tool to create/modify files at `analysis_relpath`
Claude-->>Script: return output and logs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/continuous-learning-v2/agents/observer-loop.sh`:
- Around line 65-69: The relative-analysis path variable analysis_relpath
(".observer-tmp/$(basename \"$analysis_file\")") can be unresolved if the script
is run from a different cwd; update the observer launch flow so the Claude
subprocess runs with PROJECT_DIR as its working directory: ensure
start-observer.sh (the code that launches this script around the
start-observer.sh start/stop block) does a cd "$PROJECT_DIR" (or uses an
explicit subprocess cwd equivalent) before invoking the observer script, and/or
make the observer-loop.sh explicitly cd "$PROJECT_DIR" near the top before any
use of analysis_relpath so Read sees .observer-tmp/... reliably.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9009a8b5-c472-4776-a98e-414a4086e8e4
📒 Files selected for processing (2)
.gitignoreskills/continuous-learning-v2/agents/observer-loop.sh
Greptile SummaryThis PR closes two remaining Windows-compat bugs from #842 in the observer loop's Haiku analysis cycle.
The Confidence Score: 5/5Safe to merge — both targeted bugs are addressed correctly, no regressions introduced on Linux/macOS. The two bugs are fixed with minimal, well-scoped changes. The previous review concern about CWD assumption has been resolved by the explicit No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[analyze_observations called] --> B{OBSERVATIONS_FILE exists\nand obs_count >= MIN?}
B -- No --> Z[return early]
B -- Yes --> C[mktemp analysis_file\ntail OBSERVATIONS_FILE into it]
C --> D[Compute analysis_relpath\n.observer-tmp/basename]
D --> E[mktemp prompt_file\nwrite IMPORTANT block + relpath into it]
E --> F{cd PROJECT_DIR}
F -- Fails --> G[Log error\nrm prompt_file & analysis_file\nreturn]
F -- OK --> H["Launch: claude -p $(cat prompt_file)\n--allowedTools Read,Write\n--print background &"]
H --> I[Start watchdog subshell\nsleep timeout_seconds]
I --> J[wait claude_pid]
J --> K[kill watchdog\nrm prompt_file & analysis_file]
K --> L{exit_code != 0?}
L -- Yes --> M[Log failure]
L -- No --> N[Archive OBSERVATIONS_FILE]
M --> N
Reviews (4): Last reviewed commit: "fix(observer): clean up temp files on cd..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ECC_SKIP_OBSERVE=1 ECC_HOOK_PROFILE=minimal claude --model haiku --max-turns "$max_turns" --print \ | ||
| --allowedTools "Read,Write" \ | ||
| < "$prompt_file" >> "$LOG_FILE" 2>&1 & | ||
| -p "$(cat "$prompt_file")" >> "$LOG_FILE" 2>&1 & |
There was a problem hiding this comment.
With -p "$(cat "$prompt_file")" the shell reads and expands $prompt_file before launching claude, so the file's contents are baked into the argument list at launch time. This means $prompt_file is not actually needed by the claude process at all — you could rm -f "$prompt_file" immediately after the command is started (before the sleep watchdog subshell) and simplify the cleanup on line 146. Minor point, but avoids the file persisting on disk through the full $timeout_seconds window in the edge case where wait is reached late.
There was a problem hiding this comment.
Good catch — adopted in commit 7a4cb8c. The prompt_file is now rm -f'd immediately after the -p "$(cat "$prompt_file")" shell expansion, and the final cleanup line only removes analysis_file.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/continuous-learning-v2/agents/observer-loop.sh">
<violation number="1" location="skills/continuous-learning-v2/agents/observer-loop.sh:74">
P1: Relative analysis path in prompt is not anchored to PROJECT_DIR, so Claude may fail to read the temp analysis file when observer-loop runs from a different working directory.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Reviewers correctly identified that the relative analysis_relpath (.observer-tmp/<file>) only resolves when CWD equals PROJECT_DIR. Without an explicit cd, non-Windows users launching the observer from a different directory would fail to read the analysis file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
skills/continuous-learning-v2/agents/observer-loop.sh (1)
127-130:⚠️ Potential issue | 🟠 MajorGuard
cd "$PROJECT_DIR"failure before invoking Claude.Line 129 can fail silently (the script runs with
set +e), which would continue in the wrong working directory and break relativeanalysis_relpathresolution. The prompt passed to Claude at line 48 uses the relative path.observer-tmp/..., so the working directory is critical.🔧 Suggested fix
- cd "$PROJECT_DIR" + if ! cd "$PROJECT_DIR"; then + echo "[$(date)] Failed to cd into PROJECT_DIR: ${PROJECT_DIR}" >> "$LOG_FILE" + rm -f "$prompt_file" "$analysis_file" + return + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/continuous-learning-v2/agents/observer-loop.sh` around lines 127 - 130, The script currently does an unguarded cd "$PROJECT_DIR" which can fail silently and break relative analysis_relpath resolution used when invoking Claude (the prompt built around .observer-tmp/...). Update observer-loop.sh to verify the cd succeeded immediately after the cd "$PROJECT_DIR" call (check the exit status of that command) and abort with a clear error/log and non-zero exit if it fails, so the Claude invocation (the block that constructs/sends the prompt using analysis_relpath) never runs from the wrong working directory; reference PROJECT_DIR, the cd command, analysis_relpath, and the Claude invocation when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@skills/continuous-learning-v2/agents/observer-loop.sh`:
- Around line 127-130: The script currently does an unguarded cd "$PROJECT_DIR"
which can fail silently and break relative analysis_relpath resolution used when
invoking Claude (the prompt built around .observer-tmp/...). Update
observer-loop.sh to verify the cd succeeded immediately after the cd
"$PROJECT_DIR" call (check the exit status of that command) and abort with a
clear error/log and non-zero exit if it fails, so the Claude invocation (the
block that constructs/sends the prompt using analysis_relpath) never runs from
the wrong working directory; reference PROJECT_DIR, the cd command,
analysis_relpath, and the Claude invocation when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24ebab6f-2f0a-42cd-8240-828f286661ca
📒 Files selected for processing (1)
skills/continuous-learning-v2/agents/observer-loop.sh
Address reviewer feedback: under set +e, a failing cd would silently leave CWD unchanged, causing the relative analysis path to break. Add || return with a diagnostic log entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/continuous-learning-v2/agents/observer-loop.sh">
<violation number="1" location="skills/continuous-learning-v2/agents/observer-loop.sh:129">
P2: Early return on failed `cd` skips cleanup of `prompt_file` and `analysis_file`, leaving temp files in `.observer-tmp`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/continuous-learning-v2/agents/observer-loop.sh`:
- Around line 127-130: The cd failure path in the observer loop returns early
and skips cleanup, leaving stale temp files; update the failure branch that
handles cd "$PROJECT_DIR" (when it logs to "$LOG_FILE" and returns) to remove
the temporary files referenced by prompt_file and analysis_file (and any other
.observer-tmp artifacts the script creates) before returning so cleanup is
always performed even if PROJECT_DIR is invalid/unavailable; locate the cd
failure block in observer-loop.sh and add safe checks/removals for prompt_file
and analysis_file (use test -f before rm or a safe rm -f) and preserve the
existing logging to "$LOG_FILE".
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4fcaaa9-f877-49d1-9028-6734b9adc59f
📒 Files selected for processing (1)
skills/continuous-learning-v2/agents/observer-loop.sh
The cd "$PROJECT_DIR" failure path returned without removing prompt_file and analysis_file, leaving stale temp files in .observer-tmp/. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
…elpath - Remove prompt_file immediately after shell expansion into -p arg, avoiding stale temp files during long analysis windows (greptile feedback) - Update test assertion to check analysis_relpath instead of analysis_file, matching the cross-platform relative path change from earlier commits Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
2 participants
Summary
-pflag instead of stdin redirect for Windows compatibilitycd "$PROJECT_DIR"failure with cleanup and early returnprompt_fileimmediately after shell expansion (greptile feedback)analysis_relpathvariable rename.observer-tmp/to.gitignoreProblem
After PR #903 moved temp files from
/tmptoPROJECT_DIR/.observer-tmp, two issues from #842 remain:Bug A: On Windows (Git Bash/MSYS2),
mktempreturns absolute paths with MSYS-style prefixes (/c/Users/...) that the spawned Claude subprocess cannot resolve, causing ~50% of analysis cycles to fail with "cannot access the file".Bug B: When the Haiku agent can read the analysis file, it asks "Would you like me to save these as instinct files?" instead of writing directly — which nobody can answer in
--printmode. Result: 0 instinct files created across 12+ analysis cycles.Changes
observer-loop.sh.observer-tmp/<filename>instead of absolute path in promptobserver-loop.shIMPORTANT:block telling agent it's in non-interactive mode, must write directlyobserver-loop.sh-p "$(cat ...)"instead of< filestdin redirectobserver-loop.shcd "$PROJECT_DIR"with error logging and temp file cleanup on failureobserver-loop.shprompt_fileimmediately after shell expansion (adopted from greptile review)observer-memory.test.js${analysis_relpath}instead of${analysis_file}.gitignore.observer-tmp/to prevent accidental commitsReviewer feedback addressed
PROJECT_DIRwhen using relative paths ✅ (commit 31af1ad)cdfailure path ✅ (commit 4517321)cd "$PROJECT_DIR"failure ✅ (commit 194bc00)Before (observer-memory tests — test fails because assertion expects analysis_file but code uses analysis_relpath)
After (observer-memory tests + full test suite — all passing after fix)
Test plan
bash -n skills/continuous-learning-v2/agents/observer-loop.sh— syntax check passesnode tests/hooks/observer-memory.test.js— 23/23 observer tests passECC_OBSERVER_ALLOW_WINDOWS=true: verify Haiku agent can read the analysis file via relative path.observer-tmp/is excluded from git trackingFixes #842