Skip to content

Commit 393e4be

Browse files
committed
fix(session-log): walk ancestor PIDs to find Claude Code session metadata
When plannotator is invoked from a slash command's `!` bang, the direct parent process is an intermediate bash shell spawned by the Bash tool — not Claude Code itself. The old resolveSessionLogByPpid() only checked process.ppid, so it always missed the session metadata file and fell back to mtime-based selection, which picks the wrong log when multiple sessions exist for the same project. New resolution ladder (four tiers): 1. Ancestor-PID walk: call `ps -o ppid=` repeatedly from process.ppid up to 8 hops, checking ~/.claude/sessions/<pid>.json at each hop. Deterministic — no guessing, matches exact session every time. 2. Cwd-scan: read every ~/.claude/sessions/*.json, filter by cwd, pick most recent startedAt. Handles cases where ps is unavailable. 3. CWD slug mtime (legacy): existing behavior, fragile with multiple sessions. 4. Ancestor directory walk: handles cd-deeper-into-subdirectory cases. Adds getAncestorPids (injectable getParent for testing), resolveSessionLogByAncestorPids, and resolveSessionLogByCwdScan. Exports SessionMetadata and accepts projectsDirOverride on findSessionLogsForCwd for test isolation. 17 new tests cover: edge cases for getAncestorPids (cycles, maxHops, self-loops), resolveSessionLogByAncestorPids (finds correct session among multiple, skips missing logs, falls back when no metadata matches), and resolveSessionLogByCwdScan (picks newest startedAt, ignores mismatched cwd, handles missing sessions dir). Closes #458
1 parent 0fe6c86 commit 393e4be

File tree

3 files changed

+444
-33
lines changed

3 files changed

+444
-33
lines changed

