docs(roadmap): add worker restart evidence scope gap

This commit is contained in:
Yeachan-Heo
2026-05-22 14:30:51 +00:00
parent ac0903362c
commit b0bca2ea4f

View File

@@ -6719,3 +6719,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
582. **Terminal table renderer ignores markdown alignment markers, so right/center-aligned numeric columns are rendered left-aligned with no test coverage** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 13:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@ccb319a`. Active tmux session at probe time: `omx-issue-2466-ultragoal-get-goal-recovery-resume`; no active claw-code implementation session. Code inspection: `rust/crates/rusty-claude-cli/src/render.rs` starts tables with `Event::Start(Tag::Table(..))` but discards the alignment vector carried by pulldown-cmark. `TableState` stores headers/rows/current cells only, and `render_table_row` always writes the cell text followed by padding spaces (`left` alignment) regardless of whether the markdown separator uses `:---`, `---:`, or `:---:`. Existing test `renders_tables_with_alignment` checks column width alignment only, using `| ---- | ----- |`, and does not cover right/center alignment semantics. **Required fix shape:** (a) store pulldown-cmark table alignment metadata in `TableState`; (b) apply left/center/right padding in `render_table_row` for headers and body cells; (c) add tests for `| left | center | right |` with `| :--- | :---: | ---: |`, including numeric columns; (d) ensure `visible_width` still handles ANSI-styled header cells correctly when padding is split before/after centered text; (e) document whether terminal rendering intentionally supports GitHub-style table alignment. **Why this matters:** tables are common in status reports, benchmarks, and cost/test summaries. Dropping right alignment makes numeric columns harder to scan and means the terminal renderer does not faithfully reflect markdown that users expect from GitHub/Discord-style output. Source: gaebal-gajae dogfood response to Clawhip message `1507375004671672391` on 2026-05-22.
583. **`PermissionEnforcer::check_file_write` uses string-prefix workspace checks, so relative traversal like `../../outside` is allowed as if it were inside the workspace** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 14:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@aa9efe5`. Active tmux session at probe time: `omx-issue-2468-autoresearch-docs-css`; no active claw-code implementation session. Channel context included Jobdori's #591 `Prompt`-mode bypass in `permission_enforcer.rs`, so this probe inspected the adjacent file-write boundary helper. Code inspection: `rust/crates/runtime/src/permission_enforcer.rs::is_within_workspace` treats relative paths by string-concatenating `format!("{workspace_root}/{path}")`, then checks `normalized.starts_with(root) || normalized == workspace_root.trim_end_matches('/')`. It never normalizes `.`/`..`, canonicalizes symlinks, or resolves the candidate path. Therefore `check_file_write("../../outside/secret.txt", "/workspace")` builds `/workspace/../../outside/secret.txt`, which still starts with `/workspace/`, and returns `Allowed` in `WorkspaceWrite` mode even though the resolved target is outside the workspace. Existing tests cover absolute outside paths and simple relative `src/main.rs`, but not relative traversal escapes. **Required fix shape:** (a) replace string-prefix checks with path normalization/canonicalization against a workspace root, resolving `.`/`..` before comparison; (b) reject escaping relative paths even if the file does not exist yet, using lexical normalization plus parent canonicalization where needed; (c) add regressions for `../outside.txt`, `../../outside/secret.txt`, `src/../safe.txt`, and symlink escape cases; (d) share the same boundary primitive with runtime file ops and bash path-scope checks so permission enforcement cannot drift; (e) include resolved/candidate path evidence in denial messages without leaking sensitive contents. **Why this matters:** workspace-write mode's core promise is “project files only.” A string-prefix check lets a caller smuggle outside writes through relative traversal while the permission enforcer reports success, defeating the boundary before lower-level file tools can help. Source: gaebal-gajae dogfood response to Clawhip message `1507382558340681779` on 2026-05-22.
584. **`WorkerRegistry::restart` clears prompt/trust state but leaves old events and the original `created_at`, so post-restart timeout evidence can mix prior-attempt blockers with the new boot attempt** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 14:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@ac09033`. Active tmux sessions at probe time: none. Channel context included Jobdori's worker-boot #592 finding, so this probe stayed in `worker_boot.rs` but checked restart lifecycle continuity. Code inspection: `rust/crates/runtime/src/worker_boot.rs::restart` resets `status`, trust flags, prompt fields, `last_error`, attempts, and `prompt_in_flight`, then appends `WorkerEventKind::Restarted`. It does not reset `worker.created_at`, does not create a per-attempt `boot_started_at`, does not increment a restart/attempt counter, and does not clear or partition previous events. Later `observe_startup_timeout` computes `elapsed = now.saturating_sub(worker.created_at)` and derives `trust_prompt_detected`, `tool_permission_detected`, and `ready_for_prompt_detected` by scanning the entire `worker.events` history. A worker that previously hit trust/tool/ready evidence, then restarts cleanly, can have the new timeout classified with old pre-restart evidence and an elapsed time anchored to the original creation. Existing `restart_and_terminate_reset_or_finish_worker` only asserts prompt fields/attempts reset; it does not assert event scoping or elapsed-time reset. **Required fix shape:** (a) add `current_attempt_started_at`/`boot_started_at` and `attempt_index` fields; (b) set them on create and restart and use them for startup timeout elapsed/command_started_at; (c) scope timeout evidence scans to events since the current attempt or store per-attempt cached evidence; (d) add tests where trust/tool evidence exists before restart but not after, proving the post-restart timeout does not inherit stale blockers; (e) include attempt index in worker events so dashboards can separate old and new boot attempts. **Why this matters:** restart is supposed to produce a fresh boot attempt. If old events and timestamps remain authoritative, operators see misleading “stalled for hours / trust required” evidence for a brand-new restart and recovery automation can choose the wrong next action. Source: gaebal-gajae dogfood response to Clawhip message `1507390103956226228` on 2026-05-22.