skills/continuous-learning-v2/agents/start-observer.sh (1)

77-80: Variable injection can break on paths with single quotes.

The '"$VAR"' pattern will produce malformed bash if any path contains a single quote (e.g., /home/user's-dir/.claude/...). While uncommon, this can cause silent failures.

A safer alternative is to use printf '%q' to escape values or export the variables so the child inherits them.

Alternative: export variables before nohup

+    # Export for child process
+    export CONFIG_DIR PID_FILE LOG_FILE OBSERVATIONS_FILE
+
     # The observer loop — fully detached with nohup, IO redirected to log
     nohup /bin/bash -c '
       set +e
       unset CLAUDECODE
-
-      CONFIG_DIR="'"$CONFIG_DIR"'"
-      PID_FILE="'"$PID_FILE"'"
-      LOG_FILE="'"$LOG_FILE"'"
-      OBSERVATIONS_FILE="'"$OBSERVATIONS_FILE"'"
+      # Variables inherited from parent via export

       trap "rm -f \"$PID_FILE\"; exit 0" TERM INT

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@skills/continuous-learning-v2/agents/start-observer.sh` around lines 77 - 80,
The current inline assignment using the pattern
'"$CONFIG_DIR"','"$PID_FILE"','"$LOG_FILE"','"$OBSERVATIONS_FILE"' can break
when values contain single quotes; update start-observer.sh to either (a) escape
each variable before embedding by using printf '%q' for CONFIG_DIR, PID_FILE,
LOG_FILE and OBSERVATIONS_FILE so the generated shell fragment is safe, or (b)
simpler: export those variables in the parent shell (export CONFIG_DIR PID_FILE
LOG_FILE OBSERVATIONS_FILE) and invoke nohup/child without embedding quoted
values so the child inherits them; modify the code that writes/echoes those
lines to use one of these approaches and ensure the unique symbols CONFIG_DIR,
PID_FILE, LOG_FILE and OBSERVATIONS_FILE are handled accordingly.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

skills/continuous-learning-v2/agents/start-observer.sh (2)

181-183: Replace fixed startup delay with bounded readiness polling.

Line 182’s fixed sleep 2 can still false-fail on slower machines/filesystems. A short polling loop with timeout is more reliable.

⏱️ Suggested refactor

-    # Wait for PID file
-    sleep 2
+    # Wait up to 10s for PID file
+    for _ in {1..40}; do
+      [ -f "$PID_FILE" ] && break
+      sleep 0.25
+    done

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@skills/continuous-learning-v2/agents/start-observer.sh` around lines 181 -
183, The fixed sleep (sleep 2) used to "Wait for PID file" can false-fail on
slow systems; replace it with a bounded polling loop that repeatedly checks for
the PID file's existence (e.g., the variable holding the PID path such as
OBSERVER_PID_FILE or PID_FILE used earlier in start-observer.sh) with a short
sleep (e.g., 0.2s) and an overall timeout (e.g., 10s); if the file appears break
and continue, otherwise after timeout emit an error and exit non-zero—implement
the loop where the current sleep 2 sits and preserve existing logging/exit
behavior.

---

26-31: Add explicit PID validation for defense-in-depth.

Lines 27, 45, 65, and 185 use PID values from the PID file with kill. While kill -0 provides a safety check, explicit validation before reading the PID improves code clarity and robustness. Consider adding a validation function to ensure the PID file contains only positive integers before use, rejecting malformed values (e.g., negative numbers, special values like -1).

Suggested validation function

+is_valid_pid() {
+  [[ "$1" =~ ^[1-9][0-9]*$ ]]
+}
+
 case "${1:-start}" in
   stop)
     if [ -f "$PID_FILE" ]; then
       pid=$(cat "$PID_FILE")
+      if ! is_valid_pid "$pid"; then
+        echo "Observer not running (invalid PID file)."
+        rm -f "$PID_FILE"
+        exit 0
+      fi
       if kill -0 "$pid" 2>/dev/null; then

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@skills/continuous-learning-v2/agents/start-observer.sh` around lines 26 - 31,
Add a defensive PID validation routine (e.g., is_valid_pid or validate_pid) that
reads the PID string from PID_FILE and ensures it is a non-zero positive integer
(reject negative numbers, zero, empty strings, and values containing non-digits
or special prefixes like "-1"); call this validator wherever the script reads
pid from PID_FILE (references: PID_FILE variable and the blocks that call kill
and kill -0) and if validation fails log an error, remove or ignore the
malformed PID_FILE, and avoid calling kill with the bad value.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

skills/continuous-learning-v2/agents/start-observer.sh (1)

259-260: Prefer a short poll loop over a fixed 2s startup wait.

A fixed delay can cause flaky “failed to start” reports on slower machines.

💡 Proposed refactor

-    # Wait for PID file
-    sleep 2
+    # Wait up to 10s for PID file
+    for _ in {1..50}; do
+      [ -f "$PID_FILE" ] && break
+      sleep 0.2
+    done

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@skills/continuous-learning-v2/agents/start-observer.sh` around lines 259 -
260, Replace the fixed "sleep 2" startup wait with a short poll loop that checks
for the observer PID file existence (the PID file referenced where "sleep 2"
currently sits) at a small interval (e.g., 100–250ms), stop polling once the PID
file appears, and enforce a reasonable overall timeout after which the script
logs a clear failure and exits non‑zero; update the block around the existing
"sleep 2" call to perform this polling, use the same PID file variable name used
elsewhere in the script (e.g., PID_FILE or PIDFILE), and ensure to log
success/failure so callers don’t get flaky “failed to start” reports.

Configuration used: defaults

Review profile: CHILL

Plan: Pro