authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-02-26 00:42:57-08:00
committergravatar for jmarminge@gmail.comJulius Marminge <jmarminge@gmail.com> 2026-02-26 13:20:36-08:00
logaa5a144d04c5067b9344289c6bc6f28a8acb9765
tree9b9499df10b6fce045c53e56d26bcb4554581fee
parentc6d1f92ba9f2a820cdbee71ff6b9499d1886bba6

small nits

7 files changed, 48 insertions(+), 21 deletions(-)

.plans/16c-pr89-remediation-checklist.md+4-4
...@@ -286,8 +286,8 @@ Counts: active `51` (`valid=33`, `partially-valid=18`), closed-invalid `6`...@@ -286,8 +286,8 @@ Counts: active `51` (`valid=33`, `partially-valid=18`), closed-invalid `6`
286 - Threads: PRRT_kwDORLtfbc5wj4cM286 - Threads: PRRT_kwDORLtfbc5wj4cM
287 - Audit note: -ilc shell invocation can pollute captured PATH output.287 - Audit note: -ilc shell invocation can pollute captured PATH output.
288288
289- [ ] `C029` `parseFileUrlHref` already decodes the path (line 46), but `safeDecode` is called again here, corrupting filenames containing `%` sequences. Consider skipping the decode when `fileUrlTarget` is non-null.289- [x] `C029` `parseFileUrlHref` already decodes the path (line 46), but `safeDecode` is called again here, corrupting filenames containing `%` sequences. Consider skipping the decode when `fileUrlTarget` is non-null.
290 - Status: `TODO`290 - Status: `DONE`
291 - Verdict: `valid`291 - Verdict: `valid`
292 - Severity: `Medium`292 - Severity: `Medium`
293 - Area: `Edge-case parsing/platform behavior`293 - Area: `Edge-case parsing/platform behavior`
...@@ -295,8 +295,8 @@ Counts: active `51` (`valid=33`, `partially-valid=18`), closed-invalid `6`...@@ -295,8 +295,8 @@ Counts: active `51` (`valid=33`, `partially-valid=18`), closed-invalid `6`
295 - Threads: PRRT_kwDORLtfbc5wnVsU295 - Threads: PRRT_kwDORLtfbc5wnVsU
296 - Audit note: file URL decoding still double-decodes in one path.296 - Audit note: file URL decoding still double-decodes in one path.
297297
298- [ ] `C030` `EXTERNAL_SCHEME_PATTERN` matches `script.ts:10` as a scheme because `.ts:` looks like `scheme:`. Consider requiring `://` after the colon, or checking that what follows the colon is not just digits. <details> <summary>🚀 Reply "<strong>fix it for me</strong>" or copy this <strong>AI Prompt</strong> for your agent:</summary>298- [x] `C030` `EXTERNAL_SCHEME_PATTERN` matches `script.ts:10` as a scheme because `.ts:` looks like `scheme:`. Consider requiring `://` after the colon, or checking that what follows the colon is not just digits. <details> <summary>🚀 Reply "<strong>fix it for me</strong>" or copy this <strong>AI Prompt</strong> for your agent:</summary>
299 - Status: `TODO`299 - Status: `DONE`
300 - Verdict: `valid`300 - Verdict: `valid`
301 - Severity: `Medium`301 - Severity: `Medium`
302 - Area: `Edge-case parsing/platform behavior`302 - Area: `Edge-case parsing/platform behavior`
apps/server/src/checkpointing/Layers/CheckpointDiffQuery.test.ts+4-8
...@@ -14,10 +14,6 @@ import { CheckpointDiffQueryLive } from "./CheckpointDiffQuery.ts";...@@ -14,10 +14,6 @@ import { CheckpointDiffQueryLive } from "./CheckpointDiffQuery.ts";
14import { CheckpointStore, type CheckpointStoreShape } from "../Services/CheckpointStore.ts";14import { CheckpointStore, type CheckpointStoreShape } from "../Services/CheckpointStore.ts";
15import { CheckpointDiffQuery } from "../Services/CheckpointDiffQuery.ts";15import { CheckpointDiffQuery } from "../Services/CheckpointDiffQuery.ts";
1616
17const asProjectId = (value: string): ProjectId => ProjectId.makeUnsafe(value);
18const asTurnId = (value: string): TurnId => TurnId.makeUnsafe(value);
19const asCheckpointRef = (value: string): CheckpointRef => CheckpointRef.makeUnsafe(value);
20
21function makeSnapshot(input: {17function makeSnapshot(input: {
22 readonly projectId: ProjectId;18 readonly projectId: ProjectId;
23 readonly threadId: ThreadId;19 readonly threadId: ThreadId;
...@@ -49,7 +45,7 @@ function makeSnapshot(input: {...@@ -49,7 +45,7 @@ function makeSnapshot(input: {
49 model: "gpt-5-codex",45 model: "gpt-5-codex",
50 branch: null,46 branch: null,
51 worktreePath: input.worktreePath,47 worktreePath: input.worktreePath,
52 latestTurnId: asTurnId("turn-1"),48 latestTurnId: TurnId.makeUnsafe("turn-1"),
53 createdAt: "2026-01-01T00:00:00.000Z",49 createdAt: "2026-01-01T00:00:00.000Z",
54 updatedAt: "2026-01-01T00:00:00.000Z",50 updatedAt: "2026-01-01T00:00:00.000Z",
55 deletedAt: null,51 deletedAt: null,
...@@ -57,7 +53,7 @@ function makeSnapshot(input: {...@@ -57,7 +53,7 @@ function makeSnapshot(input: {
57 activities: [],53 activities: [],
58 checkpoints: [54 checkpoints: [
59 {55 {
60 turnId: asTurnId("turn-1"),56 turnId: TurnId.makeUnsafe("turn-1"),
61 checkpointTurnCount: input.checkpointTurnCount,57 checkpointTurnCount: input.checkpointTurnCount,
62 checkpointRef: input.checkpointRef,58 checkpointRef: input.checkpointRef,
63 status: "ready",59 status: "ready",
...@@ -74,9 +70,9 @@ function makeSnapshot(input: {...@@ -74,9 +70,9 @@ function makeSnapshot(input: {
7470
75describe("CheckpointDiffQueryLive", () => {71describe("CheckpointDiffQueryLive", () => {
76 it("computes diffs using canonical turn-0 checkpoint refs", async () => {72 it("computes diffs using canonical turn-0 checkpoint refs", async () => {
77 const projectId = asProjectId("project-1");73 const projectId = ProjectId.makeUnsafe("project-1");
78 const threadId = ThreadId.makeUnsafe("thread-1");74 const threadId = ThreadId.makeUnsafe("thread-1");
79 const toCheckpointRef = asCheckpointRef("refs/t3/checkpoints/thread-1/turn/1");75 const toCheckpointRef = checkpointRefForThreadTurn(threadId, 1);
80 const hasCheckpointRefCalls: Array<CheckpointRef> = [];76 const hasCheckpointRefCalls: Array<CheckpointRef> = [];
81 const diffCheckpointsCalls: Array<{77 const diffCheckpointsCalls: Array<{
82 readonly fromCheckpointRef: CheckpointRef;78 readonly fromCheckpointRef: CheckpointRef;
apps/server/src/codexAppServerManager.ts+1-1
...@@ -986,7 +986,7 @@ function readResumeThreadId(input: ProviderSessionStartInput): ProviderThreadId...@@ -986,7 +986,7 @@ function readResumeThreadId(input: ProviderSessionStartInput): ProviderThreadId
986 typeof input.resumeCursor !== "object" ||986 typeof input.resumeCursor !== "object" ||
987 Array.isArray(input.resumeCursor)987 Array.isArray(input.resumeCursor)
988 ) {988 ) {
989 return undefined;989 return input.resumeThreadId;
990 }990 }
991 const rawThreadId = (input.resumeCursor as Record<string, unknown>).threadId;991 const rawThreadId = (input.resumeCursor as Record<string, unknown>).threadId;
992 return typeof rawThreadId === "string" ? toProviderThreadId(rawThreadId) : input.resumeThreadId;992 return typeof rawThreadId === "string" ? toProviderThreadId(rawThreadId) : input.resumeThreadId;
apps/server/src/open.ts+11-4
...@@ -85,10 +85,17 @@ export const resolveEditorLaunch = Effect.fnUntraced(function* (...@@ -85,10 +85,17 @@ export const resolveEditorLaunch = Effect.fnUntraced(function* (
8585
86export const launchDetached = (launch: EditorLaunch) =>86export const launchDetached = (launch: EditorLaunch) =>
87 Effect.callback<void, OpenError>((resume) => {87 Effect.callback<void, OpenError>((resume) => {
88 const child = spawn(launch.command, [...launch.args], {88 let child;
89 detached: true,89 try {
90 stdio: "ignore",90 child = spawn(launch.command, [...launch.args], {
91 });91 detached: true,
92 stdio: "ignore",
93 });
94 } catch (error) {
95 return resume(
96 Effect.fail(new OpenError({ message: "failed to spawn detached process", cause: error })),
97 );
98 }
9299
93 const handleSpawn = () => {100 const handleSpawn = () => {
94 child.unref();101 child.unref();
apps/server/src/terminal/Layers/Manager.ts+4-1
...@@ -1033,7 +1033,10 @@ export const TerminalManagerLive = Layer.effect(...@@ -1033,7 +1033,10 @@ export const TerminalManagerLive = Layer.effect(
1033 const logsDir = join(stateDir, "logs", "terminals");1033 const logsDir = join(stateDir, "logs", "terminals");
10341034
1035 const ptyAdapter = yield* PtyAdapter;1035 const ptyAdapter = yield* PtyAdapter;
1036 const runtime = new TerminalManagerRuntime({ logsDir, ptyAdapter });1036 const runtime = yield* Effect.acquireRelease(
1037 Effect.sync(() => new TerminalManagerRuntime({ logsDir, ptyAdapter })),
1038 (r) => Effect.sync(() => r.dispose()),
1039 );
10371040
1038 return {1041 return {
1039 open: (input) => Effect.promise(() => runtime.open(input)),1042 open: (input) => Effect.promise(() => runtime.open(input)),
apps/web/src/markdown-links.test.ts+12
...@@ -15,6 +15,12 @@ describe("resolveMarkdownFileLinkTarget", () => {...@@ -15,6 +15,12 @@ describe("resolveMarkdownFileLinkTarget", () => {
15 );15 );
16 });16 });
1717
18 it("does not treat filename line references as external schemes", () => {
19 expect(resolveMarkdownFileLinkTarget("script.ts:10", "/Users/julius/project")).toBe(
20 "/Users/julius/project/script.ts:10",
21 );
22 });
23
18 it("resolves bare file names against cwd", () => {24 it("resolves bare file names against cwd", () => {
19 expect(resolveMarkdownFileLinkTarget("AGENTS.md", "/Users/julius/project")).toBe(25 expect(resolveMarkdownFileLinkTarget("AGENTS.md", "/Users/julius/project")).toBe(
20 "/Users/julius/project/AGENTS.md",26 "/Users/julius/project/AGENTS.md",
...@@ -31,6 +37,12 @@ describe("resolveMarkdownFileLinkTarget", () => {...@@ -31,6 +37,12 @@ describe("resolveMarkdownFileLinkTarget", () => {
31 expect(resolveMarkdownFileLinkTarget("https://example.com/docs")).toBeNull();37 expect(resolveMarkdownFileLinkTarget("https://example.com/docs")).toBeNull();
32 });38 });
3339
40 it("does not double-decode file URLs", () => {
41 expect(resolveMarkdownFileLinkTarget("file:///Users/julius/project/file%2520name.md")).toBe(
42 "/Users/julius/project/file%20name.md",
43 );
44 });
45
34 it("does not treat app routes as file links", () => {46 it("does not treat app routes as file links", () => {
35 expect(resolveMarkdownFileLinkTarget("/chat/settings")).toBeNull();47 expect(resolveMarkdownFileLinkTarget("/chat/settings")).toBeNull();
36 });48 });
apps/web/src/markdown-links.ts+12-3
...@@ -2,11 +2,12 @@ import { resolvePathLinkTarget } from "./terminal-links";...@@ -2,11 +2,12 @@ import { resolvePathLinkTarget } from "./terminal-links";
22
3const WINDOWS_DRIVE_PATH_PATTERN = /^[A-Za-z]:[\\/]/;3const WINDOWS_DRIVE_PATH_PATTERN = /^[A-Za-z]:[\\/]/;
4const WINDOWS_UNC_PATH_PATTERN = /^\\\\/;4const WINDOWS_UNC_PATH_PATTERN = /^\\\\/;
5const EXTERNAL_SCHEME_PATTERN = /^[A-Za-z][A-Za-z0-9+.-]*:/;5const EXTERNAL_SCHEME_PATTERN = /^([A-Za-z][A-Za-z0-9+.-]*):(.*)$/;
6const RELATIVE_PATH_PREFIX_PATTERN = /^(~\/|\.{1,2}\/)/;6const RELATIVE_PATH_PREFIX_PATTERN = /^(~\/|\.{1,2}\/)/;
7const RELATIVE_FILE_PATH_PATTERN = /^[A-Za-z0-9._-]+(?:\/[A-Za-z0-9._-]+)+(?::\d+){0,2}$/;7const RELATIVE_FILE_PATH_PATTERN = /^[A-Za-z0-9._-]+(?:\/[A-Za-z0-9._-]+)+(?::\d+){0,2}$/;
8const RELATIVE_FILE_NAME_PATTERN = /^[A-Za-z0-9._-]+\.[A-Za-z0-9_-]+(?::\d+){0,2}$/;8const RELATIVE_FILE_NAME_PATTERN = /^[A-Za-z0-9._-]+\.[A-Za-z0-9_-]+(?::\d+){0,2}$/;
9const POSITION_SUFFIX_PATTERN = /:\d+(?::\d+)?$/;9const POSITION_SUFFIX_PATTERN = /:\d+(?::\d+)?$/;
10const POSITION_ONLY_PATTERN = /^\d+(?::\d+)?$/;
10const POSIX_FILE_ROOT_PREFIXES = [11const POSIX_FILE_ROOT_PREFIXES = [
11 "/Users/",12 "/Users/",
12 "/home/",13 "/home/",
...@@ -89,6 +90,14 @@ function isRelativePath(path: string): boolean {...@@ -89,6 +90,14 @@ function isRelativePath(path: string): boolean {
89 );90 );
90}91}
9192
93function hasExternalScheme(path: string): boolean {
94 const match = path.match(EXTERNAL_SCHEME_PATTERN);
95 if (!match) return false;
96 const rest = match[2] ?? "";
97 if (rest.startsWith("//")) return true;
98 return !POSITION_ONLY_PATTERN.test(rest);
99}
100
92export function resolveMarkdownFileLinkTarget(101export function resolveMarkdownFileLinkTarget(
93 href: string | undefined,102 href: string | undefined,
94 cwd?: string,103 cwd?: string,
...@@ -101,14 +110,14 @@ export function resolveMarkdownFileLinkTarget(...@@ -101,14 +110,14 @@ export function resolveMarkdownFileLinkTarget(
101 ? parseFileUrlHref(rawHref)110 ? parseFileUrlHref(rawHref)
102 : null;111 : null;
103 const source = fileUrlTarget ?? stripSearchAndHash(rawHref);112 const source = fileUrlTarget ?? stripSearchAndHash(rawHref);
104 const decodedPath = safeDecode(source.path.trim());113 const decodedPath = fileUrlTarget ? source.path.trim() : safeDecode(source.path.trim());
105 const decodedHash = safeDecode(source.hash.trim());114 const decodedHash = safeDecode(source.hash.trim());
106115
107 if (decodedPath.length === 0) return null;116 if (decodedPath.length === 0) return null;
108 if (117 if (
109 !WINDOWS_DRIVE_PATH_PATTERN.test(decodedPath) &&118 !WINDOWS_DRIVE_PATH_PATTERN.test(decodedPath) &&
110 !WINDOWS_UNC_PATH_PATTERN.test(decodedPath) &&119 !WINDOWS_UNC_PATH_PATTERN.test(decodedPath) &&
111 EXTERNAL_SCHEME_PATTERN.test(decodedPath)120 hasExternalScheme(decodedPath)
112 ) {121 ) {
113 return null;122 return null;
114 }123 }