Skip to content

fix(release): integrate v0.8.68 TUI and Android QA#4337

Merged
Hmbown merged 3 commits into
mainfrom
codex/v0868-final-integration
Jul 10, 2026
Merged

fix(release): integrate v0.8.68 TUI and Android QA#4337
Hmbown merged 3 commits into
mainfrom
codex/v0868-final-integration

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Summary

  • land the final v0.8.68 cancelled-shell transcript state and PTY regression coverage
  • authenticate the Android loaded image before any updater replacement on Termux

Evidence provenance

This is the focused integration delta from validated local Android/TUI QA at
fb2e5587281e098ca8eb9fcee5d1d3d2f997cb5b, replayed onto the current merged
main rather than pushing the scratch train.

  • Android CLI evidence: /Volumes/VIXinSSD/android/qa-20260710-api35-fb2e55872/cli-evidence/cli-qa.log
  • Updater adversarial evidence: /Volumes/VIXinSSD/android/qa-20260710-api35-fb2e55872/updater-mirror/updater-mirror-adversarial.log
  • TUI PTY evidence: /Volumes/VIXinSSD/android/qa-20260710-api35-fb2e55872/tui-pty/tui-pty/driver.log

The updater keeps the final dladdr marker plus matching /proc/self/maps
canonical-path/device/inode proof and rechecks identity before destructive
renames. It does not rely on argv0, PATH, AT_EXECFN, or /proc/self/exe
as Android update authority.

The known raw fast-exit tmux receipt race remains documented in the QA evidence;
the one-second-drain rerun is clean. This PR does not claim that raw harness race
is fixed.

Verification

  • ./scripts/release/check-versions.sh
  • cargo fmt --all -- --check
  • git diff --check
  • cargo test -p codewhale-cli --all-targets (145 + 1 passed)
  • cargo test -p codewhale-tui --test qa_pty (9 passed)
  • cargo clippy -p codewhale-cli -p codewhale-tui --all-targets -- -D warnings
  • cargo build --release --locked -p codewhale-cli -p codewhale-tui

Boundaries

No tag, GitHub Release, deployment, registry publish, global installation, or
issue closure is part of this PR.

Hmbown added 2 commits July 10, 2026 12:12
Preserve the validated cancelled shell transcript behavior and PTY regression coverage after the workflow integration.
Use the validated loaded-image identity proof before any destructive updater replacement on Termux.
Copilot AI review requested due to automatic review settings July 10, 2026 19:24

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 10, 2026

Copy link
Copy Markdown

Claude finished @Hmbown's task in 2m 46s —— View job


