Skip to content

tui: make work_update the canonical progress tool (#4132) - #4251

Merged
Hmbown merged 6 commits into
mainfrom
codex/v0868-fix-4132
Jul 8, 2026
Merged

tui: make work_update the canonical progress tool (#4132)#4251
Hmbown merged 6 commits into
mainfrom
codex/v0868-fix-4132

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • Introduce work_update as the sole model-facing To-do / Work progress tool (v0.9.0 2.15: make one canonical model-facing work progress tool #4132).
  • Keep checklist_* and todo_* registered as hidden compat aliases so saved transcripts and legacy payloads still replay.
  • Nudge mode/approval/subagent prompts and update_plan copy so Strategy stays metadata/context/route, not a second checklist.

Test plan

  • cargo test -p codewhale-tui --bin codewhale-tui --locked filters: work_update, todo_aliases_stay, compat aliases, catalog sole-surface, missing-tool redirects, card render
  • Replaced deferred checklist_update preflight expectation with catalog-visibility assertion (aliases are hidden)
  • cargo fmt --all --check
  • clippy -Dwarnings on codewhale-tui
  • ./scripts/release/check-versions.sh (root CHANGELOG synced)

Fixes #4132
Refs #4092

Ordinary To-do progress now flows through one model-facing surface; checklist_* and todo_* stay as hidden replay aliases, and update_plan remains Strategy-only.
Copilot AI review requested due to automatic review settings July 8, 2026 19:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Reviewing PR #4251

  • Gather context (diff, changed files, CLAUDE.md/AGENTS.md guidance)
  • Review correctness of work_update canonicalization + compat aliases
  • Check provider/model/route safety implications (none expected, but verifying)
  • Check reuse/simplification concerns
  • Check test coverage for new/changed behavior
  • Check security concerns
  • Post final review

View job run

Hmbown added 2 commits July 8, 2026 12:38
Replace the deferred checklist_update preflight expectation with a catalog
visibility assertion now that only work_update is model-visible.

Signed-off-by: Hunter B <hmbown@gmail.com>
Version-drift CI requires the TUI changelog slice to match root Unreleased.

Signed-off-by: Hunter B <hmbown@gmail.com>
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 3m 19s —— View job


Reviewing PR #4251

  • Gather context (diff, changed files, CLAUDE.md/AGENTS.md guidance)
  • Review correctness of work_update canonicalization + compat aliases
  • Check provider/model/route safety implications (none expected, but verifying)
  • Check reuse/simplification concerns
  • Check test coverage for new/changed behavior
  • Check security concerns
  • Post final review

Summary

This is a clean, well-scoped rename/canonicalization: work_update becomes the sole model-visible progress tool, checklist_*/todo_* stay registered-but-hidden (model_visible() == false) for transcript replay, and every call site that gates on tool name (registry.rs, tool_catalog.rs, approval policy, history/tool-card rendering, prompts) was updated consistently. I traced all 9 registered spellings (work_update, checklist_write/add/update/list, todo_write/add/update/list) end-to-end — names are unique, to_api_tools() correctly filters on model_visible(), and the new tests in todo.rs, registry.rs, and engine/tests.rs assert both "callable for replay" and "hidden from catalog" for every alias. No provider/model/route logic is touched — not applicable here.

Good:

  • work_progress_metadata now reports both canonical_tool (work_update) and invoked_as (actual name called), which is a nice improvement over the old checklist_metadata that only recorded the invoked name — this makes alias usage visible in transcripts without extra plumbing.
  • missing_tool_error_message's checklist-mis-call redirect and likely_field_corrections were both updated to point at work_update (tool_catalog.rs:669, 935), so stray checklist_update/todo_update calls with todos payloads still get corrected to the right entrypoint.
  • DEFAULT_ACTIVE_NATIVE_TOOLS swap (checklist_writework_update) is consistent with the model_visible() change, and the plan-mode / write-tool exclusion lists in engine/tests.rs were updated to include work_update alongside the legacy names.

Finding — doc inconsistency (non-blocking): docs/TOOL_LIFECYCLE.md is self-contradictory after this PR. The new "Canonical work-tracking surface" section (lines 50–66) correctly states work_update is now the sole canonical tool, but two other tables in the same file weren't updated to match:

  • Section 4's "authoritative manifest" (docs/TOOL_LIFECYCLE.md:198-201) still lists todo_write/add/update/listchecklist_write/add/update/list as the canonical replacement, with no row for work_update or for checklist_* itself now being a hidden alias.
  • Section 6's "canonical-surface rule" table (docs/TOOL_LIFECYCLE.md:269) still shows checklist_write as the kept-visible canonical for the "Checklist / todo" cluster.

A reader hitting line 52 ("canonical is work_update") then line 198 or 269 ("canonical is checklist_write") a few dozen lines later gets conflicting guidance from the doc that's supposed to be authoritative. Since docs/TOOL_SURFACE.md's equivalent tables were updated in this PR (TOOL_SURFACE.md:145, 323), this looks like an oversight rather than an intentional split. Fix this →

Minor: docs/TOOL_LIFECYCLE.md:50 labels the new section "for v0.8.68", but the workspace Cargo.toml is still at 0.8.67 (unreleased) — harmless, just double-check the version label matches whatever lane this actually ships in.

No issues found in:

  • Rust correctness: no unwrap/expect on new fallible paths, no panics, exhaustive match arms in TodoWriteTool::description().
  • Security: no secret handling, shell/exec policy, or input validation touched.
  • Test coverage: new tests (work_update_returns_canonical_task_update_metadata, checklist_write_compat_alias_still_replays, todo_write_compat_alias_still_replays, model_catalog_exposes_work_update_as_sole_progress_surface) cover both the canonical path and every hidden alias, including the "no deferred-preflight path for hidden aliases" case.

Note: I could not run cargo test/cargo fmt/clippy myself in this sandbox (network/build commands required approval that wasn't available), so I verified correctness by reading the diff and cross-referencing call sites rather than executing the test suite. The PR description states these were run locally and passed.

Keep nested prompt assets LF-normalized and measure prompt budget against LF text so Windows autocrlf checkouts do not trip the mode-delta guard. Trim the Agent mode delta for extra budget margin and finish the TOOL_LIFECYCLE work_update manifest update.

Signed-off-by: Hunter B <hmbown@gmail.com>
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Reviewing PR #4251 (update after new commits)

  • Gather context (new commits since last review, diff vs origin/main)
  • Check whether previously flagged docs/TOOL_LIFECYCLE.md inconsistency was addressed
  • Re-review any new changes for correctness/safety/tests
  • Post final review

View job run

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Reviewing PR #4251 (update after new commits)

  • Gather context (new commits since last review, diff vs origin/main)
  • Check whether previously flagged docs/TOOL_LIFECYCLE.md inconsistency was addressed
  • Re-review any new changes for correctness/safety/tests
  • Post final review

View job run

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@Hmbown
Hmbown force-pushed the codex/v0868-fix-4132 branch from 0279d6e to ce32242 Compare July 8, 2026 20:00
Keep the Windows LF budget fix, but put back the load-bearing self-report and sentinel wording that prompt contract tests assert.

Signed-off-by: Hunter B <hmbown@gmail.com>
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@Hmbown
Hmbown force-pushed the codex/v0868-fix-4132 branch from ce32242 to 8a6d0b1 Compare July 8, 2026 20:01
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 1m 59s —— View job


Reviewing PR #4251 (update after new commits)

  • Gather context (new commits since last review, diff vs origin/main)
  • Check whether previously flagged docs/TOOL_LIFECYCLE.md inconsistency was addressed
  • Re-review any new changes for correctness/safety/tests
  • Post final review

Summary

The four new commits (f3dc676, 56eb94e, 074bd52, 8a6d0b1) address the previously flagged issue and fix a real CI-stability bug found along the way. No new correctness, provider/route-safety, or security concerns.

Previously flagged docs/TOOL_LIFECYCLE.md inconsistency — fixed. 074bd52 updates both remaining stale tables:

  • Section 4's manifest (docs/TOOL_LIFECYCLE.md:200-207) now lists checklist_write/add/update/list and todo_write/add/update/list as hidden-compatibility aliases of work_update (previously showed todo_*checklist_* as deprecated).
  • Section 6's canonical-surface table (docs/TOOL_LIFECYCLE.md:275) now shows work_update as the canonical entry for the "Work progress / checklist / todo" cluster.

Both now agree with the "Canonical work-tracking surface" section and with docs/TOOL_SURFACE.md. Good catch-and-fix.

f3dc676 — test correctness: deferred_tool_preflight_guides_checklist_update_list_replacement was replaced with model_catalog_exposes_work_update_as_sole_progress_surface, which now loops over all 8 hidden aliases (checklist_*, todo_*) and asserts each: (1) stays registry-callable (registry.contains(hidden)), (2) is absent from the model catalog, and (3) has no deferred-catalog preflight path. This is a stronger, correctly-scoped replacement for the old single-alias test — matches the "hidden, not deferred" semantics established in b7d124b.

074bd52 + 8a6d0b1 — real Windows CI bug, correctly fixed:

  • .gitattributes previously only normalized crates/tui/src/prompts/*.md (top-level), missing nested modes/*.md, approvals/*.md, and .txt prompt assets — on a Windows autocrlf checkout those files would carry CRLF, inflating char_count / 3 token estimates and tripping the mode-delta budget guard. 074bd52 widens the glob to prompts/**/*.md and adds *.txt, and prompts.rs's mode_prompts_remain_small_deltas_not_base_policy_copies test now normalizes \r\n/\r before measuring word/token counts as defense in depth. Sound fix.
  • 074bd52 initially trimmed agent.md wording for budget margin, which silently broke sentinel-phrase assertions elsewhere in prompts.rs (e.g. AGENT_MODE.contains("not user input"), "Do not tell the user they pasted sentinels", "I'll run tests" — see prompts.rs:3265-3269). 8a6d0b1 correctly restores that load-bearing wording while keeping the .gitattributes/normalization fix. Good self-correction.

Note (non-blocking): with 8a6d0b1's wording restored, agent.md measures ~689 words / ~4739 chars ⇒ ~1580 estimated tokens against the test's 1600-token ceiling — about a 20-token margin. Not a bug (the test will catch any regression), but it's thin enough that the next small addition to agent.md will likely need a corresponding trim elsewhere. Worth knowing if you add to this file again soon.

No issues found: changelog sync (56eb94e) mirrors the identical entry into both root and crates/tui/CHANGELOG.md, matching the "Version-drift CI requires the TUI changelog slice to match root Unreleased" constraint. docs/TOOL_SURFACE.md is unchanged since the last review round and remains consistent.

As before, I could not execute cargo test/cargo fmt/clippy in this sandbox (build commands require approval unavailable here); this review is based on reading the diff, cross-referencing call sites, and hand-computing the token-budget math against estimate_text_tokens_conservative's chars/3 formula.

Signed-off-by: Hunter B <hmbown@gmail.com>
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 57s —— View job


Reviewing PR #4251 (update after new commits)

  • Gather context (new commits since last review, diff vs origin/main)
  • Check whether previously flagged docs/TOOL_LIFECYCLE.md inconsistency was addressed
  • Re-review any new changes for correctness/safety/tests
  • Post final review

Summary

The only new commit since the last review round is 1957a55 ("merge main into #4132 after #4250"), which merges origin/main (through PR #4139's catalog-cache work, fd11512) into this branch. It is a clean merge with no substantive changes to the work_update canonicalization itself.

Merge conflict resolution — clean. The merge only produced diffs in CHANGELOG.md and crates/tui/CHANGELOG.md (both files' ## [Unreleased] sections had competing insertions from #4139 and this PR). Checked both:

  • The work_update changelog entry (v0.9.0 2.15: make one canonical model-facing work progress tool #4132) and the OFFERING_SEEDS/live-catalog entry (v0.9.0 3.1: wire live catalog cache into provider and model pickers #4139) are both present, correctly ordered, and neither was duplicated or dropped.
  • No leftover conflict markers (<<<<<<</=======/>>>>>>>) anywhere in the tree.
  • The #4139 files it pulled in (crates/config/src/catalog.rs, route/offering.rs, route/resolver.rs, crates/tui/src/client.rs, models_dev_live.rs, model_picker.rs, provider_picker.rs) don't intersect with any file this PR touches (tools/todo.rs, tools/registry.rs, core/engine/tool_catalog.rs, prompts, etc.) — no cross-feature interaction to worry about, and no re-opened work_update/checklist/todo surface.

Previously flagged docs/TOOL_LIFECYCLE.md inconsistency remains fixed (from 074bd52) and is untouched by the merge — Section 4's manifest and Section 6's canonical-surface table still correctly show work_update as canonical with checklist_*/todo_* as hidden aliases, consistent with docs/TOOL_SURFACE.md.

No new correctness, provider/route-safety, reuse, test-coverage, or security issues introduced by this update. As before, I could not execute cargo test/cargo fmt/clippy in this sandbox (build commands require approval unavailable here); this pass is based on diffing the merge commit against its first parent and re-checking the previously-flagged doc issue.

@Hmbown
Hmbown merged commit aaaf5a8 into main Jul 8, 2026
18 checks passed
@Hmbown
Hmbown deleted the codex/v0868-fix-4132 branch July 24, 2026 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v0.9.0 2.15: make one canonical model-facing work progress tool

2 participants