diff --git a/.pql/changelog/ticket_history/2026-05.sql b/.pql/changelog/ticket_history/2026-05.sql index efe3ae8e..65f2ac5c 100644 --- a/.pql/changelog/ticket_history/2026-05.sql +++ b/.pql/changelog/ticket_history/2026-05.sql @@ -2378,3 +2378,4 @@ INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-52', 'status', 'in_progress', 'done', NULL, '2026-05-31 18:54:02', '2026-05-31 18:54:02', '2026-05-31 18:54:02', NULL, 'bac566c015a7d599cbb2fc9b3855400b', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-51', 'status', 'in_progress', 'done', NULL, '2026-05-31 18:54:02', '2026-05-31 18:54:02', '2026-05-31 18:54:02', NULL, 'fafab627f731815e6f83bfaf310543a6', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-53', 'status', 'backlog', 'in_progress', NULL, '2026-05-31 19:28:39', '2026-05-31 19:28:39', '2026-05-31 19:28:39', NULL, '5e29490c3b36aa2dce8dece3d0d0091f', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-53', 'status', 'in_progress', 'done', NULL, '2026-05-31 19:41:13', '2026-05-31 19:41:13', '2026-05-31 19:41:13', NULL, 'f5297165f78050d960c9301633d06a5c', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql new file mode 100644 index 00000000..fb6397cf --- /dev/null +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -0,0 +1 @@ +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-194', 'status', 'backlog', 'done', NULL, '2026-06-01 06:49:47', '2026-06-01 06:49:47', '2026-06-01 06:49:47', NULL, '09bfe6f27513d7d16f7139543961b4f8', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-05.sql b/.pql/changelog/tickets/2026-05.sql index 2cae882e..9b3b13d4 100644 --- a/.pql/changelog/tickets/2026-05.sql +++ b/.pql/changelog/tickets/2026-05.sql @@ -3060,3 +3060,20 @@ Safety (user decision) — REQUIRE A CLEAN GIT WORKING TREE before apply: refuse Ignore — same full ignore_files: layering as T-52 (D-4). Files: lib/src/daemon/files_commands.dart (files.write); new replacement-preview controller/view under lib/builtin/search/; reuse path_safety + diff render primitives; DialogRouter confirm; IPC schema + CLI parity verbs.', 'in_progress', 'medium', NULL, NULL, NULL, '2026-04-23 20:32:06', '2026-05-31 19:28:39', NULL, 'fb6ccea7b9f4b2a2f403f7ade153cfeb', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-53', 'story', NULL, 'search and replace across files', 'Companion to find-in-files. Preview all replacements before applying. Support regex capture groups in replacement. Respects ignore patterns from pql config. + +Refinement (2026-05-31): + +Depends on T-52 — consumes its match model (path, line, start/end offset per match, capture groups) as the replacement input. Sequence after T-52 (T-52 blocks T-53). + +Write path — no files.write IPC exists today (only files.root/read/ls/watch; the sole write path is EditorRegistry.save on an open buffer). Add a `files.write` verb gated by resolveUnderRootFollowingSymlinks (lib/src/files/path_safety.dart). Do NOT route through the editor (would pollute the open-buffer list with dozens of temp buffers). + +Regex capture groups — Dart replaceAllMapped / $1; the T-52 engine must surface match groups. + +Preview — the existing diff view (lib/builtin/diff) is git-only (hardwired to git.diff). Build a ReplacementPreviewController/view that reuses the diff RENDER primitives (DiffLine / _HunkView styling) fed a computed in-memory before/after set. Do NOT generalize/entangle the git DiffController. + +Safety (user decision) — REQUIRE A CLEAN GIT WORKING TREE before apply: refuse with "commit or stash your changes first" if there are unstaged changes, making git the lossless undo layer. Preview shown first; final confirmation via the existing DialogRouter (as used by git discard-confirm). + +Ignore — same full ignore_files: layering as T-52 (D-4). + +Files: lib/src/daemon/files_commands.dart (files.write); new replacement-preview controller/view under lib/builtin/search/; reuse path_safety + diff render primitives; DialogRouter confirm; IPC schema + CLI parity verbs.', 'done', 'medium', NULL, NULL, NULL, '2026-04-23 20:32:06', '2026-05-31 19:41:13', NULL, 'd3de16341cfef833829cc83c5ff76995', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql new file mode 100644 index 00000000..43c30f22 --- /dev/null +++ b/.pql/changelog/tickets/2026-06.sql @@ -0,0 +1 @@ +INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-194', 'bug', 'T-132', 'Markdown reader 404s on absolute paths handed to files.read', 'The sidebar markdown reader (and any files.read caller) was handed an absolute path (e.g. the Claude Config tab publishing a skill''s absolute SKILL.md manifestPath). resolveUnderRoot joined the absolute path onto the workspace root (/repo + /repo/x = /repo/repo/x), which does not exist, so the reader showed ''file not found'' for a file that exists. Fix: resolveUnderRoot normalizes an absolute input as-is and relies on the existing containment check (so absolute-under-root resolves, absolute-outside still throws PathOutsideRoot). Found live opening .claude/skills/frame0-wireframe/SKILL.md.', 'done', 'high', NULL, NULL, NULL, '2026-06-01 06:49:25', '2026-06-01 06:49:47', NULL, '01a3a891c9786f7afb7a1b95e8ab0b2f', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index fb01271e..2bd07f66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,10 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- The markdown reader opens files given an absolute path again (e.g. a skill's + `SKILL.md` from the Claude Config tab). `resolveUnderRoot` no longer doubles an + absolute path onto the workspace root; absolute-under-root resolves, while + paths outside the root are still rejected. (T-194) - The composer slash typeahead now lists clide-owned commands — `/resume` and `/fork` (and `/clear`) surface even though the CLI probe doesn't advertise them, unioned onto whatever command source the composer uses. (T-162) diff --git a/lib/src/files/path_safety.dart b/lib/src/files/path_safety.dart index b31dba46..3357b8ef 100644 --- a/lib/src/files/path_safety.dart +++ b/lib/src/files/path_safety.dart @@ -26,7 +26,12 @@ class PathOutsideRoot implements Exception { /// without touching disk (T-102). String resolveUnderRoot(Directory root, String relative) { final rootPath = _normalize(root.absolute.path); - final joined = _normalize('$rootPath${Platform.pathSeparator}$relative'); + // An absolute input is normalized as-is rather than joined onto the + // root — otherwise a path already under the root gets doubled + // (`/repo` + `/repo/x` → `/repo/repo/x`) and resolves to nothing. + // Containment is still enforced below, so an absolute path *outside* + // the root is rejected exactly as a `..` traversal is. + final joined = relative.startsWith(Platform.pathSeparator) ? _normalize(relative) : _normalize('$rootPath${Platform.pathSeparator}$relative'); // Containment check: joined must equal rootPath, or start with // rootPath + separator. Equality covers `relative == ''` (the diff --git a/test/daemon/files_commands_test.dart b/test/daemon/files_commands_test.dart index 876d17d3..7bb0355d 100644 --- a/test/daemon/files_commands_test.dart +++ b/test/daemon/files_commands_test.dart @@ -97,6 +97,16 @@ void main() { expect(r.data['path'], 'README.md'); }); + test('files.read accepts an absolute path under the workspace root', () async { + // Regression: the markdown reader publishes absolute skill paths + // (e.g. .claude/skills/.../SKILL.md). An absolute path under root + // must resolve, not double onto the root and 404. + final abs = '${sandbox.absolute.path}/README.md'; + final r = await call('files.read', {'path': abs}); + expect(r.ok, isTrue); + expect(r.data['content'], 'hi'); + }); + test('files.read without a path returns toolError', () async { final r = await call('files.read', const {}); expect(r.ok, isFalse); diff --git a/test/files/path_safety_test.dart b/test/files/path_safety_test.dart index 8e477c5b..4f73ea10 100644 --- a/test/files/path_safety_test.dart +++ b/test/files/path_safety_test.dart @@ -59,6 +59,18 @@ void main() { } }); + test('accepts an absolute path already under the root (no doubling)', () { + // Regression: an absolute path under the root used to be joined + // onto root (`/repo` + `/repo/x` → `/repo/repo/x`) and resolve to + // nothing. It must normalize as-is. + final abs = '${root.absolute.path}/.claude/skills/x/SKILL.md'; + expect(resolveUnderRoot(root, abs), abs); + }); + + test('rejects an absolute path outside the root', () { + expect(() => resolveUnderRoot(root, '/etc/passwd'), throwsA(isA())); + }); + test('PathOutsideRoot.toString embeds requested + resolved + root', () { final e = PathOutsideRoot('r', '/abs', '/root'); expect(e.toString(), allOf(contains('r'), contains('/abs'), contains('/root')));