fix markdown reader 404 on absolute paths

resolveUnderRoot joined an absolute input onto the workspace root
(/repo + /repo/x → /repo/repo/x), so files.read 404'd on a file that
exists. The Claude Config tab hands the reader a skill's absolute
SKILL.md path, which hit this. Normalize an absolute input as-is; the
existing containment check still rejects absolute paths outside the
root, so the T-102 boundary is preserved.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-01 08:50:00 +02:00
co-authored by Claude Opus 4.8
parent 4aed6c12a5
commit ade8a88b75
8 changed files with 52 additions and 1 deletions
@@ -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;
@@ -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;
+17
View File
@@ -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);
+1
View File
@@ -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);
+4
View File
@@ -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)
+6 -1
View File
@@ -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
+10
View File
@@ -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);
+12
View File
@@ -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<PathOutsideRoot>()));
});
test('PathOutsideRoot.toString embeds requested + resolved + root', () {
final e = PathOutsideRoot('r', '/abs', '/root');
expect(e.toString(), allOf(contains('r'), contains('/abs'), contains('/root')));