mirror of
https://github.com/affaan-m/ECC.git
synced 2026-11-10 18:16:02 +00:00
fix(continuous-learning): isolate observer project scopes
This commit is contained in:
1 parent
b38be6a7f6
commit
3bfb442a52
2 files changed
+196
-41
No files matched your search
@@ -104,16 +104,23 @@ export CLV2_PYTHON_CMD="${CLV2_PYTHON_CMD:-$PYTHON_CMD}"
|
||||
# If cwd is a subdirectory inside a git repo, resolve it to the repo root so
|
||||
# observations attach to the project instead of a nested path.
|
||||
STDIN_CWD=$(echo "$INPUT_JSON" | "$PYTHON_CMD" -c '
|
||||
import json, sys
|
||||
import json, os, sys
|
||||
try:
|
||||
data = json.load(sys.stdin)
|
||||
cwd = data.get("cwd", "")
|
||||
print(cwd)
|
||||
except(KeyError, TypeError, ValueError):
|
||||
cwd = cwd[:4096] if isinstance(cwd, str) else ""
|
||||
if isinstance(cwd, str) and os.path.isabs(cwd) and os.path.isdir(cwd):
|
||||
print(os.path.realpath(cwd))
|
||||
else:
|
||||
print("")
|
||||
except (KeyError, OSError, TypeError, ValueError):
|
||||
print("")
|
||||
' 2>/dev/null || echo "")
|
||||
|
||||
# If cwd was provided in stdin, use it for project detection
|
||||
# Only a validated, absolute payload cwd may select a project. Missing,
|
||||
# relative, or non-existent paths must not fall through to detect-project.sh's
|
||||
# inherited-process-cwd discovery, which can attach an observation to an
|
||||
# unrelated repository that happened to launch the hook.
|
||||
if [ -n "$STDIN_CWD" ] && [ -d "$STDIN_CWD" ]; then
|
||||
_GIT_ROOT=$(git -C "$STDIN_CWD" rev-parse --show-toplevel 2>/dev/null || true)
|
||||
if [ -n "$_GIT_ROOT" ]; then
|
||||
@@ -123,6 +130,9 @@ if [ -n "$STDIN_CWD" ] && [ -d "$STDIN_CWD" ]; then
|
||||
unset CLAUDE_PROJECT_DIR
|
||||
export CLV2_NO_PROJECT=1
|
||||
fi
|
||||
else
|
||||
unset CLAUDE_PROJECT_DIR
|
||||
export CLV2_NO_PROJECT=1
|
||||
fi
|
||||
|
||||
# ─────────────────────────────────────────────
|
||||
@@ -232,6 +242,7 @@ try:
|
||||
session_id = data.get("session_id", "unknown")
|
||||
tool_use_id = data.get("tool_use_id", "")
|
||||
cwd = data.get("cwd", "")
|
||||
cwd = cwd[:4096] if isinstance(cwd, str) else ""
|
||||
|
||||
# Truncate large inputs/outputs
|
||||
if isinstance(tool_input, dict):
|
||||
@@ -325,6 +336,7 @@ timestamp=$(date -u +"%Y-%m-%dT%H:%M:%SZ")
|
||||
|
||||
export PROJECT_ID_ENV="$PROJECT_ID"
|
||||
export PROJECT_NAME_ENV="$PROJECT_NAME"
|
||||
export OBSERVATION_CWD_ENV="$STDIN_CWD"
|
||||
export TIMESTAMP="$timestamp"
|
||||
|
||||
echo "$PARSED" | "$PYTHON_CMD" -c '
|
||||
@@ -382,6 +394,14 @@ def scrub(val):
|
||||
val = _SECRET_RE.sub(lambda m: m.group(1) + m.group(2) + (m.group(3) or "") + "[REDACTED]", str(val))
|
||||
return _SECRET_VALUE_RE.sub(lambda m: (m.group(2) or "") + "[REDACTED]", val)
|
||||
|
||||
# Prefer the canonical cwd that passed the project-selection boundary. Retain a
|
||||
# bounded rejected string as diagnostic metadata without ever using it to select
|
||||
# a project. Apply the same secret scrubber as tool I/O because path components
|
||||
# can still contain credentials or tokens (for example, generated checkout names).
|
||||
cwd = os.environ.get("OBSERVATION_CWD_ENV", "") or parsed.get("cwd", "")
|
||||
if cwd:
|
||||
observation["cwd"] = scrub(cwd)
|
||||
|
||||
if parsed["input"]:
|
||||
observation["input"] = scrub(parsed["input"])
|
||||
if parsed["output"] is not None:
|
||||
@@ -532,15 +552,17 @@ print(str(cfg.get('observer', {}).get('enabled', False)).lower())
|
||||
fi
|
||||
fi
|
||||
|
||||
# Check both project-scoped AND global PID files (with stale PID recovery)
|
||||
# Observers are deliberately scope-pinned: each project (and the global scope)
|
||||
# owns a distinct PROJECT_DIR, PID file, observation stream, and lazy-start lock.
|
||||
# An observer from another scope cannot consume this observation stream and must
|
||||
# not suppress startup or receive signals for the current scope.
|
||||
if [ "$OBSERVER_ENABLED" = "true" ]; then
|
||||
# Clean up stale PID files first.
|
||||
# `if` context (not `|| true`) so `set -e` stays satisfied while we still
|
||||
# capture whether either PID file pointed at a live observer.
|
||||
# capture whether this scope's PID file pointed at a live observer.
|
||||
OBSERVER_ALIVE=false
|
||||
OBSERVER_DIED=false
|
||||
if _CHECK_OBSERVER_RUNNING "${PROJECT_DIR}/.observer.pid"; then OBSERVER_ALIVE=true; fi
|
||||
if _CHECK_OBSERVER_RUNNING "${CONFIG_DIR}/.observer.pid"; then OBSERVER_ALIVE=true; fi
|
||||
|
||||
# A live observer clears the streak so a later one-off crash does not inherit
|
||||
# an old count. This is an idempotent unlink, not a read-modify-write, so it
|
||||
@@ -551,15 +573,14 @@ if [ "$OBSERVER_ENABLED" = "true" ]; then
|
||||
fi
|
||||
|
||||
# Check if observer is now running after cleanup
|
||||
if [ ! -f "${PROJECT_DIR}/.observer.pid" ] && [ ! -f "${CONFIG_DIR}/.observer.pid" ]; then
|
||||
if [ ! -f "${PROJECT_DIR}/.observer.pid" ]; then
|
||||
# Use flock if available (Linux), fallback for macOS
|
||||
if command -v flock >/dev/null 2>&1; then
|
||||
(
|
||||
flock -n 9 || exit 0
|
||||
# Double-check PID files after acquiring lock
|
||||
# Double-check this scope's PID file after acquiring the lock.
|
||||
_CHECK_OBSERVER_RUNNING "${PROJECT_DIR}/.observer.pid" || true
|
||||
_CHECK_OBSERVER_RUNNING "${CONFIG_DIR}/.observer.pid" || true
|
||||
if [ ! -f "${PROJECT_DIR}/.observer.pid" ] && [ ! -f "${CONFIG_DIR}/.observer.pid" ]; then
|
||||
if [ ! -f "${PROJECT_DIR}/.observer.pid" ]; then
|
||||
_START_OBSERVER_LOGGED
|
||||
fi
|
||||
) 9>"$LAZY_START_LOCK"
|
||||
@@ -571,8 +592,7 @@ if [ "$OBSERVER_ENABLED" = "true" ]; then
|
||||
trap '_REMOVE_FILE_IF_PRESENT "$LAZY_START_LOCK"' EXIT
|
||||
lockfile -r 1 -l 30 "$LAZY_START_LOCK" 2>/dev/null || exit 0
|
||||
_CHECK_OBSERVER_RUNNING "${PROJECT_DIR}/.observer.pid" || true
|
||||
_CHECK_OBSERVER_RUNNING "${CONFIG_DIR}/.observer.pid" || true
|
||||
if [ ! -f "${PROJECT_DIR}/.observer.pid" ] && [ ! -f "${CONFIG_DIR}/.observer.pid" ]; then
|
||||
if [ ! -f "${PROJECT_DIR}/.observer.pid" ]; then
|
||||
_START_OBSERVER_LOGGED
|
||||
fi
|
||||
_REMOVE_FILE_IF_PRESENT "$LAZY_START_LOCK"
|
||||
@@ -583,8 +603,7 @@ if [ "$OBSERVER_ENABLED" = "true" ]; then
|
||||
trap 'rmdir "${LAZY_START_LOCK}.d" 2>/dev/null || true' EXIT
|
||||
mkdir "${LAZY_START_LOCK}.d" 2>/dev/null || exit 0
|
||||
_CHECK_OBSERVER_RUNNING "${PROJECT_DIR}/.observer.pid" || true
|
||||
_CHECK_OBSERVER_RUNNING "${CONFIG_DIR}/.observer.pid" || true
|
||||
if [ ! -f "${PROJECT_DIR}/.observer.pid" ] && [ ! -f "${CONFIG_DIR}/.observer.pid" ]; then
|
||||
if [ ! -f "${PROJECT_DIR}/.observer.pid" ]; then
|
||||
_START_OBSERVER_LOGGED
|
||||
fi
|
||||
)
|
||||
@@ -699,29 +718,22 @@ PY
|
||||
) || should_signal=0
|
||||
fi
|
||||
|
||||
# Signal observer if running and throttle allows (check both project-scoped and global observer, deduplicate)
|
||||
# Signal only the observer that owns the current observation stream.
|
||||
if [ "$should_signal" -eq 1 ]; then
|
||||
signaled_pids=" "
|
||||
for pid_file in "${PROJECT_DIR}/.observer.pid" "${CONFIG_DIR}/.observer.pid"; do
|
||||
if [ -f "$pid_file" ]; then
|
||||
observer_pid=$(cat "$pid_file" 2>/dev/null || true)
|
||||
# Validate PID is a positive integer (>1)
|
||||
case "$observer_pid" in
|
||||
''|*[!0-9]*|0|1)
|
||||
_REMOVE_FILE_IF_PRESENT "$pid_file"
|
||||
continue
|
||||
;;
|
||||
esac
|
||||
# Deduplicate: skip if already signaled this pass
|
||||
case "$signaled_pids" in
|
||||
*" $observer_pid "*) continue ;;
|
||||
esac
|
||||
if kill -0 "$observer_pid" 2>/dev/null; then
|
||||
kill -USR1 "$observer_pid" 2>/dev/null || true
|
||||
signaled_pids="${signaled_pids}${observer_pid} "
|
||||
fi
|
||||
pid_file="${PROJECT_DIR}/.observer.pid"
|
||||
if [ -f "$pid_file" ]; then
|
||||
observer_pid=$(cat "$pid_file" 2>/dev/null || true)
|
||||
# Validate PID is a positive integer (>1)
|
||||
case "$observer_pid" in
|
||||
''|*[!0-9]*|0|1)
|
||||
_REMOVE_FILE_IF_PRESENT "$pid_file"
|
||||
observer_pid=""
|
||||
;;
|
||||
esac
|
||||
if [ -n "$observer_pid" ] && kill -0 "$observer_pid" 2>/dev/null; then
|
||||
kill -USR1 "$observer_pid" 2>/dev/null || true
|
||||
fi
|
||||
done
|
||||
fi
|
||||
fi
|
||||
|
||||
exit 0
|
||||
@@ -86,17 +86,20 @@ function gitInit(dir) {
|
||||
assert.strictEqual(commitResult.status, 0, commitResult.stderr);
|
||||
}
|
||||
|
||||
function runObserve({ homeDir, cwd, args = ['post'], extraEnv = {} }) {
|
||||
const payload = JSON.stringify({
|
||||
function runObserve({ homeDir, cwd, runCwd = repoRoot, args = ['post'], extraEnv = {} }) {
|
||||
const payloadData = {
|
||||
tool_name: 'Read',
|
||||
tool_input: { file_path: 'README.md' },
|
||||
tool_response: 'ok',
|
||||
session_id: 'session-subdir-test',
|
||||
cwd,
|
||||
});
|
||||
};
|
||||
if (cwd !== undefined) {
|
||||
payloadData.cwd = cwd;
|
||||
}
|
||||
const payload = JSON.stringify(payloadData);
|
||||
|
||||
return spawnSync('bash', [observeShPath, ...args], {
|
||||
cwd: repoRoot,
|
||||
cwd: runCwd,
|
||||
encoding: 'utf8',
|
||||
input: payload,
|
||||
env: {
|
||||
@@ -138,6 +141,14 @@ test('observe.sh resolves cwd to git root before setting CLAUDE_PROJECT_DIR', ()
|
||||
content.includes('export CLV2_NO_PROJECT=1'),
|
||||
'observe.sh should mark non-git cwd payloads as global instead of registering raw cwd'
|
||||
);
|
||||
assert.ok(
|
||||
!content.includes('_CHECK_OBSERVER_RUNNING "${CONFIG_DIR}/.observer.pid"'),
|
||||
'project observations must not treat an unrelated global observer as their observer'
|
||||
);
|
||||
assert.ok(
|
||||
!content.includes('for pid_file in "${PROJECT_DIR}/.observer.pid" "${CONFIG_DIR}/.observer.pid"'),
|
||||
'a project hook must signal only the observer for its current scope'
|
||||
);
|
||||
});
|
||||
|
||||
test('git rev-parse resolves a subdirectory to the repo root', () => {
|
||||
@@ -211,6 +222,12 @@ test('observe.sh writes project metadata for the git root when cwd is a subdirec
|
||||
|
||||
const observationsPath = path.join(projectDir, 'observations.jsonl');
|
||||
assert.ok(fs.existsSync(observationsPath), 'observe.sh should append an observation');
|
||||
const observation = JSON.parse(fs.readFileSync(observationsPath, 'utf8').trim());
|
||||
assert.strictEqual(
|
||||
normalizeComparablePath(observation.cwd),
|
||||
normalizeComparablePath(subDir),
|
||||
'observation should persist the canonical cwd used for project detection'
|
||||
);
|
||||
} finally {
|
||||
cleanupDir(testRoot);
|
||||
}
|
||||
@@ -278,6 +295,132 @@ test('observe.sh records non-git cwd payloads globally without project registry
|
||||
}
|
||||
});
|
||||
|
||||
test('observe.sh does not infer a project from inherited cwd when payload cwd is missing', () => {
|
||||
const testRoot = createTempDir();
|
||||
|
||||
try {
|
||||
const homeDir = path.join(testRoot, 'home');
|
||||
const inheritedRepo = path.join(testRoot, 'inherited-repo');
|
||||
fs.mkdirSync(homeDir, { recursive: true });
|
||||
fs.mkdirSync(inheritedRepo, { recursive: true });
|
||||
gitInit(inheritedRepo);
|
||||
|
||||
const result = runObserve({ homeDir, cwd: undefined, runCwd: inheritedRepo });
|
||||
assert.strictEqual(result.status, 0, result.stderr);
|
||||
|
||||
const homunculusDir = path.join(homeDir, '.local', 'share', 'ecc-homunculus');
|
||||
assert.ok(!fs.existsSync(path.join(homunculusDir, 'projects.json')));
|
||||
assert.ok(fs.existsSync(path.join(homunculusDir, 'observations.jsonl')));
|
||||
} finally {
|
||||
cleanupDir(testRoot);
|
||||
}
|
||||
});
|
||||
|
||||
test('observe.sh rejects invalid and relative payload cwd without inherited project fallback', () => {
|
||||
for (const payloadCwd of ['does-not-exist', '.']) {
|
||||
const testRoot = createTempDir();
|
||||
|
||||
try {
|
||||
const homeDir = path.join(testRoot, 'home');
|
||||
const inheritedRepo = path.join(testRoot, 'inherited-repo');
|
||||
fs.mkdirSync(homeDir, { recursive: true });
|
||||
fs.mkdirSync(inheritedRepo, { recursive: true });
|
||||
gitInit(inheritedRepo);
|
||||
|
||||
const result = runObserve({ homeDir, cwd: payloadCwd, runCwd: inheritedRepo });
|
||||
assert.strictEqual(result.status, 0, result.stderr);
|
||||
|
||||
const homunculusDir = path.join(homeDir, '.local', 'share', 'ecc-homunculus');
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(homunculusDir, 'projects.json')),
|
||||
`${payloadCwd} must not register the inherited git repository`
|
||||
);
|
||||
const observation = JSON.parse(
|
||||
fs.readFileSync(path.join(homunculusDir, 'observations.jsonl'), 'utf8').trim()
|
||||
);
|
||||
assert.strictEqual(
|
||||
observation.cwd,
|
||||
payloadCwd,
|
||||
'rejected cwd should remain available as scrubbed diagnostic metadata'
|
||||
);
|
||||
} finally {
|
||||
cleanupDir(testRoot);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('observe.sh scrubs secrets from persisted cwd metadata', () => {
|
||||
const testRoot = createTempDir();
|
||||
|
||||
try {
|
||||
const homeDir = path.join(testRoot, 'home');
|
||||
const repoDir = path.join(testRoot, 'repo');
|
||||
const secretDir = path.join(repoDir, 'token=abcdefghijklmnop');
|
||||
fs.mkdirSync(homeDir, { recursive: true });
|
||||
fs.mkdirSync(secretDir, { recursive: true });
|
||||
gitInit(repoDir);
|
||||
|
||||
const result = runObserve({ homeDir, cwd: secretDir });
|
||||
assert.strictEqual(result.status, 0, result.stderr);
|
||||
|
||||
const { projectDir } = readSingleProjectMetadata(homeDir);
|
||||
const observation = JSON.parse(
|
||||
fs.readFileSync(path.join(projectDir, 'observations.jsonl'), 'utf8').trim()
|
||||
);
|
||||
assert.ok(observation.cwd.includes('token=[REDACTED]'));
|
||||
assert.ok(!observation.cwd.includes('abcdefghijklmnop'));
|
||||
} finally {
|
||||
cleanupDir(testRoot);
|
||||
}
|
||||
});
|
||||
|
||||
test('a live global observer does not suppress the current project observer', () => {
|
||||
const testRoot = createTempDir();
|
||||
let projectObserverPid;
|
||||
|
||||
try {
|
||||
const homeDir = path.join(testRoot, 'home');
|
||||
const repoDir = path.join(testRoot, 'repo');
|
||||
const configPath = path.join(testRoot, 'config.json');
|
||||
const homunculusDir = path.join(homeDir, '.local', 'share', 'ecc-homunculus');
|
||||
fs.mkdirSync(homeDir, { recursive: true });
|
||||
fs.mkdirSync(repoDir, { recursive: true });
|
||||
fs.mkdirSync(homunculusDir, { recursive: true });
|
||||
gitInit(repoDir);
|
||||
fs.writeFileSync(
|
||||
configPath,
|
||||
JSON.stringify({
|
||||
observer: { enabled: true, run_interval_minutes: 1, min_observations_to_analyze: 20 },
|
||||
})
|
||||
);
|
||||
fs.writeFileSync(path.join(homunculusDir, '.observer.pid'), `${process.pid}\n`);
|
||||
|
||||
const result = runObserve({
|
||||
homeDir,
|
||||
cwd: repoDir,
|
||||
extraEnv: { CLV2_CONFIG: configPath },
|
||||
});
|
||||
assert.strictEqual(result.status, 0, result.stderr);
|
||||
|
||||
const { projectDir } = readSingleProjectMetadata(homeDir);
|
||||
const projectPidPath = path.join(projectDir, '.observer.pid');
|
||||
assert.ok(fs.existsSync(projectPidPath), 'current project should start its own observer');
|
||||
projectObserverPid = Number.parseInt(fs.readFileSync(projectPidPath, 'utf8').trim(), 10);
|
||||
assert.ok(Number.isSafeInteger(projectObserverPid) && projectObserverPid > 1);
|
||||
assert.notStrictEqual(projectObserverPid, process.pid);
|
||||
process.kill(projectObserverPid, 0);
|
||||
} finally {
|
||||
if (projectObserverPid) {
|
||||
try {
|
||||
process.kill(projectObserverPid, 'SIGTERM');
|
||||
} catch {
|
||||
// The observer may already have exited; cleanup is best-effort.
|
||||
}
|
||||
}
|
||||
cleanupDir(testRoot);
|
||||
}
|
||||
});
|
||||
|
||||
console.log(`\nPassed: ${passed}`);
|
||||
console.log(`Failed: ${failed}`);
|
||||
|
||||
|
||||
Reference in new issue
Block a user