Repository navigation
lorenzo/start kw integration pr 9 - #207
Draft
lorenzoberts wants to merge 61 commits into
Draft
lorenzoberts wants to merge 61 commits into
lorenzoberts wants to merge 61 commits into
Conversation
This commit moves src/kw/actor.rs to src/kw/actor/mod.rs unchanged, so its tests can live in a sibling file. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
This commit moves the actor's test module out of mod.rs into sibling files. The tests themselves are unchanged; rustfmt rejoins a few lines that fit once the module indent is gone. They are split into actor_test.rs and deploy_test.rs to keep each file under 2,000 lines. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
InputContext and CurrentScreen now derive Default, and tests build them with struct literals. KeyInput cannot derive Default because KeyCode and KeyEventKind do not implement it, so its Default is a null press with no modifiers. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
TerminalHandle::read_event and TerminalMessage::ReadEvent existed only to block on the session from tests. Event delivery already goes through TerminalHandle::poll_event and the input actor. Removed read_event_returns_terminal_event_from_actor: it only asserted that the deleted message returned the event stubbed on the session mock. The input actor tests already assert that a session key is delivered. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
ConfigState::page_size only reread the crate-visible field, and PatchFeedIndex::get_patch only forwarded the private map. Tests read those directly. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
apply_record, build_record, and latest_build_record had no production callers. History tests now read through apply_record_for_branch and build_records, which still check the stored records, overwrite, coexistence, missing files, corrupt files, and newest-across-branches. KwReadiness.kernel_image, build_record, and latest_build were written and never read outside tests. evaluate_readiness still feeds them into deploy_alone, and the tests assert that verdict. On unix the allow on KwHandle hid nothing; on non-unix `KwHandle::new` is unused until the next commit gates it. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
CachePolicy had no implementors. PatchFeedIndex stored target_list and never read it, so new() no longer takes that string. The empty-state test still checks the offset and representative ids; the list-name assertion only read the removed getter. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
log_scan is compiled only on Unix, with the actor. Functions whose only production caller is that actor are cfg(unix) too, including build history, the image and deploy probes, and remote.config parsing. FileSystemTrait::read_dir and KwHandle::new follow the same gate. The UI still names several types on every platform, so those stay, with #[cfg_attr(not(unix), expect(dead_code))] on the item the actor alone fills in: - KwError and KwStartError: variants only the actor constructs - DeployOptions, StartRequest, and KwMessage: fields only the actor reads - KwJobKind, KwPhase, and KwJobStatus: variants only the actor constructs - TreeReadiness, DeployAloneRefusal, and BootOnceState - RemoteRefusal No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
…llows Config callers now import the re-exports from crate::config, so the module-wide unused_imports allow is unnecessary. resolve_config_path stays on the repository module; nothing else named the re-export. #[expect(clippy::too_many_arguments)] is on five functions whose arguments are independent inputs. Folding them into a struct would only exist to be unpacked at the call: - App::new - LoreService::new - evaluate_readiness - prepare_deploy_blocking - app_with_details_and_kw No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
The readiness probes stay associated functions on a unit struct, with the same arguments and the same per-item unix gates. Private helpers whose names did not start with a verb are renamed to find_newest_image_in, read_image_mtime, read_boot_once_from_file, and resolve_xdg_kw_config_file. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
KwArgvService, RemoteConfigService, and LogScanService hold the former free functions as associated functions. Helpers that did not start with a verb are now find_reserved_option, find_reserved_long_option, find_unique_long_abbrev, find_reserved_short_cluster, find_exact_short, is_value_attached, read_remote_from_file, resolve_xdg_remote_config, find_first_error, and check_grub_missed_release. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
KwGitService holds the worktree probe, branch switch, and HEAD probe. Job running, cancel, and outcome mapping are private KwActor functions, along with prepare_deploy_blocking, which was a free function beside them. head_branch, outcome_after_building_cancel, and outcome_after_cancel are probe_head_branch, rewrite_outcome_after_building_cancel, and map_outcome_after_cancel so each name starts with a verb. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
ApplyPatchsetService and ReviewedReplyService hold the action functions. Apply keeps its own switch_to_branch: it takes a kernel tree, omits git's -- separator, and returns a formatted String, unlike KwGitService::switch_to_branch. Request builders that fed these actions are private App functions, renamed so each name starts with a verb. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Screen handlers now run on App. Each flow's help popup is an associated build_*_help_popup. Helpers that did not start with a verb are map_tail_read, is_snapshot_job_terminal, is_job_running, is_job_busy, and count_preview_scroll_lines. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
The input dispatch, kw-status wait, and confirm-popup helpers are private AppActor functions. kw_status_changed, kw_ops_should_tail, on_input, and kw_job_is_running are await_kw_status_change, should_tail_kw_ops, dispatch_input, and is_kw_job_running so each name starts with a verb. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
ConfigService, ConfigParsingService, EnvOverrideService, and ConfigPathService hold the former free functions. normalize_derived_paths is a ConfigState method. unknown_target_kernel_tree is reject_unknown_kernel_tree so the name starts with a verb. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
…RendererService The patch and cover preview helpers are associated functions on those services. bat, delta, and diff-so-fancy entry points are render_patch_with_bat, render_patch_with_delta, render_patch_with_diff_so_fancy, and render_cover_with_bat so each name starts with a verb. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Each screen module paints through a Painter. The frame layout is FramePainter and the loading overlay is LoadingPainter. Helpers that did not start with a verb are build_mode_spans, build_keys_hint_span, build_review_trailers_line, compute_log_scroll_offset, build_labeled_line, build_colored_label, build_field_line, center_rect, and advance_spinner. No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
ActorReplyService::send_actor_reply replaces the result-reply copies. They shared the same control flow and differed only in the static failure and dropped-receiver log lines, which each call site still passes through. Value replies that never log a failure use deliver_value. The reply log events' tracing target, file and line now point at `infrastructure::actor_reply` instead of each actor's module, while message text, level and fields are unchanged (the `patch_hub` prefix filter still matches). No tests removed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
…ices Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
The type's methods split a patch into cover and diff, build a reply template, and pull a git send-email command out of the patch page. Reply would describe only the last two; the renderer also uses the cover/diff split, which is not a reply. PatchsetTextService names the shared job of turning patchset text into those three results. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
The eyre hook runs when a Report is constructed, so a failed restore printed "failed to restore terminal" on every eyre!, bail!, and foreign-error conversion, not only on a fatal exit. The panic hook and the eyre hook now share one process-wide guard: restore is attempted once, and a failure is reported once. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
eyre calls the installed hook from capture_handler on every Report construction (Report::from_std, from_adhoc, from_display, from_msg, and from_boxed), which is `?` into eyre::Result, eyre!, and bail!. That includes errors the app handles and keeps running after. A failed fetch does bail! in src/app/screens/latest.rs:60, and src/app/flows/open_patchset.rs:42 turns that Err into an error popup. Restoring the terminal there dropped the alternate screen and raw mode while the TUI was still running. The eyre hook now only forwards to color_eyre (src/infrastructure/errors.rs:73). The panic hook still restores first (src/infrastructure/errors.rs:66-69). main returns color_eyre::Result (src/main.rs:47). #[tokio::main] propagates that Result, and std::process::Termination prints it with Debug after main returns. The eyre hook does not print; it only captures color_eyre's handler when the Report is built. The report would therefore be printed into raw/alternate mode unless something restores first. Nothing on the fatal Err path did. AppActor::run (src/app/actor.rs:79) returns I/O errors with `?` and does not shut the terminal down. AppHandle::run_until_done (src/app/handle.rs:20-22) only joins that task. main calls terminal_handle.shutdown (src/main.rs:190-193) only after run_until_done returns Ok. That reaches CrosstermTerminalSession::shutdown (src/terminal/session.rs:118-125), which calls restore(). teardown_user_io (src/infrastructure/terminal.rs:59-63) turns raw mode back on for an editor; it is not process teardown. Errors before init (src/main.rs:58, src/main.rs:61-62, src/main.rs:73, src/main.rs:77) never enter raw mode, so they must not restore: `LeaveAlternateScreen` would emit a stray escape sequence. init (src/infrastructure/terminal.rs:28-41) rolls its own partial setup back: LeaveAlternateScreen if raw mode fails, restore() if Terminal::new fails. After init succeeds, bootstrap (src/main.rs:132-135), App::new (src/main.rs:137-150), subscribe_app (src/main.rs:153-156), run_until_done (src/main.rs:177-178), and input shutdown (src/main.rs:179-182) can return Err before terminal shutdown. main calls restore_once() (src/main.rs:200-202) before returning that Err, so the runtime prints the report on a normal terminal. TerminalRestoreGuard stays. A panic in a spawned task runs the panic hook and then comes back as a join error from main, so both would restore. The guard makes the second call a no-op. restore_guard_runs_the_operation_once still covers that. A test that the eyre hook does not restore is not feasible without a real terminal or a new injection seam: install_hooks sets a process-global hook, and restore() talks to crossterm. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Snapshots and edit-config drafts are projections of their source state, so callers build them with From instead of dedicated converter methods. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Screen projections are derived from application state, so callers use From instead of project helpers. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Cache keys are projections of their sources. Reviewed-reply indexes became `collect_successful_indexes`. The preview constructor keeps its arguments and takes a verb name. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
ConfigService::parse_non_empty replaces the eight blank-string match
guards in validate_update. None still skips the field. A blank Some
still takes that field's rejection. A non-blank Some still takes the
old unguarded arm.
page_size: blank is InvalidPageSize of the raw text; non-blank parses
the trimmed text as usize and reports InvalidPageSize of the raw text
on failure.
cache_dir: blank is InvalidDirectory("cache directory is empty");
non-blank still trims and validate_dir.
data_dir: blank is InvalidDirectory("data directory is empty");
non-blank still trims and validate_dir.
max_log_age: blank is InvalidMaxLogAge of the raw text; non-blank
parses the trimmed text as usize.
stay_on_applied_branch: blank is InvalidStayOnAppliedBranch of the raw
text; non-blank parses the trimmed text as bool.
kw_reboot_after_deploy: blank is InvalidKwRebootAfterDeploy of the raw
text; non-blank parses the trimmed text as bool.
kw_deploy_force: blank is InvalidKwDeployForce of the raw text;
non-blank parses the trimmed text as bool.
target_kernel_tree: blank is still Some(None); non-blank still accepts
a trimmed known key as Some(Some(key)) or InvalidTargetKernelTree.
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Each guard's false case still reaches the same result as the arm it used to fall through to. view_model succeeded-with-warnings: an empty warning list still yields (None, None), which was the wildcard arm. Failed is unchanged. Idle, Running, Cancelled, and a missing job stay (None, None). kw_job_running_popup: Ok with a non-running job still returns None, as does a status error. handle_job_event: an unsuccessful Exited outcome still becomes Failed. WaitFailed and Cancelled are unchanged. probe_head_branch: Ok with success still returns the trimmed branch; Ok without success still warns and returns an empty string. Err is unchanged. resolve_output_dir: an empty XDG_CACHE_HOME still uses $HOME/.cache, and so does a missing variable. A non-empty value is still used as-is, and HOME is still read only on the fallback. check_kw_version: a parsed version below the floor is still Below. None is still Unknown. A version at or above the floor is still Meets. resolve_xdg_kw_config_file and resolve_xdg_remote_config: an empty XDG_CONFIG_HOME still uses $HOME/.config via HOME.ok(), and so does a missing variable. terminal_event_from_crossterm_event: a non-release key still becomes KeyInput. Release is still dropped. The wildcard for other crossterm events stays. map_tail_read: NotFound is still an empty tail. Any other IoError still formats "(could not read log: ...)". map_details_go_to_first_line_chord: a pending chord inside the timeout still fires PreviewGoToFirstLine. An expired chord and no pending chord still arm a new chord and return None. KwJobStatus::running_indicator lists Idle, Succeeded, Failed, and Cancelled instead of a wildcard; only Running still produces text. is_snapshot_job_terminal is true only for Succeeded, Failed, and Cancelled. is_job_running and is_kw_job_running are true only for Running; should_tail_kw_ops uses is_kw_job_running. needs_boot_once_confirm is true for On and Unknown when not yet acknowledged, and false for Off. A deploy start still checks deploy_alone only for KwStartKind::Deploy. An unverified kw version warning still fires for Below and Unknown, not Meets. Boot-once refusal still fires for On and Unknown when the option is not acknowledged. Left in place: the select! precondition in AppActor, KeyCode::Enter while a confirm popup is open (a closed popup still falls through to the wildcard), and the argv pointer-equality guard (a different option still falls through to the ambiguous-prefix rejection). apply_kw_snapshot clears start_requested for every non-Idle status (Running, Succeeded, Failed, and Cancelled). Idle leaves the flag unchanged. KwOpsViewModel.running is true only while the job is Running. Idle, Succeeded, Failed, Cancelled, and a missing status stay false. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Bindings no longer spell the collection type before collect. Where the following use does not pin the type, the same type is written as a turbofish. Two sites infer it and drop the annotation. collect::<Vec<String>>: edit-config kernel-tree keys, config reject_unknown_kernel_tree keys, merged kw argv, the view-model reply index list, and the bufreader test lines. collect::<Vec<&str>>: lore tag parts, pre blocks, and patch preview lines. collect::<Vec<char>>: reserved short-cluster characters. collect::<Vec<PatchTagSummary>>: fetched patchset tag summaries. collect::<String>: the minor component of a kw version line. collect::<HashMap<PathBuf, String>>: the remote-config test filesystem. Inferred with no annotation: kw config keys inserted into HashMap<String, String>, and get_page's Vec<&Patch> returned as Option<Vec<&Patch>>. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Hoist crate paths and other multi-segment paths into use so call sites name the imported item, or keep a module prefix when that prefix carries the meaning. That covers fs, io, time, task, and process, and names that would mislead if imported bare: fmt::Result, io::Error, and ptr::eq. Doc links drop fully qualified crate paths for the same reason. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Drop notes that record an old shape of the code, lab log paths, and a leftover TODO. Sentences that still state a real constraint stay, with that wording removed: PATCH_HUB_* overrides still apply onto config state, and the remote parser still matches Hostname, Port, and User by key rather than by position after Host. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Trim every comment block to five lines or fewer, including module docs. Notes that capture behaviour the code cannot show stay: GNU getopt prefix rules, kw exit codes, the kw O= resolution order, and shutdown ordering. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Blank lines mark the phases of multi-step functions in the kw actor, KwOps flow, and patchset apply: probe then switch then spawn, record then status, and outcome checks then the command trace. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Panic sites in tests now name the call they were waiting on, so a failure points at that call instead of a bare unwrap. The messages stay short: "json parses", "build starts", "temp dir removes". Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Kw mock expectations now say how often they run and which arguments they accept. Counts come from a run of the kw tests. Shared fixtures use a measured range, because callers probe them a different number of times: the recording shell, the ready-tree filesystem, and the remote env helper. .withf(|_| true) appears 7 times, each with .times(0). The call never happens, so the argument is not observed: - record_build on deploy-alone history, in deploy_history and in the deploy test that must not write a build record. - execute when the kw binary is missing, and on the four evaluate_readiness paths that return before any shell command. history.rs tests use the real store and set no mock expectations. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
App, config, lore, input, terminal, and infrastructure mocks now state how often they run and which arguments they accept. Counts come from a test run. Shared fixtures use a measured range when callers read a different subset: config env keys, the apply shell, and the clean worktree filesystem. A generated patchset branch name is matched by its patchset- prefix so the count stays stable. .withf(|_| true) or .withf(|_, _| true) appears 11 times, each with .times(0). The call never happens, so the argument is not observed: execute when a dependency check must not spawn, the apply refusal that never touches the tree, and history lookups that must not run. .withf(|| true) appears 18 times on methods with no arguments (cache loads, terminal setup and shutdown, process wait and kill). There is no argument to check. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Test-only builders and doubles now live in a helpers submodule next to the tests that call them. None became Default: the built types do not implement Default, or they are parsed from JSON or XML, and this crate does not use cfg_attr(test, derive(Default)). No test was renamed. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
The filesystem, process, and kw readiness suites each owned a
TempDir(PathBuf) tuple. They now share TempDir { path } in
test_support, including the same Drop cleanup.
Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Deny unwrap_used and allow_attributes so new unwrap calls and #[allow] attributes fail clippy. Warn on redundant type annotations and single-variant match wildcards; CI denies warnings, so those are clean too. Test unwrap_err calls now use expect_err with a message for the expected failure. The patch-renderer match names Default, and four bindings drop type annotations rustc already infers. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
Paths that were hoisted onto their own use lines now sit in the std, external, or crate group already present in the file. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
KwActor::spawn always runs in this unix-only binary, so main keeps the handle itself. Shutdown is still after the input actor and before the config actor. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
The non-test helpers move into the helpers submodule next to the other kw_ops fixtures. Test paths drop the unix segment. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
The root lint tables move under [workspace.lints] and both packages inherit them. The proc-macro crate reports bad input with syn::Error and tests use expect. Signed-off-by: lorenzoberts <lorenzobs@usp.br>
lorenzoberts
added this pull request to stack #200
October 4, 2026 18:56
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack created with GitHub Stacks CLI • Give Feedback 💬