fix(review): schema-version gate, output-filter escape-bypass, generic double-wrap, follow-up resolution
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
25
.changeset/cli-agent-review-fixes.md
Normal file
25
.changeset/cli-agent-review-fixes.md
Normal file
@@ -0,0 +1,25 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Fix a batch of CLI Agent Executor review defects:
|
||||
|
||||
- **Schema-version gate**: bump `SCHEMA_VERSION` to 110 so a DB already at 109
|
||||
runs migration 110 and gains the `chat_sessions.cliExecutorAdapterId` column
|
||||
(it was previously short-circuited). Add the column to the compat-fingerprint
|
||||
`MIGRATION_ONLY_TABLE_SCHEMAS.chat_sessions` entry so the fingerprint matches.
|
||||
- **Generic adapter double-wrap**: `formatInjection` no longer re-wraps injected
|
||||
text in bracketed-paste markers when `bracketedPasteActive`; the session
|
||||
manager's security path is the sole wrapper, so the generic adapter (like every
|
||||
native one) only appends a carriage return.
|
||||
- **Output-filter cross-boundary bypass**: thread one carry buffer across the
|
||||
scrollback→live seam in the CLI session WS bridge so a dangerous escape (e.g.
|
||||
OSC 52) split across the seam is fully neutralized instead of the held
|
||||
introducer being flushed verbatim into the scrollback frame.
|
||||
- **Output-filter overflow leak**: when an over-length carry begins with a
|
||||
recognized dangerous introducer (OSC `ESC ]` / DCS `ESC P`), drop the
|
||||
introducer instead of flushing it as literal, so it cannot recombine with a
|
||||
later terminator at the client.
|
||||
- **Follow-up never resolves**: `followUp()` now drives the authoritative state
|
||||
machine `done→busy` before injecting, so the re-armed result promise resolves
|
||||
on the next positive `done` instead of hanging on an idempotent done.
|
||||
@@ -715,7 +715,7 @@ describe("schema migration", () => {
|
||||
|
||||
const row = db.prepare("SELECT deletedAt FROM tasks WHERE id = 'FN-legacy'").get() as { deletedAt: string | null };
|
||||
expect(row.deletedAt).toBeNull();
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -748,7 +748,7 @@ describe("schema migration", () => {
|
||||
{ id: "WS-001", mode: "prompt", gateMode: "advisory" },
|
||||
{ id: "WS-002", mode: "script", gateMode: "advisory" },
|
||||
]);
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -798,7 +798,7 @@ describe("schema migration", () => {
|
||||
reviewerContextRetryCount: 0,
|
||||
reviewerFallbackRetryCount: 0,
|
||||
});
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -827,7 +827,7 @@ describe("schema migration", () => {
|
||||
|
||||
const columns = db.prepare("PRAGMA table_info(milestones)").all() as Array<{ name: string }>;
|
||||
expect(columns.map((column) => column.name)).toContain("acceptanceCriteria");
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -868,7 +868,7 @@ describe("schema migration", () => {
|
||||
const missionColumns = db.prepare("PRAGMA table_info(missions)").all() as Array<{ name: string }>;
|
||||
expect(missionColumns.map((column) => column.name)).toContain("autoMerge");
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
db.close();
|
||||
});
|
||||
|
||||
@@ -902,7 +902,7 @@ describe("schema migration", () => {
|
||||
{ id: "WS-002", mode: "script", enabled: 1, gateMode: "advisory" },
|
||||
{ id: "WS-003", mode: "prompt", enabled: 0, gateMode: "advisory" },
|
||||
]);
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -939,7 +939,7 @@ describe("schema migration", () => {
|
||||
|
||||
const indexes = db.prepare("PRAGMA index_list(mission_goals)").all() as Array<{ name: string }>;
|
||||
expect(indexes.some((index) => index.name === "idxMissionGoalsGoalId")).toBe(true);
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -1000,7 +1000,7 @@ describe("schema migration", () => {
|
||||
expect(customFieldsColumn).toBeDefined();
|
||||
expect(customFieldsColumn?.dflt_value).toBe("'{}'");
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
db.close();
|
||||
});
|
||||
|
||||
@@ -1053,7 +1053,39 @@ describe("schema migration", () => {
|
||||
expect(indexNames).toContain("idx_cli_sessions_chatSessionId");
|
||||
expect(indexNames).toContain("idx_cli_sessions_project_state");
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
db.close();
|
||||
});
|
||||
|
||||
it("adds cliExecutorAdapterId to chat_sessions when migrating from schema version 109", () => {
|
||||
const db = new Database(fusionDir);
|
||||
db.exec("CREATE TABLE IF NOT EXISTS __meta (key TEXT PRIMARY KEY, value TEXT)");
|
||||
db.exec("INSERT INTO __meta (key, value) VALUES ('schemaVersion', '109')");
|
||||
db.exec("INSERT INTO __meta (key, value) VALUES ('lastModified', '1000')");
|
||||
db.exec(`
|
||||
CREATE TABLE IF NOT EXISTS chat_sessions (
|
||||
id TEXT PRIMARY KEY,
|
||||
agentId TEXT NOT NULL,
|
||||
title TEXT,
|
||||
status TEXT NOT NULL DEFAULT 'active',
|
||||
projectId TEXT,
|
||||
modelProvider TEXT,
|
||||
modelId TEXT,
|
||||
createdAt TEXT NOT NULL,
|
||||
updatedAt TEXT NOT NULL,
|
||||
cliSessionFile TEXT,
|
||||
inFlightGeneration TEXT
|
||||
)
|
||||
`);
|
||||
|
||||
db.init();
|
||||
|
||||
const columns = db
|
||||
.prepare("PRAGMA table_info(chat_sessions)")
|
||||
.all() as Array<{ name: string }>;
|
||||
expect(columns.map((column) => column.name)).toContain("cliExecutorAdapterId");
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
db.close();
|
||||
});
|
||||
|
||||
@@ -1063,7 +1095,7 @@ describe("schema migration", () => {
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table'").all() as Array<{ name: string }>;
|
||||
expect(tables.map((row) => row.name)).toContain("cli_sessions");
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
db.close();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -334,7 +334,7 @@ describe("Database", () => {
|
||||
});
|
||||
|
||||
it("seeds schema version", () => {
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
});
|
||||
|
||||
it("includes tokenUsageCacheWriteTokens on freshly initialized tasks table", () => {
|
||||
@@ -393,7 +393,7 @@ describe("Database", () => {
|
||||
|
||||
it("is idempotent - calling init() twice does not fail", () => {
|
||||
expect(() => db.init()).not.toThrow();
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
});
|
||||
it("does not overwrite existing config on re-init", () => {
|
||||
// Update the config
|
||||
@@ -1463,7 +1463,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29 (includes v1→v2 through v26→v29)
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -1488,11 +1488,11 @@ describe("schema migrations", () => {
|
||||
const db = new Database(fusionDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
// Re-init should not fail
|
||||
db.init();
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -1527,7 +1527,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
expect(cols.map((col) => col.name)).toContain("priority");
|
||||
@@ -1568,7 +1568,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
const colNames = cols.map((col) => col.name);
|
||||
@@ -1640,7 +1640,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
const colNames = cols.map((col) => col.name);
|
||||
@@ -1880,7 +1880,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(chat_messages)").all() as Array<{ name: string }>;
|
||||
expect(cols.map((col) => col.name)).toContain("attachments");
|
||||
@@ -1954,7 +1954,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "agentRatings" }]);
|
||||
@@ -1978,7 +1978,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "mission_events" }]);
|
||||
@@ -2082,7 +2082,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -2301,7 +2301,7 @@ describe("schema migrations", () => {
|
||||
|
||||
localDb.init();
|
||||
|
||||
expect(localDb.getSchemaVersion()).toBe(109);
|
||||
expect(localDb.getSchemaVersion()).toBe(110);
|
||||
const columns = localDb.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
expect(columns.map((column) => column.name)).toContain("tokenUsageCacheWriteTokens");
|
||||
|
||||
@@ -2612,7 +2612,7 @@ describe("createDatabase factory", () => {
|
||||
const db = createDatabase(fusionDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
expect(db.getLastModified()).toBeGreaterThan(0);
|
||||
|
||||
db.close();
|
||||
@@ -2766,7 +2766,7 @@ describe("migration v77 task token budget columns", () => {
|
||||
|
||||
migrated = new Database(fusion);
|
||||
migrated.init();
|
||||
expect(migrated.getSchemaVersion()).toBe(109);
|
||||
expect(migrated.getSchemaVersion()).toBe(110);
|
||||
const rows = migrated.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
const names = new Set(rows.map((row) => row.name));
|
||||
expect(names.has("tokenBudgetSoftAlertedAt")).toBe(true);
|
||||
@@ -2797,7 +2797,7 @@ describe("migration v106 adds tasks.transitionPending (FN-1417)", () => {
|
||||
const fresh = new Database(fusion);
|
||||
try {
|
||||
fresh.init();
|
||||
expect(fresh.getSchemaVersion()).toBe(109);
|
||||
expect(fresh.getSchemaVersion()).toBe(110);
|
||||
const names = new Set(
|
||||
(fresh.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>).map((r) => r.name),
|
||||
);
|
||||
@@ -2825,7 +2825,7 @@ describe("migration v106 adds tasks.transitionPending (FN-1417)", () => {
|
||||
|
||||
migrated = new Database(fusion);
|
||||
migrated.init();
|
||||
expect(migrated.getSchemaVersion()).toBe(109);
|
||||
expect(migrated.getSchemaVersion()).toBe(110);
|
||||
const names = new Set(
|
||||
(migrated.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>).map((r) => r.name),
|
||||
);
|
||||
@@ -2851,7 +2851,7 @@ describe("migration v107 adds workflow_run_branches + index (FN-1417)", () => {
|
||||
const fresh = new Database(fusion);
|
||||
try {
|
||||
fresh.init();
|
||||
expect(fresh.getSchemaVersion()).toBe(109);
|
||||
expect(fresh.getSchemaVersion()).toBe(110);
|
||||
const table = fresh
|
||||
.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'workflow_run_branches'")
|
||||
.get() as { name: string } | undefined;
|
||||
@@ -2885,7 +2885,7 @@ describe("migration v107 adds workflow_run_branches + index (FN-1417)", () => {
|
||||
|
||||
migrated = new Database(fusion);
|
||||
migrated.init();
|
||||
expect(migrated.getSchemaVersion()).toBe(109);
|
||||
expect(migrated.getSchemaVersion()).toBe(110);
|
||||
const table = migrated
|
||||
.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'workflow_run_branches'")
|
||||
.get() as { name: string } | undefined;
|
||||
@@ -2926,7 +2926,7 @@ describe("migration v67 drops orphan project auth tables", () => {
|
||||
|
||||
migrated = new Database(fusion);
|
||||
migrated.init();
|
||||
expect(migrated.getSchemaVersion()).toBe(109);
|
||||
expect(migrated.getSchemaVersion()).toBe(110);
|
||||
const tables = migrated
|
||||
.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'")
|
||||
.all() as Array<{ name: string }>;
|
||||
@@ -2953,7 +2953,7 @@ describe("migration v67 drops orphan project auth tables", () => {
|
||||
|
||||
try {
|
||||
fresh.init();
|
||||
expect(fresh.getSchemaVersion()).toBe(109);
|
||||
expect(fresh.getSchemaVersion()).toBe(110);
|
||||
const tables = fresh
|
||||
.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'")
|
||||
.all() as Array<{ name: string }>;
|
||||
|
||||
@@ -91,6 +91,6 @@ describe("goals schema", () => {
|
||||
});
|
||||
|
||||
it("reports schema version 101", () => {
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1000,7 +1000,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
// Step 1: Create a fresh database at v33 (runs all migrations up to 33)
|
||||
const db1 = createDatabase(legacyDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(109);
|
||||
expect(db1.getSchemaVersion()).toBe(110);
|
||||
db1.close();
|
||||
|
||||
// Step 2: Manually downgrade to version 32 and drop insight tables
|
||||
@@ -1035,7 +1035,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
expect(tableNamesBefore).not.toContain("project_insight_runs");
|
||||
// Now run init — this triggers the v32→v33 migration
|
||||
db3.init();
|
||||
expect(db3.getSchemaVersion()).toBe(109);
|
||||
expect(db3.getSchemaVersion()).toBe(110);
|
||||
|
||||
// Step 4: Verify insight tables exist after migration
|
||||
const tablesAfter = db3.prepare(
|
||||
@@ -1066,12 +1066,12 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
try {
|
||||
const db1 = createDatabase(testDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(109);
|
||||
expect(db1.getSchemaVersion()).toBe(110);
|
||||
db1.close();
|
||||
|
||||
const db2 = createDatabase(testDir);
|
||||
expect(() => db2.init()).not.toThrow();
|
||||
expect(db2.getSchemaVersion()).toBe(109);
|
||||
expect(db2.getSchemaVersion()).toBe(110);
|
||||
db2.close();
|
||||
} finally {
|
||||
rmSync(testDir, { recursive: true, force: true });
|
||||
@@ -1085,7 +1085,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
// Step 1: Create a fresh DB and run migrations
|
||||
const db1 = createDatabase(compatDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(109);
|
||||
expect(db1.getSchemaVersion()).toBe(110);
|
||||
|
||||
// Step 2: Strip lifecycle and cancelledAt columns by recreating the
|
||||
// table without them. This simulates a DB that was created before the
|
||||
|
||||
@@ -38,7 +38,7 @@ describe("TaskStore merge request record + completion handoff marker", () => {
|
||||
.all() as Array<{ name: string }>;
|
||||
|
||||
expect(tableRows).toEqual([{ name: "completion_handoff_markers" }, { name: "merge_requests" }]);
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
});
|
||||
|
||||
it("upserts merge request records", async () => {
|
||||
|
||||
@@ -3746,7 +3746,7 @@ describe("MissionStore", () => {
|
||||
|
||||
describe("Loop State & Validator Run Schema (v31)", () => {
|
||||
it("schema version is 101 after migration", () => {
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
});
|
||||
|
||||
it("mission_features table has loop state columns", () => {
|
||||
|
||||
@@ -584,7 +584,7 @@ describe("Run Audit", () => {
|
||||
});
|
||||
|
||||
it("schema version is bumped to 40", () => {
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -60,7 +60,7 @@ describe("TaskStore merge queue", () => {
|
||||
expect.arrayContaining(["idx_mergeQueue_lease_ready", "idx_mergeQueue_leaseExpiresAt"]),
|
||||
);
|
||||
|
||||
expect(store.getDatabase().getSchemaVersion()).toBe(109);
|
||||
expect(store.getDatabase().getSchemaVersion()).toBe(110);
|
||||
});
|
||||
|
||||
it("migrates a legacy v88 database and preserves task rows", async () => {
|
||||
|
||||
@@ -51,7 +51,7 @@ describe("TaskStore task documents", () => {
|
||||
|
||||
expect(tableNames.has("task_documents")).toBe(true);
|
||||
expect(tableNames.has("task_document_revisions")).toBe(true);
|
||||
expect(db.getSchemaVersion()).toBe(109);
|
||||
expect(db.getSchemaVersion()).toBe(110);
|
||||
|
||||
const index = db
|
||||
.prepare(
|
||||
|
||||
@@ -149,7 +149,7 @@ export function probeFts5(db: DatabaseSync): boolean {
|
||||
|
||||
// ── Schema Definition ────────────────────────────────────────────────
|
||||
|
||||
const SCHEMA_VERSION = 109;
|
||||
const SCHEMA_VERSION = 110;
|
||||
|
||||
export { SCHEMA_VERSION };
|
||||
|
||||
@@ -1232,6 +1232,7 @@ export const MIGRATION_ONLY_TABLE_SCHEMAS: Record<string, Record<string, string>
|
||||
updatedAt: "TEXT NOT NULL",
|
||||
cliSessionFile: "TEXT",
|
||||
inFlightGeneration: "TEXT",
|
||||
cliExecutorAdapterId: "TEXT",
|
||||
},
|
||||
cli_sessions: {
|
||||
id: "TEXT PRIMARY KEY",
|
||||
|
||||
@@ -116,6 +116,23 @@ describe("neutralizeTerminalOutput", () => {
|
||||
expect(r.carry.length).toBeLessThanOrEqual(MAX_CARRY_LENGTH);
|
||||
});
|
||||
|
||||
it("drops (not flushes) an overflowing OSC 52 prefix so it cannot reconstruct across chunks", () => {
|
||||
// An unterminated OSC 52 grows past MAX_CARRY. The dangerous introducer must
|
||||
// NOT be emitted as literal — otherwise a terminator in the next chunk would
|
||||
// recombine at the client into a working OSC 52 clipboard write.
|
||||
const huge = `${ESC}]52;c;` + "A".repeat(MAX_CARRY_LENGTH + 100);
|
||||
const r1 = neutralizeTerminalOutput(huge, "");
|
||||
// Nothing reconstructable was emitted, and the carry was dropped.
|
||||
expect(r1.output).not.toContain(`${ESC}]`);
|
||||
expect(r1.output).not.toContain("52;");
|
||||
expect(r1.carry).toBe("");
|
||||
// The terminator arriving next has no held introducer to recombine with.
|
||||
const r2 = neutralizeTerminalOutput(`${BEL}visible`, r1.carry);
|
||||
const combined = r1.output + r2.output;
|
||||
expect(combined).not.toContain(`${ESC}]52;`);
|
||||
expect(r2.output).toContain("visible");
|
||||
});
|
||||
|
||||
it("neutralizes a stream with OSC 52, OSC 8 js link, and a DSR query together", () => {
|
||||
const stream =
|
||||
`start${ESC}]52;c;ZXZpbA==${BEL}` +
|
||||
|
||||
@@ -472,6 +472,51 @@ describe("cli-session WS attach", () => {
|
||||
ws.close();
|
||||
});
|
||||
|
||||
it("does not leak an OSC 52 split across the scrollback→live seam", async () => {
|
||||
const ESC = "\x1b";
|
||||
const BEL = "\x07";
|
||||
// The OSC 52 introducer lands at the very TAIL of scrollback (unterminated);
|
||||
// its terminator arrives in the first LIVE chunk. The carry must thread
|
||||
// across the seam so the neutralizer sees the full sequence and strips it —
|
||||
// the scrollback frame must NOT flush the held introducer verbatim.
|
||||
h = await startHarness();
|
||||
h.manager.scrollbackById.set("cli-1", `prior-output${ESC}]52;c;ZXZ`);
|
||||
const t = await mintTicket("cli-1");
|
||||
const ws = connect(h.port, { sessionId: "cli-1", ticket: t });
|
||||
|
||||
const scrollback = await nextMessage(ws, (m) => m.type === "scrollback");
|
||||
const scrollText = decode(scrollback.data);
|
||||
// Normal scrollback still renders; the unterminated tail is withheld (carry).
|
||||
expect(scrollText).toContain("prior-output");
|
||||
expect(scrollText).not.toContain("52;");
|
||||
expect(scrollText).not.toContain(`${ESC}]`);
|
||||
|
||||
// Deliver the terminator in the first live chunk.
|
||||
h.manager.broadcast("cli-1", `pbA==${BEL}after`);
|
||||
let collected = scrollText;
|
||||
await vi.waitFor(
|
||||
() =>
|
||||
new Promise<void>((resolve, reject) => {
|
||||
const onMsg = (raw: Buffer) => {
|
||||
const m = JSON.parse(raw.toString());
|
||||
if (m.type === "data") collected += decode(m.data);
|
||||
if (collected.includes("after")) {
|
||||
ws.off("message", onMsg);
|
||||
resolve();
|
||||
}
|
||||
};
|
||||
ws.on("message", onMsg);
|
||||
setTimeout(() => reject(new Error("timeout")), 1000);
|
||||
}),
|
||||
);
|
||||
// The full sequence, reassembled across the seam, was neutralized.
|
||||
expect(collected).not.toContain("52;");
|
||||
expect(collected).not.toContain(`${ESC}]52`);
|
||||
expect(collected).toContain("prior-output");
|
||||
expect(collected).toContain("after");
|
||||
ws.close();
|
||||
});
|
||||
|
||||
it("read-only session rejects input with an error frame (server-side)", async () => {
|
||||
h = await startHarness({
|
||||
sessions: [makeSession({ id: "cli-ro", purpose: "validator" })],
|
||||
|
||||
@@ -220,10 +220,19 @@ export function neutralizeTerminalOutput(chunk: string, carry = ""): NeutralizeR
|
||||
}
|
||||
|
||||
let newCarry = input.slice(i);
|
||||
// Bound the carry: if a "sequence" never terminates, don't buffer forever —
|
||||
// flush it as literal output (it isn't a recognized hazard if it's this long).
|
||||
// Bound the carry: if a "sequence" never terminates, don't buffer forever.
|
||||
if (newCarry.length > MAX_CARRY_LENGTH) {
|
||||
out += newCarry;
|
||||
// If the overflowing carry begins with a recognized dangerous introducer
|
||||
// (OSC `ESC ]` or DCS `ESC P`), do NOT flush it as literal — emitting the
|
||||
// raw `ESC ]52;…` prefix would let it recombine with a terminator that
|
||||
// arrives in a later chunk and reconstruct the hazardous sequence at the
|
||||
// client. Drop the introducer (and everything held with it) so it can never
|
||||
// be reassembled. Harmless overflow (anything else) is flushed as before.
|
||||
if (newCarry.startsWith(`${ESC}]`) || newCarry.startsWith(`${ESC}P`)) {
|
||||
// Strip the dangerous prefix entirely.
|
||||
} else {
|
||||
out += newCarry;
|
||||
}
|
||||
newCarry = "";
|
||||
} else if (newCarry.length > 0 && !isIncompleteSequence(newCarry)) {
|
||||
// The residual isn't actually a growing prefix — emit it.
|
||||
|
||||
@@ -208,13 +208,20 @@ export function setupCliSessionWebSocket(
|
||||
{
|
||||
const scrollText = Buffer.from(attachment.scrollback).toString("utf8");
|
||||
const result = neutralizeTerminalOutput(scrollText, "");
|
||||
// Flush carry into the scrollback frame (replay is a complete snapshot).
|
||||
const full = result.output + flushTerminalOutput(result.carry);
|
||||
// Thread the carry across the scrollback→live seam. Do NOT flush the
|
||||
// scrollback carry verbatim: if a dangerous sequence (e.g. OSC 52) is
|
||||
// split so its introducer lands at the tail of scrollback and its
|
||||
// terminator arrives in the first live chunk, flushing the held prefix
|
||||
// here would let it recombine at the client and reconstruct the hazard.
|
||||
// Instead we hand the unterminated tail to the live `sendData` carry so
|
||||
// the neutralizer sees the full sequence and strips it. Only the safe,
|
||||
// fully-neutralized prefix is sent in the scrollback frame.
|
||||
carry = result.carry;
|
||||
try {
|
||||
ws.send(
|
||||
JSON.stringify({
|
||||
type: "scrollback",
|
||||
data: Buffer.from(full, "utf8").toString("base64"),
|
||||
data: Buffer.from(result.output, "utf8").toString("base64"),
|
||||
}),
|
||||
);
|
||||
} catch {
|
||||
@@ -235,6 +242,25 @@ export function setupCliSessionWebSocket(
|
||||
} catch {
|
||||
/* stream error — close below */
|
||||
}
|
||||
// Flush any residual carry at true stream end. The held bytes are an
|
||||
// unterminated tail; no further chunk can arrive to recombine with them,
|
||||
// so emitting them as literal is safe and avoids losing trailing output.
|
||||
if (!streamClosed && ws.readyState === ws.OPEN && carry.length > 0) {
|
||||
const tail = flushTerminalOutput(carry);
|
||||
carry = "";
|
||||
if (tail.length > 0) {
|
||||
try {
|
||||
ws.send(
|
||||
JSON.stringify({
|
||||
type: "data",
|
||||
data: Buffer.from(tail, "utf8").toString("base64"),
|
||||
}),
|
||||
);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!streamClosed && ws.readyState === ws.OPEN) {
|
||||
try {
|
||||
ws.send(JSON.stringify({ type: "exit" }));
|
||||
|
||||
@@ -316,6 +316,20 @@ describe("CliTaskSession (U7)", () => {
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
expect(pty().written.length).toBeGreaterThan(writesBefore);
|
||||
expect(pty().written.some((w) => w.includes("follow-up"))).toBe(true);
|
||||
|
||||
// The follow-up drove the machine done→busy, so the re-armed result promise
|
||||
// must resolve on the NEXT positive done (it would hang forever if the
|
||||
// machine were left parked in `done`, since signalDone-from-done is a no-op).
|
||||
let resolved = false;
|
||||
const next = session.result().then((o) => {
|
||||
resolved = true;
|
||||
return o;
|
||||
});
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
expect(resolved).toBe(false);
|
||||
hub.ingest(session.sessionId, { kind: "done" });
|
||||
const outcome = await next;
|
||||
expect(outcome.kind).toBe("success");
|
||||
});
|
||||
|
||||
it("follow-up returns false when the adapter does not support resume (caller launches fresh)", async () => {
|
||||
|
||||
@@ -32,6 +32,20 @@ describe("GenericCliAdapter capabilities", () => {
|
||||
const asInterface: CliAgentAdapter = adapter;
|
||||
expect(asInterface.buildResume).toBeUndefined();
|
||||
});
|
||||
|
||||
it("formatInjection only appends CR and never wraps bracketed paste (no double-wrap)", () => {
|
||||
// Bracketed-paste wrapping is the session manager's responsibility; the
|
||||
// adapter must not also wrap, otherwise the payload double-wraps.
|
||||
expect(adapter.formatInjection("hello", { bracketedPasteActive: false })).toEqual({
|
||||
payload: "hello\r",
|
||||
});
|
||||
expect(adapter.formatInjection("hello", { bracketedPasteActive: true })).toEqual({
|
||||
payload: "hello\r",
|
||||
});
|
||||
const wrapped = adapter.formatInjection("multi\nline", { bracketedPasteActive: true });
|
||||
expect(wrapped.payload).not.toContain("\x1b[200~");
|
||||
expect(wrapped.payload).not.toContain("\x1b[201~");
|
||||
});
|
||||
});
|
||||
|
||||
// ── buildLaunch / env ────────────────────────────────────────────────────────
|
||||
|
||||
@@ -388,14 +388,12 @@ export class GenericCliAdapter implements CliAgentAdapter {
|
||||
return new GenericReadinessDetector();
|
||||
}
|
||||
|
||||
formatInjection(text: string, opts: { bracketedPasteActive: boolean }): CliInjectionFormat {
|
||||
// Submit with a carriage return. Bracketed-paste wrapping is handled by the
|
||||
// session manager's security path; the generic adapter only decides submit
|
||||
// semantics. When the child negotiated bracketed paste, wrap so multi-line
|
||||
// text is delivered atomically before the submit CR.
|
||||
if (opts.bracketedPasteActive) {
|
||||
return { payload: `\x1b[200~${text}\x1b[201~\r` };
|
||||
}
|
||||
formatInjection(text: string, _opts: { bracketedPasteActive: boolean }): CliInjectionFormat {
|
||||
// Submit with a carriage return only. Bracketed-paste wrapping is handled
|
||||
// exclusively by the session manager's security path; if we also wrapped
|
||||
// here when bracketedPasteActive, the payload would be double-wrapped with
|
||||
// \x1b[200~...\x1b[201~. Like every native adapter, generic ignores the
|
||||
// bracketedPasteActive hint and only appends \r.
|
||||
return { payload: `${text}\r` };
|
||||
}
|
||||
|
||||
|
||||
@@ -397,6 +397,22 @@ export class CliTaskSession {
|
||||
// Re-arm the result promise so the next done resolves it again.
|
||||
if (this.settled) this.rearm();
|
||||
this.subscribe();
|
||||
// Drive the authoritative state machine done→busy BEFORE injecting. If we
|
||||
// leave it parked in `done`, the next native done is idempotent (no state
|
||||
// change emitted), so onMachineState("done") never fires and the re-armed
|
||||
// result promise never resolves. The hub swallows signalBusy-from-done, so
|
||||
// we transition via the machine's followUp()/injectPrompt directly.
|
||||
const machine = this.hub.getStateMachine(this.sessionId);
|
||||
if (machine) {
|
||||
try {
|
||||
if (machine.getState() === "done") machine.followUp();
|
||||
else if (machine.getState() === "ready" || machine.getState() === "resuming") {
|
||||
machine.injectPrompt();
|
||||
}
|
||||
} catch {
|
||||
// best-effort transition
|
||||
}
|
||||
}
|
||||
await this.manager.inject(this.sessionId, prompt);
|
||||
this.log(`cli-task-session ${this.sessionId}: follow-up injected (live resume)`);
|
||||
return true;
|
||||
|
||||
Reference in New Issue
Block a user