sledtools/pika branch #57

pika-git-ph-split-1

Split ph into internal modules

Target branch: master

Merge Commit: ca6fd726ecf463b8a911d5a5bab38d6b9828ac99

branch: merged tutorial: ready ci: success

Open CI Details

Continuous Integration

CI: success

Compact status on the review page, with full logs on the CI page.

Open CI Details

Latest run #73 success

2 passed

head 4d6e982d1b232b2c203ffbfab5f3adb8059f9a1b · queued 2026-03-25 19:27:20 · 2 lane(s)

queued 39s · ran 24s

check-notifications · success check-agent-contracts · success

Summary

This branch refactors the monolithic crates/ph/src/lib.rs file (~2,000 lines) into five focused internal modules: api (HTTP client and response types), commands (CLI command implementations), resolve (branch/lane/run resolution logic), session (authentication persistence and Nostr NIP-98 signing), and tests. The original lib.rs is reduced to module declarations, CLI struct definitions, and a thin run() dispatcher that delegates to commands::*. All types gain pub(crate) visibility, and no public API or behavior changes are introduced — this is a pure structural decomposition.

Tutorial Steps

Declare the new module structure in lib.rs

Intent: Replace the monolithic file with module declarations and keep only the CLI argument structs and the top-level `run()` dispatcher, delegating every command to `commands::*`.

Affected files: crates/ph/src/lib.rs

Evidence

@@ -1,44 +1,40 @@
-use std::fs;
-use std::io::{Read, Write};
-use std::path::{Path, PathBuf};
-use std::process::Command as ProcessCommand;
-use std::thread;
-use std::time::Duration;
+mod api;
+mod commands;
+mod resolve;
+mod session;
+
+#[cfg(test)]
+mod tests;
+
+use std::path::PathBuf;

@@ -50,7 +46,7 @@
-    #[arg(long, global = true, default_value_os_t = default_state_dir())]
+    #[arg(long, global = true, default_value_os_t = session::default_state_dir())]