Review complete

  • Gather context (diff, changed files, CLAUDE.md/AGENTS.md guidance)
  • Review crates/cli/src/update.rs Android image authentication changes
  • Review crates/tui/src/core/engine.rs + tests (cancelled transcript state)
  • Review crates/tui/src/tui/live_transcript.rs and widgets/mod.rs
  • Review crates/tui/tests/qa_pty.rs
  • Check provider/model prefix safety invariant (EPIC v0.8.65 EPIC: Separate provider facts, model facts, offerings, and route resolution #2608)
  • Post final review summary

Summary

This is a well-scoped, well-tested integration of two independent fixes. No correctness bugs found.

1. Android update-target authentication (crates/cli/src/update.rs)

  • The core fix is sound: instead of trusting std::env::current_exe()/AT_EXECFN//proc/self/exe (which can resolve to Bionic's runtime linker on Android), it derives a dladdr marker compiled into the binary, cross-checks it against the matching /proc/self/maps row (executable, non-zero inode, absolute non-deleted path), rejects known linker filenames (linker, linker64, linker_asan[64], linker_hwasan[64]), and re-verifies device/inode identity immediately before each destructive rename (replace_verified_downloadsreplace_binary_with_validation).
  • The preflight-then-just-in-time recheck pattern correctly prevents a swap from splitting a paired primary/sibling install — confirmed by android_identity_preflight_prevents_all_paired_replacements and android_identity_recheck_before_sibling_prevents_pair_split, which assert neither file is mutated when identity changes mid-flight.
  • replace_binary is now correctly gated #[cfg(test)] since its only remaining callers are unit tests; production code goes through replace_binary_with_validation.
  • Unsafe dladdr/CStr usage has accurate SAFETY comments and validates all invariants it relies on (non-null dli_fname, non-empty bytes) before dereferencing.
  • Test coverage is thorough: marker-outside-mapping, non-executable mapping, anonymous mapping, relative/deleted paths, linker rejection (direct and via symlink), authority disagreement, non-executable target file, device/inode mismatch, and the pre-replace swap race. Good use of Cell<usize> to assert the exact number of validation calls in the race tests.
  • Non-Android platforms are unaffected — update_executable_identity()/validate_primary_update_identity() are no-ops there aside from the existing current_exe() call.

2. TUI cancelled-shell transcript settling (engine.rs, widgets/mod.rs, live_transcript.rs, qa_pty.rs)

  • user_shell_turn_outcome correctly distinguishes Interrupted (explicit cancel or tool-reported canceled metadata) from Failed (non-success result) from Completed, replacing the old binary is_err() check that couldn't represent cancellation. TurnOutcomeStatus::Interrupted is a pre-existing, already-consumed variant (turn_loop.rs, ui.rs, runtime_threads.rs), so this isn't introducing new unhandled states.
  • The cache-key aliasing fix in widgets/mod.rs (ACTIVE_REVISION_DOMAIN bit-63 tag via revision_in_domain/history_entry_revision/active_entry_revision) directly targets a real bug class: a committed history revision and an active-cell revision could alias on the same positional cache slot, causing a stale "running" render to survive after cancellation. The regression tests construct the exact 128-entry rotate-by-7 alias and a forced raw-revision collision (app.next_history_revision = ACTIVE_REVISION_DOMAIN | 1) to prove the tag prevents reuse — good adversarial test design, not just happy-path coverage.
  • qa_pty.rs's new cancelled_bang_shell_settles_transcript_card test exercises the full PTY path end-to-end (spawn, cancel via \x03, assert run running disappears and no false turn completed), matching the engine/widget unit-level fixes with an integration-level one.
  • The PR body explicitly notes the known raw fast-exit tmux receipt race is not claimed fixed here — consistent with what's in the diff.

Reuse/simplification: No dead code or needless duplication spotted; the new helper functions (revision_in_domain, user_shell_turn_outcome) factor out logic that was previously inline/duplicated, which is a net simplification.

Provider/model safety (EPIC #2608): Not applicable — this diff touches update-target authentication and TUI transcript rendering only; no provider/model prefix inference logic present.

Note: I wasn't able to run cargo check/cargo test/cargo clippy locally in this sandbox (command execution requires approval that isn't available in this context), so I relied on careful manual reading of the diff plus the PR's stated local verification. CI (CI workflow, run 29117897563) was still in progress at review time — worth confirming it goes green before merge, though nothing in the diff looks like it would fail to compile or lint.

The helper is only called by #[cfg(unix)] Android loaded-image tests.
Compiling it on Windows produced a dead_code warning under -D warnings,
which failed the windows-latest CI job. Gate the whole function so it is
not built on non-Unix targets.
@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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


Review of latest push (commit d0f9fe6)

  • Gather context (new commit since last review, diff, CI)
  • Review the new write_test_executable Unix-gating fix
  • Confirm nothing else changed since the prior full review
  • Check CI status
  • Post final summary

Summary

This push adds one small follow-up commit on top of the two already reviewed (2c62042 transcript settling, a448efe Android image authentication): d0f9fe6 — gate write_test_executable on Unix.

What changed: crates/cli/src/update.rs:1437-1443 — the test helper write_test_executable (used only by #[cfg(unix)] Android loaded-image tests) is now itself #[cfg(unix)], instead of being compiled on every platform with an inner #[cfg(unix)] block around the chmod. This fixes a dead_code warning that was failing the windows-latest job under -D warnings.

Correctness check: I verified every call site of write_test_executable (lines 1497, 1521-1522, 1548, 1565, 1585, 1603, 1640, 1695-1696, 1731, 1759/1762, 1782/1785/1787, 1829/1832/1834, 1882/1885) sits inside a test function already annotated #[cfg(unix)]. So gating the helper itself introduces no dead references on any platform — the fix is minimal and correct, and matches the stated reason (Windows CI dead-code failure) exactly.

No other files changed in this push; my prior full review of update.rs (Android authentication), engine.rs/tests.rs, live_transcript.rs, widgets/mod.rs, and qa_pty.rs still stands — no new correctness, provider/model-safety, reuse, test-coverage, or security concerns.

CI: was still in progress at review time (CI, PR #4337, and this Claude PR Review run all running; DCO passed). Worth confirming the CI workflow — including the previously-failing windows-latest job — goes green before merge.

@Hmbown Hmbown merged commit 3e97b27 into main Jul 10, 2026
19 checks passed
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.

2 participants