Skip to content

fix(tui): restore the build after local Ollama discovery landed - #7129

Merged
Hmbown merged 2 commits into
codewhale-hq:mainfrom
gaord:fix/tui-build-after-ollama-discovery
Oct 29, 2026
Merged

Hmbown merged 2 commits into
codewhale-hq:mainfrom
gaord:fix/tui-build-after-ollama-discovery

Conversation

@gaord

@gaord gaord commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

What and why

main does not compile right now, so every PR built on it is red before review. This fixes the three errors that block it.

The compile error. fetch_tag_profiles hit error: implementation of `FnOnce` is not general enough through three call sites (client.rs::fetch_catalog_delta, client.rs route sweep, local_ollama.rs::spawn_local_ollama_adoption_probe). Since 7c49574 the per-tag async block borrows &String out of the caller's tags slice, so the stream's item type carries a caller lifetime and tokio::spawn cannot accept it. The function now owns an String per candidate; bounded concurrency (SHOW_PROBE_LIMIT) and result ordering are unchanged.

Worth knowing for anyone who hits this: the compiler's help: consider adding an explicit type annotation to the closure's argument (|tag: &String|) does not fix it — I checked that suggestion in isolation and it still fails.

The two lints. Denied by the workspace -D warnings:

  • session_manager.rs: save_checkpoint is only called from tests since the persistence path moved to save_checkpoint_owned (3016ef4). Gated #[cfg(test)] rather than suppressed, so it adds no dead-code-suppression budget.
  • session_manager.rs: a redundant return.
  • provider_routes.rs: Some(apply_route()) called the closure inside Some; now called as a statement.

No behavior changes intended.

Issue

No-Issue: main build breakage from direct-to-main landings; there is no tracking issue and CI does not run on pushes to main.

How I tested it

macOS, locked workspace, scripts/dev-cargo.sh and scripts/dev-test.sh:

  • cargo fmt --all -- --check — clean.
  • clippy -p codewhale-tui --all-targets --all-features --locked -- -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or — clean.
  • build -p codewhale-cli --locked — succeeds; ./target/debug/codewhale --version prints codewhale 0.10.2 (dev).
  • scripts/dev-test.sh crates/tui/src/local_ollama.rs — 9 passed.
  • scripts/dev-test.sh crates/tui/src/tui/ui/ — 4027 passed (provider_routes path).
  • scripts/dev-test.sh crates/tui/src/client/ — 621 passed on the earlier run.
  • scripts/dev-test.sh crates/tui/src/session_manager.rs — 135 passed, 1 failed (see below).

Two failures are pre-existing on unmodified main, and I left them alone to keep this one focused. Each was reproduced with my patch stashed, on 687a38c6e:

  1. session_manager::tests::interrupted_session_deletion_keeps_recovery_incomplete_and_can_finish — deterministic. It writes a legacy checkpoint, asserts load_legacy_checkpoint() is None (the origin is retired), then expects delete_session to finish the interrupted cleanup. The explicit-delete guard added in fed2140 (this session has a shared legacy checkpoint; remove that checkpoint separately) refuses instead. Test and guard disagree about that case.
  2. Socket-bound tests in the client suite (mcp::http_client::tests::model_added_endpoint_cannot_use_ambient_proxy_but_operator_can, client::tests::create_message_scenario) — intermittent. Baseline runs on clean main failed 1 in 3 with the same "Connection failed" / unexpected listener.accept() assertions. Each passes when run alone.

So Test (ubuntu-latest) may still be red on this PR for reasons unrelated to it. Happy to take either failure in a follow-up if you want them.

Not verified: Windows and the OpenHarmony build; cargo-nextest is not installed here, so I used the libtest harness scripts/dev-test.sh selects when nextest is absent.

Checklist

  • One focused change, started from current main (no need to rebase when main moves)
  • cargo fmt --all passes, and tests cover new or changed behavior
  • If this adds or changes a user-facing feature, I updated its docs and its row in docs/features.toml (cargo test -p codewhale-tui --test feature_registry checks it)

`cargo build` and the Lint gate have been failing on main since the local
Ollama discovery and interrupted-session landings, so every pull request
built on current main is red before it is reviewed. This makes main
compile and lint again.

The compile error was a lifetime mismatch in `fetch_tag_profiles`: the
per-tag async block borrowed `&String` from the caller's slice, so the
stream's item type carried a caller lifetime that `tokio::spawn` could
not satisfy. It now owns a `String`, which keeps the bounded-concurrency
probing and its ordering unchanged. Annotating the closure argument, as
the compiler suggests, does not fix it.

The two remaining errors were lints denied by the workspace
`-D warnings`: a redundant `return` and a unit value passed to `Some`.
Both keep their existing behavior.

Verified on macOS with the locked workspace: `cargo fmt --check`,
`clippy --all-targets --all-features` with the CI allowances, a full
`codewhale-cli` build, and the client, local_ollama and session_manager
suites. Two failures that reproduce on unmodified main are left alone:
session_manager's interrupted-deletion test, and intermittently failing
socket-bound tests in the client suite.

Signed-off-by: Ben Gao <bengao168@msn.com>
@gaord
gaord requested a review from Hmbown as a code owner October 28, 2026 13:27

@claude claude Bot 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

Keep the contributor build fix while following the workspace policy for new code comments.
@Hmbown
Hmbown merged commit c08b7f9 into codewhale-hq:main Oct 29, 2026
22 of 24 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