feat(web): render tool evidence and preview session artifacts - #496
Conversation
tt-a1i
left a comment
There was a problem hiding this comment.
Reviewed at exact head 3a81319.
Standards
[P1] The Web-only write wrapper silently adds read authority to every permitted write. It opens pre/post snapshots with O_RDONLY and persists up to 32 KiB of the old content in details.diff, without passing those reads through Pi’s read tool or permission/trust hooks. A policy may allow overwriting a path while denying disclosure of its contents. Please obtain evidence from a Pi-owned authorized mutation primitive, require explicit read admission, or omit the pre-image when read authority is unavailable.
[P2] Every visible preview polls every two seconds and rereads, allocates, and hashes the full artifact (up to 20 MiB), indefinitely. Concurrency is bounded but cumulative I/O is not. Poll bounded metadata/revision identity and fetch bytes only after a change or explicit refresh/backoff boundary.
Spec
[P1] Windows absolute Markdown artifact links never reach the new resolver. rehypeSanitize removes C: / C:/ hrefs before the custom local-link component runs. Exact-head SSR rendered both C:\\work\\repo\\report.md and C:/work/repo/report.md without an artifact button, violating the issue’s explicit absolute-path acceptance. Add a component/browser regression that safely preserves Windows local paths without enabling arbitrary URL schemes.
[P2] Successful foreground terminal/test evidence projects state: returned but processState: unknown, so the UI reports “Process: unknown · exit code unavailable” even for canonical success. Project completed/returned without inventing exit code 0.
tt-a1i
left a comment
There was a problem hiding this comment.
Reviewed fd8c4f9 after current-main integration. Standards: no remaining actionable blocker identified. Spec: The unauthorized preimage reads are removed. Artifact access is explicit, Session/workspace scoped, bounded, and checked through an opened file identity. Windows local links and polling bounds address the old findings. No P0/P1 found. This supersedes findings against older implementations where stated; merge remains conditional on current required CI and branch protection.
# Conflicts: # web/dist/app.js # web/dist/styles.css
tt-a1i
left a comment
There was a problem hiding this comment.
最新 main 集成修复已完成,重新生成 Web 产物。双线复审保留既有侧栏焦点行为和本 PR 功能。 bun run check 已通过;完整测试已执行或正在串行验证,本机部分测试存在资源竞争超时,最终状态以最新 CI 为准。未发现 P0/P1。此账户推送了修复,仍需另一位维护者满足最后一次推送后的独立批准。
# Conflicts: # web/dist/app.js # web/dist/styles.css # web/host/web-host.ts # web/protocol/types.ts # web/ui/src/protocol/client.ts
Problem
Closes #345. Generic tool output makes file changes and execution evidence hard to review, and ordinary local links cannot reach authenticated artifacts. Includes the preview/download and revision-following scope requested in the maintainer comments:
Value
Users can inspect bounded file, change, test and terminal evidence, then preview/download a generated report and follow its revision. Presentation remains tied to canonical results; unknown tools retain a sanitized raw fallback.
Approach
Validation
Latest repair commit:
a7159b0.bun run test: passed, including Windows serial background-process tests and 10 Vitest files / 135 tests.canonicalWorkspace.split("/"), leaving a full Windows path instead of its menu label and timing out. Archived history passed. The complete browser suite was not repeated after the final prefix repair; its component regressions and artifact browser scenario passed.bun run check: passed in a temporary LF copy of the final source. The Windows checkout still fails formatting on existing CRLF files; no repository-wide rewrite was made.git diff --check: passed. New remote CI results are separate from local validation.Impact