apps/hook/server/index.ts

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ import { hostnameOrFallback } from "@plannotator/shared/project";
8181
import { planDenyFeedback } from "@plannotator/shared/feedback-templates";
8282
import { readImprovementHook } from "@plannotator/shared/improvement-hooks";
8383
import type { Origin } from "@plannotator/shared/agents";
84-
import { findSessionLogsForCwd, resolveSessionLogByPpid, findSessionLogsByAncestorWalk, getLastRenderedMessage, type RenderedMessage } from "./session-log";
84+
import { findSessionLogsForCwd, resolveSessionLogByAncestorPids, resolveSessionLogByCwdScan, findSessionLogsByAncestorWalk, getLastRenderedMessage, type RenderedMessage } from "./session-log";
8585
import { findCodexRolloutByThreadId, getLastCodexMessage } from "./codex-session";
8686
import { findCopilotPlanContent, findCopilotSessionForCwd, getLastCopilotMessage } from "./copilot-session";
8787
import {
@@ -641,12 +641,18 @@ if (args[0] === "sessions") {
641641
// Claude Code path: resolve session log
642642
//
643643
// Strategy (most precise → least precise):
644-
// 1. PPID session metadata: ~/.claude/sessions/<ppid>.json gives us the
645-
// exact sessionId and original cwd. Deterministic, O(1), no scanning.
646-
// 2. CWD slug match: existing behavior — works when the shell CWD hasn't
647-
// changed from the session's project directory.
648-
// 3. Ancestor walk: walk up the directory tree trying parent slugs. Handles
649-
// the common case where the user `cd`'d deeper into a subdirectory.
644+
// 1. Ancestor-PID session metadata: walk up the process tree checking
645+
// ~/.claude/sessions/<pid>.json at each hop. When invoked from a slash
646+
// command's `!` bang, the direct parent is a bash subshell — Claude's
647+
// session file is a few hops up. Deterministic when it matches.
648+
// 2. Cwd-scan of session metadata: read every ~/.claude/sessions/*.json,
649+
// filter by cwd, pick the most recent startedAt. Better than mtime
650+
// guessing because it uses session-level metadata.
651+
// 3. CWD slug match (mtime-based): legacy behavior — picks the most
652+
// recently modified jsonl in the project dir. Fragile when multiple
653+
// sessions exist for the same project.
654+
// 4. Ancestor directory walk: handles the case where the user `cd`'d
655+
// deeper into a subdirectory after session start.
650656

651657
if (process.env.PLANNOTATOR_DEBUG) {
652658
console.error(`[DEBUG] Project root: ${projectRoot}`);
@@ -666,15 +672,19 @@ if (args[0] === "sessions") {
666672
}
667673
}
668674

669-
// 1. Try PPID-based session metadata (most reliable)
670-
const ppidLog = resolveSessionLogByPpid();
671-
tryLogCandidates("PPID session metadata", () => ppidLog ? [ppidLog] : []);
675+
// 1. Walk ancestor PIDs for a matching session metadata file
676+
const ancestorLog = resolveSessionLogByAncestorPids();
677+
tryLogCandidates("Ancestor PID session metadata", () => ancestorLog ? [ancestorLog] : []);
672678

673-
// 2. Fall back to CWD slug match
674-
tryLogCandidates("CWD slug match", () => findSessionLogsForCwd(projectRoot));
679+
// 2. Scan all session metadata files for one whose cwd matches
680+
const cwdScanLog = resolveSessionLogByCwdScan({ cwd: projectRoot });
681+
tryLogCandidates("Cwd-scan session metadata", () => cwdScanLog ? [cwdScanLog] : []);
675682

676-
// 3. Fall back to ancestor directory walk
677-
tryLogCandidates("Ancestor walk", () => findSessionLogsByAncestorWalk(projectRoot));
683+
// 3. Fall back to CWD slug match (mtime-based)
684+
tryLogCandidates("CWD slug match (mtime)", () => findSessionLogsForCwd(projectRoot));
685+
686+
// 4. Fall back to ancestor directory walk
687+
tryLogCandidates("Directory ancestor walk", () => findSessionLogsByAncestorWalk(projectRoot));
678688
}
679689

680690
if (!lastMessage) {

apps/hook/server/session-log.test.ts

Lines changed: 295 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ import {
1616
projectSlugFromCwd,
1717
findSessionLogsByAncestorWalk,
1818
findSessionLogsForCwd,
19+
getAncestorPids,
20+
resolveSessionLogByAncestorPids,
21+
resolveSessionLogByCwdScan,
1922
type SessionLogEntry,
2023
} from "./session-log";
2124
import { mkdirSync, writeFileSync, rmSync } from "node:fs";
@@ -635,3 +638,295 @@ describe("findSessionLogsByAncestorWalk", () => {
635638
}
636639
});
637640
});
641+
642+
// --- Resolver Tests (new) ---
643+
644+
describe("getAncestorPids", () => {
645+
test("returns empty array for invalid startPid", () => {
646+
expect(getAncestorPids(0, 5, () => null)).toEqual([]);
647+
expect(getAncestorPids(1, 5, () => null)).toEqual([]);
648+
});
649+
650+
test("returns startPid when there is no parent", () => {
651+
expect(getAncestorPids(100, 5, () => null)).toEqual([100]);
652+
});
653+
654+
test("walks up the PID chain until root", () => {
655+
const parents: Record<number, number> = { 100: 200, 200: 300, 300: 1 };
656+
expect(
657+
getAncestorPids(100, 10, (p) => parents[p] ?? null)
658+
).toEqual([100, 200, 300]);
659+
});
660+
661+
test("respects maxHops limit", () => {
662+
const parents: Record<number, number> = { 100: 200, 200: 300, 300: 400 };
663+
expect(
664+
getAncestorPids(100, 2, (p) => parents[p] ?? null)
665+
).toEqual([100, 200]);
666+
});
667+
668+
test("breaks on PID cycles", () => {
669+
const parents: Record<number, number> = { 100: 200, 200: 100 };
670+
const chain = getAncestorPids(100, 50, (p) => parents[p] ?? null);
671+
expect(chain).toEqual([100, 200]);
672+
});
673+
674+
test("breaks when getParent returns startPid (self-loop)", () => {
675+
const chain = getAncestorPids(100, 50, () => 100);
676+
expect(chain).toEqual([100]);
677+
});
678+
});
679+
680+
/** Build an isolated sessions + projects dir under tmpdir for a test. */
681+
function makeTempDirs(label: string): {
682+
sessionsDir: string;
683+
projectsDir: string;
684+
cleanup: () => void;
685+
} {
686+
const base = join(tmpdir(), `plannotator-resolver-${label}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`);
687+
const sessionsDir = join(base, "sessions");
688+
const projectsDir = join(base, "projects");
689+
mkdirSync(sessionsDir, { recursive: true });
690+
mkdirSync(projectsDir, { recursive: true });
691+
return {
692+
sessionsDir,
693+
projectsDir,
694+
cleanup: () => rmSync(base, { recursive: true, force: true }),
695+
};
696+
}
697+
698+
/** Write a session metadata file for a PID. */
699+
function writeSessionMeta(
700+
sessionsDir: string,
701+
pid: number,
702+
meta: { sessionId: string; cwd: string; startedAt?: number }
703+
): void {
704+
writeFileSync(
705+
join(sessionsDir, `${pid}.json`),
706+
JSON.stringify({
707+
pid,
708+
sessionId: meta.sessionId,
709+
cwd: meta.cwd,
710+
startedAt: meta.startedAt ?? Date.now(),
711+
})
712+
);
713+
}
714+
715+
/** Create a session jsonl for a given cwd + sessionId. */
716+
function writeSessionLog(
717+
projectsDir: string,
718+
cwd: string,
719+
sessionId: string,
720+
content = '{"type":"assistant","message":{"id":"m1","content":[{"type":"text","text":"hi"}]}}\n'
721+
): string {
722+
const slug = cwd.replace(/[^a-zA-Z0-9-]/g, "-");
723+
const dir = join(projectsDir, slug);
724+
mkdirSync(dir, { recursive: true });
725+
const path = join(dir, `${sessionId}.jsonl`);
726+
writeFileSync(path, content);
727+
return path;
728+
}
729+
730+
describe("resolveSessionLogByAncestorPids", () => {
731+
test("returns null when no ancestor PID has session metadata", () => {
732+
const { sessionsDir, projectsDir, cleanup } = makeTempDirs("no-ancestor");
733+
try {
734+
const result = resolveSessionLogByAncestorPids({
735+
startPid: 100,
736+
getParentPid: () => null,
737+
sessionsDir,
738+
projectsDir,
739+
});
740+
expect(result).toBeNull();
741+
} finally {
742+
cleanup();
743+
}
744+
});
745+
746+
test("finds session at direct parent (1 hop)", () => {
747+
const { sessionsDir, projectsDir, cleanup } = makeTempDirs("direct-parent");
748+
try {
749+
const cwd = "/tmp/fake-project-direct";
750+
const sessionId = "abcd-1234";
751+
writeSessionMeta(sessionsDir, 999, { sessionId, cwd });
752+
const logPath = writeSessionLog(projectsDir, cwd, sessionId);
753+
754+
const result = resolveSessionLogByAncestorPids({
755+
startPid: 999,
756+
getParentPid: () => null,
757+
sessionsDir,
758+
projectsDir,
759+
});
760+
expect(result).toBe(logPath);
761+
} finally {
762+
cleanup();
763+
}
764+
});
765+
766+
test("walks past bash subshell to find Claude Code ancestor", () => {
767+
// Simulates: plannotator (ppid=500 = sh) → sh (ppid=400 = claude)
768+
// Claude Code's session file is at pid 400, NOT 500.
769+
const { sessionsDir, projectsDir, cleanup } = makeTempDirs("walks-past");
770+
try {
771+
const cwd = "/tmp/fake-project-walk";
772+
const sessionId = "walk-1234";
773+
writeSessionMeta(sessionsDir, 400, { sessionId, cwd });
774+
const logPath = writeSessionLog(projectsDir, cwd, sessionId);
775+
776+
const parents: Record<number, number> = { 500: 400, 400: 1 };
777+
const result = resolveSessionLogByAncestorPids({
778+
startPid: 500,
779+
getParentPid: (p) => parents[p] ?? null,
780+
sessionsDir,
781+
projectsDir,
782+
});
783+
expect(result).toBe(logPath);
784+
} finally {
785+
cleanup();
786+
}
787+
});
788+
789+
test("skips metadata when matching jsonl does not exist", () => {
790+
const { sessionsDir, projectsDir, cleanup } = makeTempDirs("skip-missing");
791+
try {
792+
const cwd = "/tmp/fake-project-skip";
793+
// Metadata exists but the log file does not
794+
writeSessionMeta(sessionsDir, 400, { sessionId: "missing-id", cwd });
795+
796+
const result = resolveSessionLogByAncestorPids({
797+
startPid: 400,
798+
getParentPid: () => null,
799+
sessionsDir,
800+
projectsDir,
801+
});
802+
expect(result).toBeNull();
803+
} finally {
804+
cleanup();
805+
}
806+
});
807+
808+
test("returns null when sessionsDir doesn't exist", () => {
809+
const { projectsDir, cleanup } = makeTempDirs("no-sessions");
810+
try {
811+
const result = resolveSessionLogByAncestorPids({
812+
startPid: 100,
813+
getParentPid: () => null,
814+
sessionsDir: "/nonexistent/sessions/dir/xyz",
815+
projectsDir,
816+
});
817+
expect(result).toBeNull();
818+
} finally {
819+
cleanup();
820+
}
821+
});
822+
});
823+
824+
describe("resolveSessionLogByCwdScan", () => {
825+
test("returns null when sessionsDir is empty", () => {
826+
const { sessionsDir, projectsDir, cleanup } = makeTempDirs("empty");
827+
try {
828+
const result = resolveSessionLogByCwdScan({
829+
cwd: "/tmp/any",
830+
sessionsDir,
831+
projectsDir,
832+
});
833+
expect(result).toBeNull();
834+
} finally {
835+
cleanup();
836+
}
837+
});
838+
839+
test("returns null when no session metadata matches cwd", () => {
840+
const { sessionsDir, projectsDir, cleanup } = makeTempDirs("no-match");
841+
try {
842+
writeSessionMeta(sessionsDir, 100, {
843+
sessionId: "other-id",
844+
cwd: "/tmp/other-project",
845+
});
846+
const result = resolveSessionLogByCwdScan({
847+
cwd: "/tmp/my-project",
848+
sessionsDir,
849+
projectsDir,
850+
});
851+
expect(result).toBeNull();
852+
} finally {
853+
cleanup();
854+
}
855+
});
856+
857+
test("picks most recent startedAt when multiple sessions match cwd", () => {
858+
const { sessionsDir, projectsDir, cleanup } = makeTempDirs("pick-recent");
859+
try {
860+
const cwd = "/tmp/multi-project";
861+
// Two concurrent sessions with the same cwd
862+
writeSessionMeta(sessionsDir, 111, {
863+
sessionId: "old-session",
864+
cwd,
865+
startedAt: 1_000,
866+
});
867+
writeSessionMeta(sessionsDir, 222, {
868+
sessionId: "new-session",
869+
cwd,
870+
startedAt: 2_000,
871+
});
872+
writeSessionLog(projectsDir, cwd, "old-session");
873+
const newLog = writeSessionLog(projectsDir, cwd, "new-session");
874+
875+
const result = resolveSessionLogByCwdScan({
876+
cwd,
877+
sessionsDir,
878+
projectsDir,
879+
});
880+
expect(result).toBe(newLog);
881+
} finally {
882+
cleanup();
883+
}
884+
});
885+
886+
test("falls through to older session if newest has no matching jsonl", () => {
887+
const { sessionsDir, projectsDir, cleanup } = makeTempDirs("fallthrough");
888+
try {
889+
const cwd = "/tmp/fallthrough-project";
890+
writeSessionMeta(sessionsDir, 111, {
891+
sessionId: "old-session",
892+
cwd,
893+
startedAt: 1_000,
894+
});
895+
writeSessionMeta(sessionsDir, 222, {
896+
sessionId: "new-session-no-log",
897+
cwd,
898+
startedAt: 2_000,
899+
});
900+
const oldLog = writeSessionLog(projectsDir, cwd, "old-session");
901+
// Note: no jsonl for new-session-no-log
902+
903+
const result = resolveSessionLogByCwdScan({
904+
cwd,
905+
sessionsDir,
906+
projectsDir,
907+
});
908+
expect(result).toBe(oldLog);
909+
} finally {
910+
cleanup();
911+
}
912+
});
913+
914+
test("ignores malformed session metadata files", () => {
915+
const { sessionsDir, projectsDir, cleanup } = makeTempDirs("malformed");
916+
try {
917+
const cwd = "/tmp/malformed-project";
918+
writeFileSync(join(sessionsDir, "999.json"), "not valid json");
919+
writeSessionMeta(sessionsDir, 111, { sessionId: "good", cwd });
920+
const goodLog = writeSessionLog(projectsDir, cwd, "good");
921+
922+
const result = resolveSessionLogByCwdScan({
923+
cwd,
924+
sessionsDir,
925+
projectsDir,
926+
});
927+
expect(result).toBe(goodLog);
928+
} finally {
929+
cleanup();
930+
}
931+
});
932+
});

0 commit comments

Comments
 (0)