mirror of
https://github.com/affaan-m/ECC.git
synced 2026-11-05 07:35:51 +00:00
fix(hooks): attribute native activity to the registered ECC session
Verify review discussion4419348312 against actual native producer/importer behavior. Prefer the independently sanitized ECC_SESSION_ID for the ECC-owned metrics feed, retain payload then legacy fallback for standalone hooks, and leave sibling hooks unchanged. Actual Node producer plus read-only cached native binary reproduces zero imported metrics before the fix and exactly one tool/file/log after repeated sessions/status commands. Four producer regression controls and normal/adversarial inherited-env suites pass22/22. Focused coverage96.47percent lines and91.47percent branches; parent independent source review passed.
This commit is contained in:
2 files changed
+45
-11
No files matched your search
@@ -562,9 +562,8 @@ function summarizeOutput(toolOutput) {
|
||||
}
|
||||
|
||||
function buildActivityRow(input, env = process.env) {
|
||||
// Same env-var-reliance bug as the session id below: Claude Code doesn't
|
||||
// set CLAUDE_HOOK_EVENT_NAME for hook subprocesses either. Real hook stdin
|
||||
// payloads carry hook_event_name as a JSON field -- prefer that.
|
||||
// Claude Code does not set CLAUDE_HOOK_EVENT_NAME for hook subprocesses.
|
||||
// Real hook stdin payloads carry hook_event_name as a JSON field.
|
||||
const hookEvent = String(
|
||||
input?.hook_event_name || env.CLAUDE_HOOK_EVENT_NAME || ''
|
||||
).trim();
|
||||
@@ -573,15 +572,14 @@ function buildActivityRow(input, env = process.env) {
|
||||
}
|
||||
|
||||
const toolName = String(input?.tool_name || '').trim();
|
||||
// Claude Code does not set CLAUDE_SESSION_ID as an environment variable for
|
||||
// hook subprocesses -- the session id is only available in the hook's own
|
||||
// stdin JSON payload. Prefer that; keep the env vars as a fallback for any
|
||||
// caller that does set them. Sanitized the same way as the sibling hooks
|
||||
// (cost-tracker.js, ecc-context-monitor.js, ecc-metrics-bridge.js,
|
||||
// gateguard-fact-force.js) that already use this exact fallback order.
|
||||
// Native ECC workers set ECC_SESSION_ID to their registered session ID.
|
||||
// This feed belongs to that ECC session, even when the harness payload
|
||||
// carries a different conversation ID. Standalone hooks use their payload
|
||||
// ID before the legacy Claude environment fallback. Sanitize each candidate
|
||||
// separately so an invalid native ID cannot hide a usable payload ID.
|
||||
const sessionId =
|
||||
sanitizeSessionId(input?.session_id) ||
|
||||
sanitizeSessionId(env.ECC_SESSION_ID) ||
|
||||
sanitizeSessionId(input?.session_id) ||
|
||||
sanitizeSessionId(env.CLAUDE_SESSION_ID) ||
|
||||
'';
|
||||
if (!toolName || !sessionId) {
|
||||
|
||||
@@ -60,7 +60,11 @@ function runScript(input, envOverrides = {}, options = {}) {
|
||||
encoding: 'utf8',
|
||||
input: inputStr,
|
||||
timeout: 10000,
|
||||
env: { ...process.env, ...ISOLATED_HOME_ENV, ...envOverrides },
|
||||
env: {
|
||||
...process.env, ...ISOLATED_HOME_ENV,
|
||||
ECC_SESSION_ID: '', CLAUDE_SESSION_ID: '',
|
||||
...envOverrides,
|
||||
},
|
||||
cwd: options.cwd,
|
||||
});
|
||||
return { code: result.status || 0, stdout: result.stdout || '', stderr: result.stderr || '' };
|
||||
@@ -414,6 +418,38 @@ function runTests() {
|
||||
fs.rmSync(tmpHome, { recursive: true, force: true });
|
||||
}) ? passed++ : failed++);
|
||||
|
||||
for (const [label, eccId, payloadId, legacyId, expectedId] of [
|
||||
['native ECC identity wins over the distinct harness payload', 'ecc-native-owner', 'claude-conversation', 'stale-parent', 'ecc-native-owner'],
|
||||
['standalone payload wins over a stale legacy environment', '', 'claude-standalone', 'stale-parent', 'claude-standalone'],
|
||||
['invalid ECC identity falls back to the payload', '../invalid-owner', 'claude-standalone', 'stale-parent', 'claude-standalone'],
|
||||
['invalid ECC and payload identities fall back to the legacy environment', 'bad/owner', 'bad\\payload', 'claude-legacy', 'claude-legacy'],
|
||||
]) {
|
||||
(test(`actual activity producer: ${label}`, () => {
|
||||
const tmpHome = makeTempDir();
|
||||
try {
|
||||
const input = {
|
||||
hook_event_name: 'PostToolUse', session_id: payloadId,
|
||||
tool_name: 'Read', tool_input: { file_path: 'src/identity.ts' },
|
||||
};
|
||||
const result = runScript(input, {
|
||||
...withTempHome(tmpHome),
|
||||
ECC_AGENT_DATA_HOME: path.join(tmpHome, '.claude'),
|
||||
CLAUDE_CONFIG_DIR: path.join(tmpHome, '.claude'),
|
||||
ECC_SESSION_ID: eccId, CLAUDE_SESSION_ID: legacyId,
|
||||
});
|
||||
assert.strictEqual(result.code, 0, result.stderr);
|
||||
assert.strictEqual(result.stdout, JSON.stringify(input));
|
||||
const rows = readMetricRows(tmpHome);
|
||||
assert.strictEqual(rows.length, 1);
|
||||
assert.strictEqual(rows[0].session_id, expectedId);
|
||||
assert.strictEqual(rows[0].tool_name, 'Read');
|
||||
assert.deepStrictEqual(rows[0].file_paths, ['src/identity.ts']);
|
||||
} finally {
|
||||
fs.rmSync(tmpHome, { recursive: true, force: true });
|
||||
}
|
||||
}) ? passed++ : failed++);
|
||||
}
|
||||
|
||||
(test('reads session_id from stdin JSON when no session env var is set', () => {
|
||||
// Claude Code does not set CLAUDE_SESSION_ID for hook subprocesses in
|
||||
// real invocations -- the session id only arrives via stdin JSON.
|
||||
|
||||
Reference in new issue
Block a user