mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix(autoplan): an oversize session journal is a hard too_large denial, not a retryable identity failure (#3050)
The guard capped journal reads at 32 MiB and reported a longer journal as 'identity', which the hook treats as deny-and-retry. A journal only grows, so /autoplan could never enter Phase 1 in a long session, and the troubleshooting row sent people chasing symlinks. Oversize journals now report too_large as a hard cause that names the size and the supported fallback (run the reviews by hand or start a new session). Co-authored-by: venkatareddyg <venkatareddyg@users.noreply.github.com>
This commit is contained in:
1 parent
2e1d3e735d
commit
ffde420ffe
4 files changed
+44
-7
No files matched your search
@@ -9,7 +9,7 @@ import { isDeepStrictEqual } from 'node:util';
|
||||
import { extractImplementationPlan, checkPhaseImplementation, acceptedBlocks } from '../../bin/gstack-autoplan-snapshot';
|
||||
import { autoplanPhaseCompletions } from '../../lib/autoplan-phase-publication';
|
||||
import { readOwnedClaudePublicTranscript, nativePathSpelling, ownedNativePath, sameNativePath,
|
||||
type ClaudeParentPublicEvent, type OwnedTranscriptReason } from '../../lib/claude-public-transcript';
|
||||
type ClaudeParentPublicEvent, type OwnedTranscriptReason, OWNED_TRANSCRIPT_MAX_BYTES } from '../../lib/claude-public-transcript';
|
||||
import { resolveStateRoot } from '../../lib/state-root';
|
||||
|
||||
const PHASES = ['ceo', 'design', 'dx', 'eng', 'tasks'] as const;
|
||||
@@ -528,6 +528,13 @@ const HARD_CAUSE: Partial<Record<OwnedTranscriptReason, string>> = {
|
||||
agent: "The session journal's conversation ancestry passes through a subagent record.",
|
||||
cycle: "The session journal's parent links form a cycle.",
|
||||
};
|
||||
/** #3050: a journal only grows, so the size limit is a hard cause with the real size, never a retry. */
|
||||
function journalTooLarge(journal: string): string {
|
||||
const mib = (bytes: number) => `${Math.round(bytes / (1024 * 1024))} MiB`;
|
||||
let size = 'over';
|
||||
try { size = mib(fs.statSync(journal).size); } catch { /* size stays generic */ }
|
||||
return `This session's journal is ${size}, over the ${mib(OWNED_TRANSCRIPT_MAX_BYTES)} the guard reads, and it only grows.`;
|
||||
}
|
||||
const guidance = (code: string, read?: OwnedRead) =>
|
||||
`(code ${code}, Claude Code ${read?.diagnostic?.claudeVersion ?? 'version unknown'}). Troubleshooting: ${GUIDE}`;
|
||||
|
||||
@@ -579,8 +586,9 @@ export async function runPublicationHook(value: unknown, root: string): Promise<
|
||||
if (input.tool_name === 'Read' && evaluatePublication(input, root, read.events, true).allow)
|
||||
return {};
|
||||
}
|
||||
const code = read.transcript.reason, cause = code && HARD_CAUSE[code];
|
||||
if (cause) fail(`Publication guard cannot verify this session: ${cause} Fallback: run /plan-ceo-review, then ` +
|
||||
const code = read.transcript.reason;
|
||||
const cause = code === 'too_large' ? journalTooLarge(journal) : code && HARD_CAUSE[code];
|
||||
if (code && cause) fail(`Publication guard cannot verify this session: ${cause} Fallback: run /plan-ceo-review, then ` +
|
||||
`/plan-devex-review, then /plan-eng-review by hand, or start a new session. ${guidance(code, read)}`);
|
||||
// Unflushed records, a missing current tool_use, malformed or changing
|
||||
// bytes and identity failures stay deny-and-retry.
|
||||
|
||||
@@ -50,6 +50,7 @@ phase entry.
|
||||
| `sidechain` | The first turn or its ancestry is a sidechain (subagent) record, not the parent session. |
|
||||
| `agent` | The conversation ancestry passes through a subagent record. |
|
||||
| `cycle` | The journal's parent links form a loop. |
|
||||
| `too_large` | The session journal is over the 32 MiB the guard reads (the message names its size). Long sessions, especially with screenshots, get there; a journal only grows, so retrying never helps. Start a new session for `/autoplan`. |
|
||||
|
||||
**What to do:** run the review phases by hand: `/plan-ceo-review`, then
|
||||
`/plan-devex-review`, then `/plan-eng-review`. Or start a new Claude Code
|
||||
@@ -65,7 +66,7 @@ The guard has not reached a conclusion. The message ends with
|
||||
|------|-------|
|
||||
| (none) | The journal does not yet hold the records the guard needs. This includes the current tool call, or an ancestor record Claude Code has not written yet. |
|
||||
| `changing` | Claude Code was writing the journal during every read in the 2-second window. |
|
||||
| `identity` | The journal path, its directories or the file itself failed the identity checks: for example a symlink, a foreign session, or the wrong directory layout. |
|
||||
| `identity` | The journal path, its directories or the file itself failed the identity checks: for example a symlink, a foreign session, or the wrong directory layout. A journal over the size limit reports `too_large` instead. |
|
||||
| `malformed` | A complete journal line is not valid JSON or UTF-8, or its records contradict each other. |
|
||||
|
||||
If a retry keeps failing with the same code, use the fallback above.
|
||||
|
||||
@@ -70,7 +70,9 @@ export function unresolvedPlanQuestionCalls(calls: NativePlanQuestionCall[]): Na
|
||||
calls.slice(index + 1).some(later => later.answered && later.answers?.[q.question])));
|
||||
}
|
||||
|
||||
const MAX_BYTES = 32 * 1024 * 1024;
|
||||
/** The most journal bytes the guard reads; a longer session reports `too_large`, never `identity` (#3050). */
|
||||
export const OWNED_TRANSCRIPT_MAX_BYTES = 32 * 1024 * 1024;
|
||||
const MAX_BYTES = OWNED_TRANSCRIPT_MAX_BYTES;
|
||||
const MAX_FILES = 64;
|
||||
const object = (value: unknown): value is Record<string, any> =>
|
||||
value !== null && typeof value === 'object' && !Array.isArray(value);
|
||||
@@ -178,8 +180,9 @@ function sameRealPath(a: unknown, b: unknown): boolean {
|
||||
* Positive identity conflicts are hard; `changing`, `identity` and `malformed`
|
||||
* retry; only `unrecognized_shape:*` may degrade to an advisory.
|
||||
*/
|
||||
export type OwnedTranscriptReason = 'competing_root' | 'foreign_cwd' | 'sidechain' | 'agent' | 'cycle' |
|
||||
export type OwnedTranscriptReason = 'competing_root' | 'foreign_cwd' | 'sidechain' | 'agent' | 'cycle' | 'too_large' |
|
||||
'changing' | 'identity' | 'malformed' | `unrecognized_shape:${string}`;
|
||||
|
||||
type OwnedLines = { lines: string[]; root?: string } | { reason: OwnedTranscriptReason; shape: string[] };
|
||||
|
||||
const nativeUuid = (value: unknown): value is string => typeof value === 'string' &&
|
||||
@@ -552,7 +555,9 @@ export function readOwnedClaudePublicTranscript(file: string, cwd: string, sessi
|
||||
throw new OwnedReadError('identity');
|
||||
fd = fs.openSync(file, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW);
|
||||
const before = fs.fstatSync(fd, { bigint: true });
|
||||
if (!before.isFile() || before.size > BigInt(MAX_BYTES)) throw new OwnedReadError('identity');
|
||||
if (!before.isFile()) throw new OwnedReadError('identity');
|
||||
// A journal only grows, so an oversize one can never pass on retry (#3050).
|
||||
if (before.size > BigInt(OWNED_TRANSCRIPT_MAX_BYTES)) throw new OwnedReadError('too_large');
|
||||
bytes = fs.readFileSync(fd);
|
||||
const after = fs.fstatSync(fd, { bigint: true }), current = fs.lstatSync(file, { bigint: true });
|
||||
if (!current.isFile() || before.dev !== current.dev || before.ino !== current.ino) throw new OwnedReadError('identity');
|
||||
|
||||
@@ -329,6 +329,29 @@ describe('Autoplan parent publication guard', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('#3050: a journal over the read limit is a hard too_large denial with its size, never a retryable identity', async () => {
|
||||
const f = fixture();
|
||||
f.message(); f.current(); f.journal();
|
||||
const pad = Buffer.alloc(33 * 1024 * 1024, 0x20); pad[pad.length - 1] = 0x0a;
|
||||
fs.appendFileSync(f.input.transcript_path, pad);
|
||||
expect(readOwnedClaudePublicTranscript(f.input.transcript_path, f.input.cwd, f.sessionId).transcript.reason).toBe('too_large');
|
||||
const output: any = await withNativeProjectDirectory(f.cwd, () => runPublicationHook(f.input, ROOT));
|
||||
const reason: string = output.hookSpecificOutput.permissionDecisionReason;
|
||||
expect(output.hookSpecificOutput.permissionDecision).toBe('deny');
|
||||
expect(reason).toContain("This session's journal is 33 MiB, over the 32 MiB the guard reads");
|
||||
expect(reason).toContain('code too_large');
|
||||
expect(reason).toContain('/plan-ceo-review');
|
||||
expect(reason).not.toContain('Retry this phase-entry tool');
|
||||
});
|
||||
|
||||
test('#3050: a real identity failure still reports identity', () => {
|
||||
const f = fixture();
|
||||
f.message(); f.current(); f.journal();
|
||||
const other = path.join(path.dirname(f.input.transcript_path), 'not-this-session.jsonl');
|
||||
fs.copyFileSync(f.input.transcript_path, other);
|
||||
expect(readOwnedClaudePublicTranscript(other, f.input.cwd, f.sessionId).transcript.reason).toBe('identity');
|
||||
});
|
||||
|
||||
test('native hook accepts platform paths without evaluating shell escapes', async () => {
|
||||
// The same actual hook receives native separators in Windows CI.
|
||||
const f = fixture();
|
||||
|
||||
Reference in new issue
Block a user