@@ -123,1935 +119,3 @@
-fn cmd_login(cli: &Cli, args: LoginArgs) -> anyhow::Result<()> {
 ... (all implementations removed)

The original lib.rs contained every type, every command handler, all HTTP logic, session management, branch resolution, and tests — roughly 2,000 lines in a single file.

The refactored lib.rs now contains only:

Module declarations — mod api; mod commands; mod resolve; mod session; plus a conditional mod tests;.
CLI structs — Cli, PhCommand (the clap Subcommand enum), LoginArgs, LaneActionArgs, and RecoverRunArgs stay here because they define the public CLI surface.
The run() function — a thin match that delegates each variant to commands::cmd_*.

The default_state_dir() call in the Cli struct's #[arg] attribute now qualifies as session::default_state_dir() since the function moved to the session module.

Import cleanup is significant: std::fs, std::io, std::process::Command, std::thread, std::time::Duration, and all serde/reqwest/nostr/url imports are removed from lib.rs because they are no longer needed here.

Extract the API client and response types into api.rs

Intent: Isolate all HTTP request/response concerns — the `ApiClient` struct, every `*Response` deserialization type, and the generic `send`/`send_json`/`http_error` helpers — into a dedicated module.

Affected files: crates/ph/src/api.rs

Evidence

@@ -0,0 +1,494 @@
+use std::time::Duration;
+
+use anyhow::{Context, anyhow};
+use reqwest::blocking::{Client, RequestBuilder};
+use reqwest::{Method, StatusCode};
+use serde::{Deserialize, Serialize};

@@ +0,0 @@
+pub(crate) struct ApiClient {
+    base_url: String,
+    token: Option<String>,
+    client: Client,
+}

@@ +0,0 @@
+fn send_json<T>(request: RequestBuilder, method: Method, url: &str) -> anyhow::Result<T>

@@ +0,0 @@
+fn http_error(method: Method, url: &str, status: StatusCode, body: &str) -> anyhow::Error

The new api.rs (494 lines) owns everything related to talking to the pika forge HTTP API.

Types moved here

All response structs (ChallengeResponse, LoginResponse, MeResponse, BranchResolveResponse, BranchDetailResponse, BranchSummary, CiRun, CiLane, NightlyDetailResponse, LaneMutationResponse, RecoverRunResponse, WakeCiResponse, BranchActionResponse) plus supporting enums (CiLaneExecutionReason, CiLaneFailureKind, CiTargetHealthState) are moved verbatim. Their visibility changes from private to pub(crate) so other modules within the crate can use them.

ApiClient

The ApiClient struct encapsulates a reqwest::blocking::Client with a 5-second connect timeout and 30-second request timeout. Every API endpoint gets a dedicated method (e.g., challenge(), verify(), merge_branch(), branch_logs()) that calls through the private generic send() method.

Error handling

send_json and http_error are private helpers. http_error tries to extract a JSON "error" field from the response body for a human-readable message, falling back to the raw body or the HTTP status reason phrase.

A small utility encode_query_component wraps url::form_urlencoded::byte_serialize and is used for building query strings in resolve_branch and branch_logs.

Extract command implementations into commands.rs

Intent: Move every `cmd_*` function and its supporting display/render helpers out of lib.rs into a commands module, keeping the CLI handler logic cohesive and separate from API and session concerns.

Affected files: crates/ph/src/commands.rs

Evidence

@@ -0,0 +1,435 @@
+use crate::api::{
+    ApiClient, BranchActionResponse, BranchDetailResponse, BranchLogsResponse, CiLane,
+    CiLaneExecutionReason, CiTargetHealthState,
+};
+use crate::resolve::{
+    resolve_branch_lane, resolve_branch_ref, resolve_branch_run_id, resolve_nightly_lane,
+};
+use crate::session::{
+    Session, build_nip98_verify_event_json, load_session, login_nsec, remove_session,
+    resolve_authenticated_base_url, resolve_base_url, save_session,
+};

@@ +0,0 @@
+pub(crate) fn cmd_login(cli: &Cli, args: LoginArgs) -> anyhow::Result<()> {

@@ +0,0 @@
+pub(crate) fn cmd_wait(

@@ +0,0 @@
+pub(crate) fn render_branch_status(branch: &BranchDetailResponse) -> String {

The commands.rs module (435 lines) contains every user-facing command handler plus rendering utilities.

Command handlers

Each cmd_* function follows a consistent pattern:

Load or resolve the session via session::load_session / session::resolve_authenticated_base_url.
Build an ApiClient.
Resolve the branch reference via resolve::resolve_branch_ref.
Call the appropriate API method.
Print formatted output.

Commands included: cmd_login, cmd_whoami, cmd_logout, cmd_status, cmd_wait, cmd_logs, cmd_merge, cmd_close, cmd_url, cmd_fail_lane, cmd_requeue_lane, cmd_recover_run, cmd_wake_ci.

Rendering helpers

These private functions handle terminal output formatting:

render_branch_status / print_branch_status — multi-line branch + CI run summary.
print_branch_logs — log text output with pikaci metadata.
print_branch_action — merge/close confirmation.
render_lane_status_line — single-lane status with execution reason, failure kind, and target health annotations.
render_lane_snapshot_fragment — compact snapshot string used by cmd_wait to detect state changes.
branch_ci_active / branch_wait_snapshot / active_lane_titles — polling loop helpers.

Internal enum

LaneActionKind (Fail / Requeue) and execute_lane_action are kept private to this module, used by both cmd_fail_lane and cmd_requeue_lane to share the dispatch logic for branch vs. nightly lane mutations.

Note that cmd_logout now delegates to session::remove_session rather than directly calling fs::remove_file, improving separation of concerns.

Extract branch and lane resolution logic into resolve.rs

Intent: Centralize the logic for converting user-supplied branch names, IDs, lane selectors, and run IDs into validated API references, keeping this concern separate from both commands and the API client.

Affected files: crates/ph/src/resolve.rs

Evidence

@@ -0,0 (file not shown in truncated diff but referenced by commands.rs imports)
+use crate::resolve::{
+    resolve_branch_lane, resolve_branch_ref, resolve_branch_run_id, resolve_nightly_lane,
+};

The resolve.rs module (inferred from the imports in commands.rs) contains the following functions that were previously inline in lib.rs:

resolve_branch_ref — accepts an optional &str (branch name or numeric ID). If None, it runs git rev-parse --abbrev-ref HEAD to infer the current branch. If the value parses as i64, it's treated as a branch ID directly; otherwise it calls api.resolve_branch(name) to look it up.
resolve_branch_value — inner helper that handles the parse-or-resolve logic.
infer_current_branch — shells out to git and validates the result isn't detached HEAD.
resolve_branch_run_id — given a BranchDetailResponse and optional requested run ID, validates it exists or defaults to the first run.
resolve_branch_lane — searches across all CI runs on a branch for a lane matching by lane_id string or lane_run_id integer.
resolve_nightly_lane — same lane resolution but scoped to a NightlyDetailResponse.
resolve_lane_selector / lane_selector — low-level helpers that enforce the "exactly one of --lane or --lane-run-id" constraint.

The BranchRef struct (branch_id: i64, branch_name: Option<String>) also lives here.

Extracting this module means resolution logic can be tested independently from command I/O and API transport.

Extract session and authentication logic into session.rs

Intent: Isolate session persistence (save/load/remove), base URL resolution, Nostr NIP-98 event signing, and nsec input handling into their own module.

Affected files: crates/ph/src/session.rs

Evidence

@@ +0,0 (file not shown in truncated diff but referenced by commands.rs imports)
+use crate::session::{
+    Session, build_nip98_verify_event_json, load_session, login_nsec, remove_session,
+    resolve_authenticated_base_url, resolve_base_url, save_session,
+};

The session.rs module (inferred from the imports in commands.rs) owns all authentication and session state concerns.

Session struct

The Session struct (with fields base_url, token, npub, is_admin, can_forge_write) is serialized as JSON to <state_dir>/session.json.

Functions moved here

Function	Purpose
`default_state_dir()`	Returns `$XDG_STATE_HOME/ph`, `~/.local/state/ph`, or `.ph` as fallback. Referenced by `Cli` via `session::default_state_dir()`.
`session_path(state_dir)`	Computes the file path for the session file.
`save_session(state_dir, session)`	Creates the state directory and writes the session JSON.
`load_session(state_dir)`	Reads and deserializes the session, returning a clear error if not logged in.
`remove_session(state_dir)`	Deletes the session file, returning `Ok(true)` if it existed. This is a new helper that replaces the inline `fs::remove_file` in the old `cmd_logout`.
`resolve_base_url(cli_override, session)`	Determines the API base URL from CLI flag, session, or `DEFAULT_BASE_URL` constant.
`resolve_authenticated_base_url(cli_override, session)`	Same but uses the session's stored `base_url` as the default.
`login_nsec(nsec, nsec_file)`	Reads the Nostr secret key from argument, file, or stdin.
`read_nsec_file(path)`	File/stdin reader with empty-check.
`build_nip98_verify_event_json(nsec, base_url, challenge)`	Constructs and signs a NIP-98 auth event (Kind 27235) with `u` and `method` tags.

The DEFAULT_BASE_URL constant (https://git.pikachat.org) likely also moved here since it's only used by resolve_base_url.

Move tests into a dedicated tests.rs module

Intent: Separate test code from production code by placing it behind a `#[cfg(test)] mod tests` file, keeping lib.rs clean.

Affected files: crates/ph/src/tests.rs, crates/ph/src/lib.rs

Evidence

@@ -1,44 +1,40 @@
+#[cfg(test)]
+mod tests;

The #[cfg(test)] mod tests; declaration in lib.rs points to tests.rs, which contains the unit tests that were previously #[cfg(test)] blocks at the bottom of the monolithic file.

Because the tests module is a sibling of the other pub(crate) modules, it has full access to api::*, commands::*, resolve::*, and session::* types and functions through use crate::... imports. This keeps tests at the crate-integration level while remaining in a separate, maintainable file.

The conditional compilation attribute ensures test code is excluded from release builds, as before.

pika-git-ph-split-1

CI: success

Summary

Tutorial Steps

Declare the new module structure in lib.rs

Extract the API client and response types into api.rs

Types moved here

ApiClient

Error handling

Extract command implementations into commands.rs

Command handlers

Rendering helpers

Internal enum

Extract branch and lane resolution logic into resolve.rs

Extract session and authentication logic into session.rs

Session struct

Functions moved here

Move tests into a dedicated tests.rs module

Diff