refactor: continue scaffold desloping#210
Open
wozos wants to merge 82 commits into
Open
Conversation
- setup.rs: fix `sha256_hex`. `sha2` is a plain (non-optional) dependency and there is no `[features]` table, so `#[cfg(feature = "sha2")]` was always false and the FNV-1a fallback compiled instead. The function returned a 16-char non-SHA digest, so prebuilt sequencer_service checksum verification always failed (and bailed) whenever a `.sha256` was published. Use `sha2` unconditionally, matching the project's existing hex-encode idiom. - config.rs: drop the dead `path` binding only consumed by `let _ = path;`. - run.rs: strip redundant "Step N:" comment prefixes that just duplicate the user-facing `[N/total_steps]` progress prints; keep all explanatory text. - wallet_support.rs: collapse stray whitespace in the legacy wallet-config warning message. https://claude.ai/code/session_01KutAyMX3auZUKhuHHp5xXq
- Remove `#[allow(dead_code)] // Fields wired up in later phases` on `BasecampAction`; all variants are constructed, so `cargo check` is clean without it. The attribute and its placeholder comment were leftover slop. - Collapse the two-arm `cwd_override` match into `as_deref().unwrap_or(project_root)`, matching the idiom the file's own tests already use. Behavior unchanged. https://claude.ai/code/session_01KutAyMX3auZUKhuHHp5xXq
Two eprintln! warnings (localnet sequencer r0vm hint, run vendored-spel hint) had runs of ~15-18 literal spaces where line breaks were intended, printing as garbled single-line text. Switch to the file's backslash line-continuation idiom so they render as clean single-spaced sentences. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the mechanical clippy warnings flagged across the codebase, all behavior-preserving: - config.rs: build OldSchemaMarkers as a struct literal instead of Default::default() + field-by-field assignment (field_reassign_with_default). - setup.rs, process.rs: move #[cfg(test)] modules to the end of the file so no real items follow them (items_after_test_module). - basecamp.rs: array literal instead of vec! for a borrow-only test fixture, and separate a trailing doc paragraph from a list item. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`build idl` and `build client` carried byte-identical copies of the optional-project-path arg parser. Promote the idl.rs copy to `pub(crate)` and reuse it from client.rs (which already imports from idl.rs), dropping the duplicate and its now-unused `PathBuf` import. Behavior-preserving; `cargo build` clean, idl tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two behavior-preserving cleanups in `resolve_selected_programs`,
both aligning with idioms already used elsewhere in deploy.rs:
- replace the `is_none()` guard + `unwrap_or_default()` re-extraction
with a `let Some(raw) = .. else { return .. }`, matching the
`let Some(..) = .. else` pattern used throughout the file.
- replace `.iter().any(|p| p == &candidate)` membership test with the
slice `.contains(&candidate)` it's equivalent to.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three control-flow blocks used the verbose match { Some(x) => x, None => return }
shape where let-else is the dominant idiom in this codebase (report.rs alone
uses it 16 times). Behavior is unchanged.
- template/copy.rs: patch_simple_tail_call_program_id marker lookup
- process.rs: port_open SocketAddr parse
- commands/run_state.rs: tmp file name derivation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ct_tarball `extract_tarball` bound `target` from an `if` whose then-branch was `continue` — a value-from-diverging-branch shape that reads awkwardly and is the only such case in the tree. Lift the empty-stripped-path check to a guard `continue` and bind `target` unconditionally afterward. Behavior unchanged; the explanatory comment stays on the guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The two localnet reset entrypoints triggered clippy::too_many_arguments while the equivalent command-handler functions in report.rs already carry #[allow(clippy::too_many_arguments)]. Add the same attribute so the build is warning-free and the lint handling is consistent across command modules. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The other format string in this file (legacy-config warning) uses plain single quotes inside the double-quoted literal; this one escaped them as \' for no reason. Unescape to match local style. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Flatten `else { if in_description { .. } }` to `else if in_description`,
matching the if/else-if chain style already used in the loop and removing
a needless level of indentation. Behavior unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The cache_root resolution bound (bootstrap_cache, _) where the second slot was always (), and the None arm destructured default_cache_root() only to re-wrap the path in another ((), ) tuple. Collapse to a plain let binding reading the path directly. Behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The doc comment describing reseed_after_wipe (reuses cmd_setup primitives, byte-equivalent wallet.state, extracted for the byte-equivalence test) sat orphaned above watch_loop, which it does not describe. Move it onto reseed_after_wipe, which had no doc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert the three remaining positional format-arg stragglers
("{}", var) to inline captured-identifier form, matching the
codebase's dominant style (87+ inline usages). Output is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The encoder.finish()?; statement already propagates errors via ? and drops the returned writer; the let _ = binding was the only such discard-of-?-Ok in the codebase, inconsistent with the local call()?; idiom used everywhere else. Behavior unchanged.
The working-tree-dirty branch bound a boolean only to negate it one line later. Match the literal in the if-let pattern instead, dropping a nesting level and the throwaway binding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the 5-level if-let/for/if pyramid in collect_api_headers with let-else + early-continue and a single is_some_and predicate, matching the let-else / is_some_and idioms already used elsewhere (idl.rs, repo.rs). Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Each cloned value is moved or dropped immediately after, so the clone allocates needlessly: - basecamp.rs captured_source_row: inline single-use ref_text into detail - run.rs cmd_run_inner: move resolved into PipelineParams (field shorthand) - migrate.rs: drop .clone() on basecamp_pin/basecamp_source Options Behavior unchanged. cargo build + clippy --lib clean, 387 lib tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stency The two reset-flag parses chained Item::as_value (method reference) into .and_then(|v| v.as_bool()) (closure), mixing styles within one expression. config.rs otherwise consistently uses method references for toml_edit accessors (Item::as_table, Item::as_str, String::as_str). Convert the two closures to Value::as_bool to match. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `curl` not-on-PATH bail passed the constant "logos-blockchain-circuits"
as a positional {} arg. Inline it to match the codebase's inlined-format-args
style; message and behavior are unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tyle The AGENTS_HEADER constant mixed literal source newlines (title break, bullet list) with redundant `\n\` line-continuations in the middle paragraph. Since the continuation lines carry no leading whitespace, `X\n\<newline>Y` produces the same string as a plain literal newline, so collapsing to literal newlines is behavior-preserving and matches the rest of the literal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Some(Commands::Create(args)) | Some(Commands::New(args)) duplicated the Some(..) wrapper; the same file already uses the factored or-pattern form (ErrorKind::DisplayHelp | ErrorKind::DisplayVersion at cli.rs:593). Use Some(Commands::Create(args) | Commands::New(args)) for consistency. Flagged by clippy::unnested_or_patterns. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two bail! strings mixed inline captured args with positional `{}`/`{:?}`
for plain local identifiers in a single format string — repo.rs pin
mismatch (resolved_pin/pin/head, alongside inline {label}) and
config.rs check_toml_value (bad, alongside inline {key}/{value:?}).
Inline them to match the codebase's uniform inline-arg convention.
Output unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The write_basecamp_state comment described `project_sources` / `dependencies` fields as "intentionally ignored ... removed in Phase 3", but those fields no longer exist on BasecampState and that removal has already happened. Keep only the accurate note that source lines live in [basecamp.modules.*]. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_flake_attr Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The comment narrated the removal of `parse_inline_string_array`, `unquote`, `escape_toml_string`, and the hand-rolled emitter — none of which exist in the file anymore. It documented nothing in the current code, sitting orphaned between a function and the test module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Iterate `for &name in names` instead of `for name in names`, matching the file's own convention (`for &k in kids`, `for &b in bytes`) and avoiding a `to_string` call on a `&&str` double reference. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ommands cmd_spel was the lone passthrough command-entry taking args: Vec<String> by value; cmd_client and cmd_idl both take &[String]. The vector is only borrowed internally (Command::args), never consumed, so this was a sharp local inconsistency and a needless allocation. Caller updated to pass by reference. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both subcommand arms built the same vec![build, optional-path] via a .map().unwrap_or_else() that differed only in the called command. Hoist into a small helper and append the optional path with Option's IntoIterator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The .map(|p| p.to_string()).unwrap_or_else(|| "unknown".to_string()) idiom was copy-pasted six times across localnet and run. Collapse into a shared process::pid_text helper using map_or_else. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert single-method closures to method references, matching the codebase's dominant style (.map(str::trim), .map(str::to_string), char::is_control). config.rs read_string chained .and_then(Item::as_str) straight into a closure-wrapped .to_string() in the same expression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The dominant local idiom for collecting strings is map(str::to_string)/ map(ToString::to_string) (8 sites) over the closure form |x| x.to_string() (2 sites). Align the two stragglers in localnet log-tail JSON rendering and the basecamp FakeProbe test fixture. Behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… idiom The build_args path-passthrough used .to_string_lossy().to_string(), while the established idiom for this exact .map(|x| x.to_string_lossy()...) shape in the codebase (project.rs, basecamp.rs) is .into_owned(). Aligns the closure and avoids the redundant Cow->String reallocation. Behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ult::succeeded Collapse the four repeated `error.is_some() || status.unwrap_or(1) != 0` warning guards in collect_tool_versions behind a single method, and flatten the basecamp.modules migration if-let chain via and_then(Item::as_table) to match the sibling and_then idiom one line above. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d57abd3 to
7474926
Compare
Both the inline [run].reset and per-profile [run.profiles.*].reset fields used the same get/as_value/as_bool/unwrap_or(false) chain. Factor it into a read_bool helper alongside read_string, matching the existing accessor idiom. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lbacks - basecamp: collapse the six repeated `if field.is_empty()` default-fills for [repos.basecamp]/[repos.lgpm] into a shared `fill_repo_defaults` helper. - run: bind joined paths before canonicalize so the fallback reuses the binding instead of recomputing the same `project.root.join(..)` expression, matching the `unwrap_or(binding)` idiom used elsewhere. Behavior-preserving; is_portable_basecamp is pure so eager attr selection is equivalent. fmt + clippy --all-targets clean; full test suite passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The project overlay tests reinvented temp-dir creation with a manual SystemTime/process-id naming scheme plus hand-rolled remove_dir_all cleanup at the end of every test. Every other test module in the crate (state, process, circuits, repo, doctor_checks, setup, and the sibling template::skills) uses tempfile::tempdir(), whose TempDir auto-cleans on drop. Align project.rs to that idiom, which also fixes the temp-dir leak when a test panics before reaching its manual cleanup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The git/rustc/cargo probes already iterate; docker/podman were hand-unrolled copies differing only by tool name. Fold them into the same loop idiom (format! warning, shared result binding). No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_error The is_connectivity_failure branch (emit JSON error + bail with the sequencer-unreachable hint) was copy-pasted verbatim across all three topup steps (preflight, init, pinata claim), differing only in the message string. Collapse into a connectivity_error helper that returns the anyhow::Error. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The vendored and cache-managed branches each called sync_repo_to_pin_at_path_with_opts with the same source, pin, and "lez" args, differing only in target path and sync options. Hoist the path+options selection into a tuple and make a single call site. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scrub_manifest_text was a one-line wrapper that forwarded verbatim to scrub_path_string. Inline the callers and delete the alias. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ensure_repo_present and reconcile_repo_source built the identical `git clone --no-hardlinks -- <source> <path>` command, differing only by the run_forwarded label. Extract a clone_repo helper so the clone flags live in one place. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ing) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…arse_failed flag Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pid_is_zombie and pid_command both ran ps -o <field>= -p <pid> with identical status-check + first-non-empty-line plumbing. Extract that into ps_field(pid, field) and have both call it. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The docker and podman Pass arms were byte-identical save the runtime name in the detail string. Resolve (runtime, path) first, then build the CheckRow once. Behavior unchanged: docker still wins when both are present. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…meContext::command The wallet-binary Command with the verbose NSSA_WALLET_HOME_DIR env was copy-pasted across deploy and wallet command sites. Hoist it into a WalletRuntimeContext::command() method; callers just append subcommand args. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
to_cargo_crate_name (new.rs) and sanitize_file_stem (idl.rs) were the same character-sanitization algorithm differing only in separator char and empty fallback. Extract a shared commands::sanitize_separated helper and make both named functions thin wrappers; behavior and call sites unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…secamp_state cmd_basecamp_launch and cmd_basecamp_install both loaded basecamp.state, verified both binaries exist on disk, and bailed with the identical setup hint. Factor that into one helper. The launch site's extra !is_empty() match guard was redundant with the .exists() check (an empty path never exists), so the single helper preserves behavior at both sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
02d66d5 to
249b6ba
Compare
# Conflicts: # src/commands/localnet.rs # src/commands/new.rs # src/commands/spel.rs # src/process.rs
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
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.
Summary
Continuation of the scaffold desloping pass from the existing branch/PR, moved to a
wozos-owned branch so the Pi can keep iterating.This includes:
Verification
cargo test