From 51470e9937115e66b9fc02201812a590e67ca800 Mon Sep 17 00:00:00 2001 From: Forgejo Actions Date: Thu, 4 Jun 2026 17:53:31 -0700 Subject: [PATCH 01/18] Release v1.2.0 [skip ci] --- Cargo.lock | 10 +++++----- Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index be8f974..6557e6b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2196,7 +2196,7 @@ checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" [[package]] name = "heph" -version = "0.0.0" +version = "1.2.0" dependencies = [ "anyhow", "chrono", @@ -2210,7 +2210,7 @@ dependencies = [ [[package]] name = "heph-core" -version = "0.0.0" +version = "1.2.0" dependencies = [ "chrono", "proptest", @@ -2227,7 +2227,7 @@ dependencies = [ [[package]] name = "heph-quickadd" -version = "0.0.0" +version = "1.2.0" dependencies = [ "anyhow", "chrono", @@ -2243,7 +2243,7 @@ dependencies = [ [[package]] name = "heph-tui" -version = "0.0.0" +version = "1.2.0" dependencies = [ "anyhow", "chrono", @@ -2259,7 +2259,7 @@ dependencies = [ [[package]] name = "hephd" -version = "0.0.0" +version = "1.2.0" dependencies = [ "anyhow", "apple-native-keyring-store", diff --git a/Cargo.toml b/Cargo.toml index e24c881..d90143d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ members = [ [workspace.package] edition = "2021" -version = "0.0.0" +version = "1.2.0" license = "LicenseRef-Proprietary" publish = false authors = ["Erich Blume "] From a0be0f10856dc07935a2837e2e7efe777f98248f Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Fri, 5 Jun 2026 07:09:42 -0700 Subject: [PATCH 02/18] doc(heph-pwa): in-app Authentik login replaces manual token paste Document the PKCE 'Login with Authentik' flow, the hub /config zero-config discovery, and the redirect-URI prerequisite on the Authentik heph provider. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../heph-pwa-oidc-login.feature.md | 1 + docs/how-to/host-heph-pwa.md | 21 +++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) create mode 100644 docs/changelog.d/heph-pwa-oidc-login.feature.md diff --git a/docs/changelog.d/heph-pwa-oidc-login.feature.md b/docs/changelog.d/heph-pwa-oidc-login.feature.md new file mode 100644 index 0000000..aae9d26 --- /dev/null +++ b/docs/changelog.d/heph-pwa-oidc-login.feature.md @@ -0,0 +1 @@ +heph-pwa: added a **Login with Authentik** button — a proper browser OIDC sign-in (Authorization Code + PKCE) that replaces the manual bearer-token paste. The hub exposes an unauthenticated `GET /config` (`{issuer, client_id}`) so the app is zero-config when served from the hub; the PWA discovers the IdP endpoints, runs the PKCE redirect, exchanges the code for a token, and silently refreshes it (`offline_access`). The manual token field remains as a fallback. Requires the PWA origin registered as a redirect URI on the Authentik `heph` provider. diff --git a/docs/how-to/host-heph-pwa.md b/docs/how-to/host-heph-pwa.md index 0be1d59..63b3d76 100644 --- a/docs/how-to/host-heph-pwa.md +++ b/docs/how-to/host-heph-pwa.md @@ -95,16 +95,19 @@ app defaults its hub URL to its own origin. 1. Ensure the phone is on the tailnet (or can reach the proxy). 2. Open the hub URL (`https://indri..ts.net/`) and **Add to Home Screen**. 3. The app defaults its **Hub URL** to the origin it loaded from — no typing. -4. **Token:** the hub requires an OIDC bearer token, and the PWA does **not yet - implement the in-app device-code login** — paste a token into Settings → - Token for now. Obtain one via the device-code flow against the Authentik - client (the same flow the CLI uses; e.g. reuse the access token a logged-in - spoke cached, or run a one-off device-code grant). Tap **Test** to confirm. +4. **Sign in:** open **Settings → Login with Authentik**. The app reads the + hub's `GET /config` for the issuer + client id (zero-config) and runs an + Authorization-Code + PKCE redirect to Authentik; after you approve it lands + back on the app, signed in, and silently refreshes the token from then on. + (A manual **Bearer token** field remains as a fallback for hubs without + OIDC, or for pasting a one-off token.) -> **Known gap / next step:** wire the RFC 8628 device-code flow into the PWA's -> Settings so login is in-app (open the verification URL, poll for the token, -> store it, and refresh it) — removing the manual paste. Tracked as follow-up -> work for `heph-pwa`. +**Prerequisite — register the PWA redirect URI.** Browser PKCE needs the app's +origin registered on the Authentik `heph` provider's **Redirect URIs** (Authentik +also keys token-endpoint CORS off those origins). Add the PWA origin(s) with a +trailing slash, e.g. `https://heph.ops.eblu.me/` (and `http://localhost:8787/` +for local dev). In blumeops this is the `redirect_uris` list on the heph +provider blueprint. ## Upgrades From 1f81a2e6d9c2b2c46c869b89337455c99b3c718c Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Fri, 5 Jun 2026 07:17:05 -0700 Subject: [PATCH 03/18] feat(heph-pwa): Login with Authentik (Authorization Code + PKCE) Replace the manual bearer-token paste with a proper browser OIDC sign-in. - Hub: unauthenticated GET /config -> {issuer, client_id} (added after the auth layer), sourced from the verifier's new TokenVerifier::oidc_config(). Lets the PWA self-configure when served from the hub. Tests in web_serve.rs. - PWA: src/oauth.js implements PKCE (S256), the authorize redirect, the callback token exchange, and silent refresh (offline_access). Settings gains a "Login with Authentik" button (manual token kept under a fallback disclosure); rpc.js retries once on 401 via a refresh hook; app.js completes the callback / refreshes on load; sw.js skips caching the callback URL and ships oauth.js in the shell. Requires the PWA origin registered as a redirect URI on the Authentik provider. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/hephd/src/auth.rs | 12 ++ crates/hephd/src/sync.rs | 18 +++ crates/hephd/tests/web_serve.rs | 52 +++++++- heph-pwa/src/app.js | 98 +++++++++++++-- heph-pwa/src/oauth.js | 204 ++++++++++++++++++++++++++++++++ heph-pwa/src/rpc.js | 17 ++- heph-pwa/styles.css | 20 ++++ heph-pwa/sw.js | 9 +- 8 files changed, 413 insertions(+), 17 deletions(-) create mode 100644 heph-pwa/src/oauth.js diff --git a/crates/hephd/src/auth.rs b/crates/hephd/src/auth.rs index e3081a1..c601d90 100644 --- a/crates/hephd/src/auth.rs +++ b/crates/hephd/src/auth.rs @@ -49,6 +49,13 @@ pub trait TokenVerifier: Send + Sync { /// Validate `bearer` (the raw token, no `Bearer ` prefix) and return its /// claims, or an [`AuthError`]. fn verify(&self, bearer: &str) -> Result; + + /// The public OIDC parameters a browser client (the `heph-pwa`) needs to + /// start a login: `(issuer, client_id)`. Neither is a secret. `None` for + /// non-OIDC verifiers (e.g. test stubs). + fn oidc_config(&self) -> Option<(&str, &str)> { + None + } } /// What an OIDC provider's discovery document tells us (we only need the JWKS). @@ -156,4 +163,9 @@ impl TokenVerifier for OidcVerifier { .map_err(|e| AuthError::Invalid(e.to_string()))?; Ok(data.claims) } + + fn oidc_config(&self) -> Option<(&str, &str)> { + // The audience is the OIDC client id (Authentik sets `aud` to it). + Some((&self.issuer, &self.audience)) + } } diff --git a/crates/hephd/src/sync.rs b/crates/hephd/src/sync.rs index 41e5524..bfaa323 100644 --- a/crates/hephd/src/sync.rs +++ b/crates/hephd/src/sync.rs @@ -135,6 +135,10 @@ pub fn router_with_web( .route("/sync/push", post(push)) .route("/rpc", post(rpc_call)) .route_layer(middleware::from_fn_with_state(state.clone(), require_auth)) + // Unauthenticated: the public OIDC params (issuer + client id) a browser + // client reads to start a PKCE login. Added after the auth `route_layer` + // so it is NOT gated — the app needs it *before* it has a token. + .route("/config", get(config)) // The static shell is unauthenticated and lives behind the API routes. .fallback(serve_static) // Outermost: stamp CORS headers on every response and short-circuit the @@ -174,6 +178,20 @@ async fn cors(request: Request, next: Next) -> AxumResponse { response } +/// Public OIDC parameters for a browser client (the `heph-pwa`) to start a PKCE +/// login: `{ "issuer", "client_id" }`. Unauthenticated — neither value is a +/// secret. Returns an empty object `{}` when the hub runs without OIDC, so the +/// app can detect that and fall back to a manually pasted token. +async fn config(State(state): State) -> Json { + let body = state + .verifier + .as_ref() + .and_then(|v| v.oidc_config()) + .map(|(issuer, client_id)| serde_json::json!({ "issuer": issuer, "client_id": client_id })) + .unwrap_or_else(|| serde_json::json!({})); + Json(body) +} + /// Serve the PWA shell from `web_root` for any non-API path. Returns 404 when no /// `web_root` is configured. Unknown paths fall back to `index.html` so the SPA /// can own its own routing. Path traversal (`..`) is rejected. diff --git a/crates/hephd/tests/web_serve.rs b/crates/hephd/tests/web_serve.rs index b176137..bd43577 100644 --- a/crates/hephd/tests/web_serve.rs +++ b/crates/hephd/tests/web_serve.rs @@ -12,10 +12,23 @@ use std::thread; use std::time::Duration; use heph_core::{FixedClock, LocalStore}; +use hephd::auth::{AuthError, Claims, TokenVerifier}; use hephd::sync::{self, SharedStore}; const NOW: i64 = 1_704_067_200_000; // 2024-01-01T00:00:00Z +/// A verifier that never admits a token but advertises OIDC params, so we can +/// drive the unauthenticated `/config` route without a live IdP. +struct StubOidc; +impl TokenVerifier for StubOidc { + fn verify(&self, _bearer: &str) -> Result { + Err(AuthError::Missing) + } + fn oidc_config(&self) -> Option<(&str, &str)> { + Some(("https://idp.example/application/o/heph/", "heph")) + } +} + /// One parsed HTTP response: status line code, lowercased headers, and body. struct Resp { status: u16, @@ -64,6 +77,15 @@ fn request(addr: &str, method: &str, path: &str) -> Resp { /// an ephemeral port; return its `host:port`. The server thread + temp dirs live /// for the test's duration. fn start(web_root: Option) -> String { + start_with(None, web_root) +} + +/// As [`start`], but with an explicit token verifier (to exercise the `/config` +/// route, which reports the verifier's OIDC params). +fn start_with( + verifier: Option>, + web_root: Option, +) -> String { let (tx, rx) = mpsc::channel(); thread::spawn(move || { let rt = tokio::runtime::Builder::new_current_thread() @@ -78,7 +100,7 @@ fn start(web_root: Option) -> String { let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); tx.send(listener.local_addr().unwrap()).unwrap(); let _keep = dir; - let app = sync::router_with_web(shared, None, web_root); + let app = sync::router_with_web(shared, verifier, web_root); axum::serve(listener, app).await.unwrap(); }); }); @@ -161,3 +183,31 @@ fn no_web_root_yields_404_for_static_paths() { // Even the 404 carries CORS headers (it passed through the layer). assert_eq!(resp.header("access-control-allow-origin"), Some("*")); } + +#[test] +fn config_is_empty_without_oidc() { + let addr = start(None); + let resp = request(&addr, "GET", "/config"); + assert_eq!(resp.status, 200); + assert_eq!(resp.body.trim(), "{}"); +} + +#[test] +fn config_reports_oidc_params_unauthenticated() { + // Even on an authed hub, /config is reachable without a token (it is added + // after the auth layer) and reports the issuer + public client id. + let addr = start_with(Some(Arc::new(StubOidc)), None); + let resp = request(&addr, "GET", "/config"); + assert_eq!(resp.status, 200); + assert!( + resp.body + .contains("\"issuer\":\"https://idp.example/application/o/heph/\""), + "body was: {}", + resp.body + ); + assert!( + resp.body.contains("\"client_id\":\"heph\""), + "body was: {}", + resp.body + ); +} diff --git a/heph-pwa/src/app.js b/heph-pwa/src/app.js index f06ba06..32820e0 100644 --- a/heph-pwa/src/app.js +++ b/heph-pwa/src/app.js @@ -6,6 +6,7 @@ // rpc.js). Context/KB is read-only here (no nvim editing surface). import { Client, loadSettings, saveSettings, RpcError } from "./rpc.js"; +import * as oauth from "./oauth.js"; import { parse as quickParse } from "./quickadd.js"; import { today, parseDate, toEpochMs } from "./datespec.js"; import { @@ -40,7 +41,29 @@ const state = { lastUndo: null, // { label, run } }; -state.client = new Client(state.settings); +// Build the RPC client from the current settings, wiring an OIDC silent-refresh +// hook: on a 401 the client calls this to renew the token (oauth.js) and retry +// once before surfacing the error. +function makeClient() { + return new Client({ + baseUrl: state.settings.baseUrl, + token: state.settings.token, + refresh: async () => { + const tok = await oauth.ensureFreshToken(true); + applyToken(tok || ""); + return tok; + }, + }); +} + +// Adopt `token` as the active bearer: persist it and rebuild the client. +function applyToken(token) { + state.settings.token = token || ""; + saveSettings(state.settings); + state.client = makeClient(); +} + +state.client = makeClient(); // --- tiny DOM helper -------------------------------------------------------- @@ -692,27 +715,62 @@ function openSettings() { const url = h("input", { class: "qa-input", type: "url", placeholder: "https://hub.example.com:8787", value: state.settings.baseUrl, autocomplete: "off", inputmode: "url" }); const tok = h("input", { class: "qa-input", type: "password", placeholder: "Bearer token (optional)", value: state.settings.token, autocomplete: "off" }); const test = h("div", { class: "settings-test" }); + const setTest = (msg, ok) => { + test.textContent = msg; + test.className = "settings-test" + (ok === true ? " ok" : ok === false ? " bad" : ""); + }; const save = async () => { state.settings.baseUrl = url.value.trim(); state.settings.token = tok.value.trim(); saveSettings(state.settings); - state.client = new Client(state.settings); + state.client = makeClient(); closeModal(); reload(); }; const check = async () => { - test.textContent = "Checking…"; + setTest("Checking…", null); const probe = new Client({ baseUrl: url.value.trim(), token: tok.value.trim() }); try { const v = await probe.call("version", {}); - test.textContent = `✓ Connected (hephd ${v.version})`; - test.className = "settings-test ok"; + setTest(`✓ Connected (hephd ${v.version})`, true); } catch (e) { - test.textContent = `✗ ${e.message}`; - test.className = "settings-test bad"; + setTest(`✗ ${e.message}`, false); } }; + // Login with Authentik: read the hub's /config for the issuer + client id, + // then start the PKCE redirect (this navigates away and returns to init()). + const login = async () => { + const hub = url.value.trim() || state.settings.baseUrl; + if (!hub) return setTest("✗ Set the hub URL first.", false); + setTest("Contacting hub…", null); + const cfg = await oauth.fetchHubConfig(hub); + if (!cfg) { + return setTest("✗ Hub has no OIDC (/config) — paste a token, or enable OIDC on the hub.", false); + } + state.settings.baseUrl = hub; + saveSettings(state.settings); // persist before we navigate away + try { + await oauth.beginLogin(cfg); + } catch (e) { + setTest(`✗ ${e.message}`, false); + } + }; + const logout = () => { + oauth.clearAuth(); + applyToken(""); + closeModal(); + reload(); + }; + + const authRow = oauth.loggedIn() + ? h( + "div", + { class: "settings-auth" }, + h("span", { class: "settings-test ok" }, "✓ Signed in with Authentik"), + h("button", { class: "act", onclick: logout }, "Log out"), + ) + : h("button", { class: "qa-add settings-login", onclick: login }, "Login with Authentik"); openModal( h( @@ -721,8 +779,14 @@ function openSettings() { h("div", { class: "modal-title" }, "Settings"), h("label", { class: "settings-label" }, "Hub URL"), url, - h("label", { class: "settings-label" }, "Token"), - tok, + h("label", { class: "settings-label" }, "Sign-in"), + authRow, + h( + "details", + { class: "settings-manual" }, + h("summary", {}, "Or paste a bearer token"), + tok, + ), test, h( "div", @@ -730,7 +794,7 @@ function openSettings() { h("button", { class: "act", onclick: check }, "Test"), h("button", { class: "qa-add", onclick: save }, "Save"), ), - h("div", { class: "settings-hint" }, "The hub is your server-mode hephd. Leave the token blank if the hub runs without OIDC."), + h("div", { class: "settings-hint" }, "“Login with Authentik” needs OIDC enabled on the hub. Leave sign-in empty for a hub running without OIDC."), ), ); } @@ -802,6 +866,20 @@ async function init() { } }); + // OIDC: finish a redirect callback (back from Authentik), or refresh an + // existing session, so the first reload() already carries a valid bearer. + if (oauth.isCallback()) { + try { + applyToken(await oauth.completeLogin()); + toast("Signed in."); + } catch (e) { + toast(`Sign-in failed: ${e.message}`); + } + } else if (oauth.loggedIn()) { + const tok = await oauth.ensureFreshToken(); + if (tok) applyToken(tok); + } + render(); reload(); diff --git a/heph-pwa/src/oauth.js b/heph-pwa/src/oauth.js new file mode 100644 index 0000000..0bc0763 --- /dev/null +++ b/heph-pwa/src/oauth.js @@ -0,0 +1,204 @@ +// Browser OIDC sign-in for the PWA: Authorization Code + PKCE (RFC 7636) against +// the hub's IdP (Authentik). Unlike the CLI's device-code flow, a browser SPA +// uses a redirect + PKCE — no client secret, no polling. The resulting access +// token is the same bearer the hub's OidcVerifier checks (iss / aud=client_id / +// RS256 / exp), so once signed in the app talks to /rpc exactly as a pasted +// token would. We also keep a refresh token (offline_access) to renew silently. +// +// Zero-config: the hub serves GET /config -> { issuer, client_id }, so the app +// learns the IdP without the user typing anything when served from the hub. + +const AUTH_KEY = "heph-pwa:auth"; // localStorage: { issuer, clientId, access, refresh, expiresAt } +const PKCE_KEY = "heph-pwa:pkce"; // sessionStorage: in-flight { verifier, state, ... } + +// --- persistence ------------------------------------------------------------ + +export function loadAuth() { + try { + return JSON.parse(localStorage.getItem(AUTH_KEY) || "null"); + } catch { + return null; + } +} +function saveAuth(a) { + localStorage.setItem(AUTH_KEY, JSON.stringify(a)); +} +export function clearAuth() { + localStorage.removeItem(AUTH_KEY); +} +export function loggedIn() { + return !!loadAuth(); +} + +// --- PKCE helpers ----------------------------------------------------------- + +function b64url(bytes) { + return btoa(String.fromCharCode(...new Uint8Array(bytes))) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); +} +function randomString(nbytes = 32) { + const a = new Uint8Array(nbytes); + crypto.getRandomValues(a); + return b64url(a); +} +async function challengeOf(verifier) { + const digest = await crypto.subtle.digest("SHA-256", new TextEncoder().encode(verifier)); + return b64url(digest); +} + +// The redirect URI is the app's own base directory (query/hash stripped), so it +// is stable across the login start and the callback. Register this exact value +// (with trailing slash) on the Authentik provider, e.g. https://heph.ops.eblu.me/. +function redirectUri() { + return new URL(".", location.href).href; +} + +// --- discovery -------------------------------------------------------------- + +async function discover(issuer) { + const url = issuer.replace(/\/+$/, "") + "/.well-known/openid-configuration"; + const r = await fetch(url); + if (!r.ok) throw new Error(`OIDC discovery failed (HTTP ${r.status}).`); + const d = await r.json(); + if (!d.authorization_endpoint || !d.token_endpoint) { + throw new Error("OIDC discovery is missing authorization/token endpoints."); + } + return { authorize: d.authorization_endpoint, token: d.token_endpoint }; +} + +/** Read the hub's public OIDC params. Returns { issuer, clientId } or null. */ +export async function fetchHubConfig(baseUrl) { + try { + const r = await fetch(baseUrl.replace(/\/+$/, "") + "/config"); + if (!r.ok) return null; + const d = await r.json(); + if (!d.issuer || !d.client_id) return null; + return { issuer: d.issuer, clientId: d.client_id }; + } catch { + return null; + } +} + +// --- login (redirect away) -------------------------------------------------- + +/** Begin a PKCE login: stash the verifier+state and redirect to the IdP. */ +export async function beginLogin({ issuer, clientId }) { + const { authorize } = await discover(issuer); + const verifier = randomString(48); + const state = randomString(16); + const redirect_uri = redirectUri(); + sessionStorage.setItem( + PKCE_KEY, + JSON.stringify({ verifier, state, issuer, clientId, redirect_uri }), + ); + const params = new URLSearchParams({ + response_type: "code", + client_id: clientId, + redirect_uri, + scope: "openid offline_access", + state, + code_challenge: await challengeOf(verifier), + code_challenge_method: "S256", + }); + location.assign(`${authorize}?${params}`); +} + +// --- callback (back from the IdP) ------------------------------------------- + +/** True when the current URL is an OAuth redirect callback. */ +export function isCallback() { + const p = new URLSearchParams(location.search); + return (p.has("code") && p.has("state")) || p.has("error"); +} + +/** Exchange the callback code for tokens. Always cleans the URL. Returns the + * access token on success; throws on failure. */ +export async function completeLogin() { + const p = new URLSearchParams(location.search); + const cleanUrl = () => history.replaceState(null, "", redirectUri()); + let pkce = null; + try { + pkce = JSON.parse(sessionStorage.getItem(PKCE_KEY) || "null"); + } catch { + pkce = null; + } + sessionStorage.removeItem(PKCE_KEY); + + if (p.get("error")) { + cleanUrl(); + throw new Error(p.get("error_description") || p.get("error")); + } + if (!pkce || pkce.state !== p.get("state")) { + cleanUrl(); + throw new Error("Login state mismatch — please try again."); + } + + const { token } = await discover(pkce.issuer); + const body = new URLSearchParams({ + grant_type: "authorization_code", + code: p.get("code"), + client_id: pkce.clientId, + redirect_uri: pkce.redirect_uri, + code_verifier: pkce.verifier, + }); + const r = await fetch(token, { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body, + }); + cleanUrl(); + if (!r.ok) throw new Error(`Token exchange failed (HTTP ${r.status}).`); + const t = await r.json(); + saveAuth({ + issuer: pkce.issuer, + clientId: pkce.clientId, + access: t.access_token, + refresh: t.refresh_token || null, + expiresAt: Date.now() + (t.expires_in || 3600) * 1000, + }); + return t.access_token; +} + +// --- token lifecycle -------------------------------------------------------- + +/** Return a usable access token, refreshing if it is near expiry (or `force`). + * Returns null when not logged in or when a refresh fails (caller re-prompts). */ +export async function ensureFreshToken(force = false) { + const a = loadAuth(); + if (!a) return null; + const stillFresh = a.expiresAt - Date.now() > 60_000; + if (!force && stillFresh) return a.access; + if (!a.refresh) return force ? null : a.access; + try { + const { token } = await discover(a.issuer); + const body = new URLSearchParams({ + grant_type: "refresh_token", + refresh_token: a.refresh, + client_id: a.clientId, + }); + const r = await fetch(token, { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body, + }); + if (!r.ok) { + // Refresh token rejected (expired/revoked) — drop the session so the UI + // shows "signed out" and the user can log in again. + clearAuth(); + return null; + } + const t = await r.json(); + saveAuth({ + ...a, + access: t.access_token, + refresh: t.refresh_token || a.refresh, + expiresAt: Date.now() + (t.expires_in || 3600) * 1000, + }); + return t.access_token; + } catch { + // Network blip — keep the (possibly stale) token; the RPC layer will retry. + return a.access; + } +} diff --git a/heph-pwa/src/rpc.js b/heph-pwa/src/rpc.js index 8f8b690..0a33631 100644 --- a/heph-pwa/src/rpc.js +++ b/heph-pwa/src/rpc.js @@ -49,8 +49,10 @@ export class Client { return !!this.settings.baseUrl; } - /** Low-level call: returns the `result` value, or throws RpcError. */ - async call(method, params = {}) { + /** Low-level call: returns the `result` value, or throws RpcError. On a 401, + * tries the optional `settings.refresh()` hook once (OIDC silent refresh) and + * retries before surfacing the error. */ + async call(method, params = {}, _retried = false) { if (!this.configured) { throw new RpcError("No hub configured — open Settings and set the hub URL.", 0); } @@ -68,7 +70,16 @@ export class Client { } catch (e) { throw new RpcError(`Cannot reach hub at ${base} (${e.message}).`, 0); } - if (resp.status === 401) throw new RpcError("Unauthorized — set or refresh your token.", 401); + if (resp.status === 401) { + if (!_retried && typeof this.settings.refresh === "function") { + const fresh = await this.settings.refresh(); + if (fresh) { + this.settings.token = fresh; + return this.call(method, params, true); + } + } + throw new RpcError("Unauthorized — sign in again (Settings).", 401); + } if (resp.status === 403) throw new RpcError("Forbidden — this token does not own the hub.", 403); if (!resp.ok) throw new RpcError(`Hub returned HTTP ${resp.status}.`, resp.status); diff --git a/heph-pwa/styles.css b/heph-pwa/styles.css index ed64983..ec734a8 100644 --- a/heph-pwa/styles.css +++ b/heph-pwa/styles.css @@ -435,6 +435,26 @@ body { font-size: 12px; color: var(--fg-dim); } +.settings-login { + align-self: stretch; +} +.settings-auth { + display: flex; + align-items: center; + gap: 10px; +} +.settings-auth .settings-test { + flex: 1; +} +.settings-manual > summary { + font-size: 13px; + color: var(--fg-dim); + cursor: pointer; + margin-bottom: 6px; +} +.settings-manual[open] > summary { + margin-bottom: 10px; +} /* --- Search --- */ .search-pane { diff --git a/heph-pwa/sw.js b/heph-pwa/sw.js index 571d1e6..5793eab 100644 --- a/heph-pwa/sw.js +++ b/heph-pwa/sw.js @@ -1,7 +1,7 @@ // Service worker: cache the app shell so heph launches offline. Data is never // cached — every /rpc call must hit the live hub (and POSTs aren't cacheable // anyway). Bump CACHE when shell assets change to evict the old set. -const CACHE = "heph-pwa-v3"; +const CACHE = "heph-pwa-v4"; const SHELL = [ "./", "./index.html", @@ -9,6 +9,7 @@ const SHELL = [ "./manifest.webmanifest", "./src/app.js", "./src/rpc.js", + "./src/oauth.js", "./src/quickadd.js", "./src/datespec.js", "./src/fmt.js", @@ -31,8 +32,10 @@ self.addEventListener("activate", (e) => { self.addEventListener("fetch", (e) => { const req = e.request; // Only cache same-origin GETs (the shell). Everything else (RPC, cross-origin) - // goes straight to the network. - if (req.method !== "GET" || new URL(req.url).origin !== self.location.origin) { + // goes straight to the network. Skip URLs with a query string too, so the OAuth + // redirect callback (`/?code=…&state=…`) is never cached or served from cache. + const u = new URL(req.url); + if (req.method !== "GET" || u.origin !== self.location.origin || u.search) { return; } e.respondWith( From c8512b2b50a9940b0aa9d8e3640950843afd67fe Mon Sep 17 00:00:00 2001 From: Forgejo Actions Date: Fri, 5 Jun 2026 07:36:46 -0700 Subject: [PATCH 04/18] Update changelog for v1.2.1 [skip ci] --- CHANGELOG.md | 7 +++++++ docs/changelog.d/heph-pwa-oidc-login.feature.md | 1 - 2 files changed, 7 insertions(+), 1 deletion(-) delete mode 100644 docs/changelog.d/heph-pwa-oidc-login.feature.md diff --git a/CHANGELOG.md b/CHANGELOG.md index ad6ed44..8315268 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [v1.2.1] - 2026-06-05 + +### Features + +- heph-pwa: added a **Login with Authentik** button — a proper browser OIDC sign-in (Authorization Code + PKCE) that replaces the manual bearer-token paste. The hub exposes an unauthenticated `GET /config` (`{issuer, client_id}`) so the app is zero-config when served from the hub; the PWA discovers the IdP endpoints, runs the PKCE redirect, exchanges the code for a token, and silently refreshes it (`offline_access`). The manual token field remains as a fallback. Requires the PWA origin registered as a redirect URI on the Authentik `heph` provider. + + ## [v1.2.0] - 2026-06-04 ### Features diff --git a/docs/changelog.d/heph-pwa-oidc-login.feature.md b/docs/changelog.d/heph-pwa-oidc-login.feature.md deleted file mode 100644 index aae9d26..0000000 --- a/docs/changelog.d/heph-pwa-oidc-login.feature.md +++ /dev/null @@ -1 +0,0 @@ -heph-pwa: added a **Login with Authentik** button — a proper browser OIDC sign-in (Authorization Code + PKCE) that replaces the manual bearer-token paste. The hub exposes an unauthenticated `GET /config` (`{issuer, client_id}`) so the app is zero-config when served from the hub; the PWA discovers the IdP endpoints, runs the PKCE redirect, exchanges the code for a token, and silently refreshes it (`offline_access`). The manual token field remains as a fallback. Requires the PWA origin registered as a redirect URI on the Authentik `heph` provider. From 00da36c63727ef99c8586923b817c3b4cd334e86 Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Fri, 5 Jun 2026 17:31:11 -0700 Subject: [PATCH 05/18] doc(explanation): hub+spoke data-evolution / migration rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document why heph's op-based sync lets most new features (new link types, read-side queries, optional payload fields) ship without a coordinated migration across the hub and spokes, and the narrow case — a new required SQLite column the apply path writes — that does need a hub-first rollout. Groundwork for the indented/counted project sidebar, which is pure read-side (existing parent links + a GROUP BY) and needs no migration. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/explanation/explanation.md | 1 + docs/explanation/hub-spoke-data-evolution.md | 87 ++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 docs/explanation/hub-spoke-data-evolution.md diff --git a/docs/explanation/explanation.md b/docs/explanation/explanation.md index c4763ab..bbc81dc 100644 --- a/docs/explanation/explanation.md +++ b/docs/explanation/explanation.md @@ -12,3 +12,4 @@ Background context and design decisions. - [[design]] — Hephaestus design document: vision, data model, architecture, sync, and roadmap - [[task-lifecycle]] — the two-axis task model (lifecycle state × attention), drop vs delete, and where each task shows up +- [[hub-spoke-data-evolution]] — why op-based sync lets most new features skip migrations, and when a coordinated SQLite migration is actually required diff --git a/docs/explanation/hub-spoke-data-evolution.md b/docs/explanation/hub-spoke-data-evolution.md new file mode 100644 index 0000000..adf3453 --- /dev/null +++ b/docs/explanation/hub-spoke-data-evolution.md @@ -0,0 +1,87 @@ +--- +title: Hub + Spoke Data Evolution +modified: 2026-06-05 +tags: + - explanation + - sync +--- + +# Hub + Spoke Data Evolution + +How the data model evolves safely when nodes run different versions across the +hub/spoke deployment (indri is the hub; see [[set-up-sync-hub]] and +[[host-heph-pwa]]). The short version: **sync is op-based, not schema-based**, so +most new features need no coordinated migration — but adding a SQLite *column* +does. + +## Two independent layers + +heph keeps two layers that evolve on different clocks: + +1. **The op-log (synced).** Every change is an operation — `node.create`, + `node.set`, `task.set`, `link.add`, `link.remove`, … — carrying an HLC, an + origin device, and a JSON payload. Spokes push/pull ops to/from the hub; both + sides run the **same** merge logic from `heph-core` (`sqlite/apply.rs`). This + is the only thing that crosses the wire. +2. **The SQLite schema (local, per node).** Each node materializes ops into local + tables. The schema version is tracked by SQLite's `PRAGMA user_version` and + advanced by the ordered, append-only migration list in + `heph-core/src/sqlite/migrations.rs`. **No schema or migration state is ever + synced.** A spoke can sit on an older schema than the hub indefinitely. + +Because the wire format is ops — not rows — a node only has to understand the +*ops* its peers emit, not their table layout. + +## What forward/backward compatibility already buys you + +The merge engine is deliberately lenient: + +- **Unknown op types are stored but not applied** (`apply.rs`) — a spoke that + receives a newer op type keeps it in the log (so a later upgrade can replay it) + but doesn't choke on it. +- **Unknown payload fields are ignored.** Field extraction is by name + (`str_field` / `i64_field`), so a payload with extra keys an older node doesn't + recognize just drops the extras. +- **Links are schema-free.** A link's `type` is a string column. A brand-new link + kind (a new `LinkType`) needs no migration — every version reads it as text and + applies OR-set add/remove identically. + +## The rule of thumb + +| Change | Needs coordinated migration? | +|--------|------------------------------| +| New `LinkType` (e.g. a new relationship between nodes) | **No** — just emit `link.add` with the new `type` string | +| New optional/nullable scalar carried in an op payload | **No, if** every node's `apply` reads it defensively and tolerates its absence | +| New *read-side* feature over existing data (counts, hierarchy from existing `parent` links) | **No** — pure local queries, no op or schema change | +| New **required** SQLite column that `apply` must write on every relevant op | **Yes** — old spokes lack the column and the `UPDATE` fails | +| Renaming/removing a column other nodes' `apply` paths reference | **Yes** | + +## When a migration *is* required, do it hub-first + +If a change genuinely needs a new column that the apply path writes: + +1. Ship the migration to **every** node (hub and all spokes) **before** any node + emits an op that depends on the new column. The migration list is + append-only and ordered, so rolling the new `hephd` out everywhere is the + gate. +2. Keep new columns **nullable / defaulted** so an op that predates the column + still applies, and so a node that hasn't yet upgraded degrades to "field + absent" rather than erroring. +3. Prefer encoding the new fact as a **link or an op-payload field** over a new + column whenever you can — that keeps the change in the no-migration column of + the table above. + +## Worked example: indented, counted projects + +The sidebar's subproject indentation and per-project task counts (see +[[install-heph]] and the agenda surface in [[design]] §8.1) are a pure read-side +feature: + +- **Nesting** is read from `parent` links that already exist — created by + `heph project add --parent ` — via the existing + `project_subtree` traversal. +- **Counts** are a read-only `SELECT … GROUP BY` over the `tasks`/`links` tables. + +No new column, no new op type, no migration — it works against a hub and a spoke +on any schema version that already understands `parent` links. That is the case +the rule of thumb is meant to make obvious. From 9a487cbe3bafa5defc8471456d9e79b3fa3f0f96 Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Fri, 5 Jun 2026 17:44:43 -0700 Subject: [PATCH 06/18] feat(heph-tui,heph-pwa): humanized recurrence + indented/counted/scrolling project sidebar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundles the cosmetic/UI-polish backlog for the agenda surfaces. All read-side; no schema or sync change (see hub-spoke-data-evolution). - humanize_rrule (hephd::datespec): inverse of parse_recurrence — renders an RRULE as 'every other week', 'weekdays', 'yearly on Apr 15', etc.; falls back to the raw rule for unmodeled parts (COUNT/UNTIL/ordinal BYDAY). Mirrored in the PWA's datespec.js. Shown in the TUI recurs detail line and PWA task/qa previews instead of the raw FREQ= string. - project.overview RPC + Store::project_overview: each project's parent (via the existing 'parent' links) and direct outstanding-task count, a read-only query. - TUI sidebar: subprojects indented by depth, per-project counts, wider pane, and ListState + scrollbar so it scrolls instead of clipping on overflow. Tests: humanize parity (Rust + JS), round-trip through parse_recurrence, raw-passthrough; project_overview count/parent; sidebar tree ordering + cycle safety. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/heph-core/src/lib.rs | 2 +- crates/heph-core/src/model.rs | 18 ++ crates/heph-core/src/sqlite/mod.rs | 69 +++++- crates/heph-core/src/sqlite/tasks.rs | 55 ++++- crates/heph-core/src/store.rs | 10 +- crates/heph-tui/src/app.rs | 161 ++++++++++-- crates/heph-tui/src/backend.rs | 21 ++ crates/heph-tui/src/ui.rs | 84 ++++++- crates/heph-tui/tests/agenda.rs | 7 +- crates/hephd/src/datespec.rs | 233 ++++++++++++++++++ crates/hephd/src/remote.rs | 4 + crates/hephd/src/rpc.rs | 1 + .../tui-polish-project-tree.doc.md | 1 + .../tui-polish-project-tree.feature.md | 1 + heph-pwa/src/app.js | 6 +- heph-pwa/src/datespec.js | 140 +++++++++++ heph-pwa/test/parsers.test.mjs | 61 ++++- 17 files changed, 837 insertions(+), 37 deletions(-) create mode 100644 docs/changelog.d/tui-polish-project-tree.doc.md create mode 100644 docs/changelog.d/tui-polish-project-tree.feature.md diff --git a/crates/heph-core/src/lib.rs b/crates/heph-core/src/lib.rs index e5ff637..6554d48 100644 --- a/crates/heph-core/src/lib.rs +++ b/crates/heph-core/src/lib.rs @@ -38,7 +38,7 @@ pub use filter::{builtin as builtin_view, ListFilter, ViewSpec, BUILTIN_VIEWS}; pub use hlc::{Hlc, HlcClock}; pub use model::{ deterministic_id, Attention, Conflict, Health, Link, LinkType, NewNode, NewTask, Node, - NodeKind, SchedulePatch, SyncCursors, Task, TaskState, + NodeKind, ProjectOverview, SchedulePatch, SyncCursors, Task, TaskState, }; pub use oplog::Op; pub use ranking::{rank, Dimension, RankedTask, RANKING}; diff --git a/crates/heph-core/src/model.rs b/crates/heph-core/src/model.rs index 423c96b..783f4cf 100644 --- a/crates/heph-core/src/model.rs +++ b/crates/heph-core/src/model.rs @@ -314,6 +314,24 @@ pub struct Health { pub sync_status: String, } +/// A project plus the two facts a sidebar needs to render it as a counted, +/// indented tree (§8.1): its parent project (via a `parent` link, if any) and +/// the number of outstanding tasks filed **directly** under it. Pure read-side — +/// both derive from existing data, so this carries no schema or sync change (see +/// [[hub-spoke-data-evolution]]). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ProjectOverview { + /// The project node id. + pub id: String, + /// The project's title. + pub title: String, + /// The parent project's node id, or `None` for a top-level project. + pub parent_id: Option, + /// Outstanding tasks filed directly in this project (children counted under + /// their own row, not summed here). + pub outstanding: usize, +} + /// An ambiguous merge surfaced to the user (a discarded LWW value, tech-spec /// §12). The winning value is already in the store; this records what was /// dropped so `heph conflicts` can show and settle it. diff --git a/crates/heph-core/src/sqlite/mod.rs b/crates/heph-core/src/sqlite/mod.rs index 28d73b5..af07a8f 100644 --- a/crates/heph-core/src/sqlite/mod.rs +++ b/crates/heph-core/src/sqlite/mod.rs @@ -32,8 +32,8 @@ use crate::error::{Error, Result}; use crate::filter::ListFilter; use crate::hlc::Hlc; use crate::model::{ - Attention, Conflict, Health, Link, LinkType, NewNode, NewTask, Node, NodeKind, SchedulePatch, - SyncCursors, Task, TaskState, + Attention, Conflict, Health, Link, LinkType, NewNode, NewTask, Node, NodeKind, ProjectOverview, + SchedulePatch, SyncCursors, Task, TaskState, }; use crate::oplog::Op; use crate::ranking::RankedTask; @@ -297,6 +297,10 @@ impl Store for LocalStore { tasks::health(&self.conn, &self.owner_id) } + fn project_overview(&self) -> Result> { + tasks::project_overview(&self.conn, &self.owner_id) + } + fn search(&self, query: &str) -> Result> { nodes::search(&self.conn, &self.owner_id, query) } @@ -498,6 +502,67 @@ mod tests { assert!(store.project_scope("Nope").is_err()); } + #[test] + fn project_overview_carries_parent_and_direct_outstanding_count() { + use crate::model::{LinkType, NewNode, NewTask, NodeKind, TaskState}; + let mut store = store_at(1); + let mk_proj = |store: &mut LocalStore, title: &str| { + store + .create_node(NewNode { + kind: NodeKind::Project, + title: title.into(), + body: None, + }) + .unwrap() + .id + }; + let mk_task = |store: &mut LocalStore, title: &str, project: Option<&str>| { + store + .create_task(NewTask { + title: title.into(), + attention: None, + do_date: None, + late_on: None, + recurrence: None, + project_id: project.map(String::from), + }) + .unwrap() + .node_id + }; + + let work = mk_proj(&mut store, "Work"); + let sub = mk_proj(&mut store, "Work Sub"); + mk_proj(&mut store, "Garden"); + // Work Sub is a child of Work (child holds the `parent` link → parent). + store.add_link(&sub, &work, LinkType::Parent).unwrap(); + + // Two outstanding + one done in Work; one outstanding in the subproject. + mk_task(&mut store, "ship", Some(&work)); + mk_task(&mut store, "review", Some(&work)); + let done = mk_task(&mut store, "archived", Some(&work)); + store.set_task_state(&done, TaskState::Done).unwrap(); + mk_task(&mut store, "nested", Some(&sub)); + // An unfiled task counts toward no project. + mk_task(&mut store, "loose", None); + + let overview = store.project_overview().unwrap(); + // Title-sorted. + let titles: Vec<_> = overview.iter().map(|p| p.title.as_str()).collect(); + assert_eq!(titles, ["Garden", "Work", "Work Sub"]); + + let by_title = |t: &str| overview.iter().find(|p| p.title == t).unwrap(); + assert_eq!(by_title("Work").outstanding, 2, "done task excluded"); + assert_eq!(by_title("Work").parent_id, None); + assert_eq!( + by_title("Work Sub").outstanding, + 1, + "direct only, not summed" + ); + assert_eq!(by_title("Work Sub").parent_id, Some(work.clone())); + assert_eq!(by_title("Garden").outstanding, 0); + assert_eq!(by_title("Garden").parent_id, None); + } + #[test] fn resolve_project_is_fuzzy_only_when_unambiguous() { use crate::model::{NewNode, NodeKind}; diff --git a/crates/heph-core/src/sqlite/tasks.rs b/crates/heph-core/src/sqlite/tasks.rs index 3df2a53..29ac341 100644 --- a/crates/heph-core/src/sqlite/tasks.rs +++ b/crates/heph-core/src/sqlite/tasks.rs @@ -3,6 +3,8 @@ //! A committed task is a `task` node plus a `tasks` row. On creation it also //! gets a canonical context `doc` and a `canonical-context` link (tech-spec §6). +use std::collections::HashMap; + use rusqlite::{Connection, OptionalExtension, Row}; use serde_json::json; @@ -12,7 +14,7 @@ use crate::error::{Error, Result}; use crate::extract; use crate::filter::ListFilter; use crate::model::{ - Attention, Health, LinkType, NewTask, NodeKind, SchedulePatch, Task, TaskState, + Attention, Health, LinkType, NewTask, NodeKind, ProjectOverview, SchedulePatch, Task, TaskState, }; use crate::oplog::op_type; use crate::ranking::{self, RankedTask}; @@ -482,6 +484,57 @@ pub(super) fn health(conn: &Connection, owner: &str) -> Result { }) } +/// Every project (owner-scoped, non-tombstoned) with its parent project (via a +/// `parent` link) and its direct outstanding-task count — the shape a sidebar +/// renders as a counted, indented tree (§8.1). Pure read-side: counts come from +/// a single `GROUP BY` over the `in-project` links, parents from the `parent` +/// links, both already in the store. Title-sorted for a stable sibling order. +pub(super) fn project_overview(conn: &Connection, owner: &str) -> Result> { + // Direct outstanding count per project: each task's project is its first + // `in-project` link target (mirrors `list`/`load_candidates`). + let mut count_stmt = conn.prepare( + "SELECT (SELECT dst_id FROM links + WHERE src_id = n.id AND type = 'in-project' AND tombstoned = 0 + ORDER BY created_at, id LIMIT 1) AS project_id, + COUNT(*) + FROM nodes n JOIN tasks t ON t.node_id = n.id + WHERE n.owner_id = ?1 AND n.tombstoned = 0 AND t.state = 'outstanding' + GROUP BY project_id", + )?; + let mut counts: HashMap = HashMap::new(); + let rows = count_stmt.query_map([owner], |r| { + Ok((r.get::<_, Option>(0)?, r.get::<_, i64>(1)?)) + })?; + for row in rows { + let (project_id, count) = row?; + if let Some(pid) = project_id { + counts.insert(pid, count as usize); + } + } + + // Parent of each project: the dst of its (first) `parent` link. + let mut parent_stmt = conn.prepare( + "SELECT dst_id FROM links + WHERE src_id = ?1 AND type = 'parent' AND tombstoned = 0 + ORDER BY created_at, id LIMIT 1", + )?; + + let mut out = Vec::new(); + for node in nodes::list(conn, owner, Some(NodeKind::Project))? { + let parent_id = parent_stmt + .query_row([&node.id], |r| r.get::<_, String>(0)) + .optional()?; + out.push(ProjectOverview { + outstanding: counts.get(&node.id).copied().unwrap_or(0), + id: node.id, + title: node.title, + parent_id, + }); + } + out.sort_by(|a, b| a.title.cmp(&b.title)); + Ok(out) +} + /// Load every non-tombstoned committed task for `owner` as a ranking candidate, /// joining in its project and canonical-context link targets. fn load_candidates(conn: &Connection, owner: &str) -> Result> { diff --git a/crates/heph-core/src/store.rs b/crates/heph-core/src/store.rs index 72e13d3..473b3ee 100644 --- a/crates/heph-core/src/store.rs +++ b/crates/heph-core/src/store.rs @@ -7,8 +7,8 @@ use crate::error::Result; use crate::filter::ListFilter; use crate::model::{ - Attention, Conflict, Health, Link, LinkType, NewNode, NewTask, Node, NodeKind, SchedulePatch, - SyncCursors, Task, TaskState, + Attention, Conflict, Health, Link, LinkType, NewNode, NewTask, Node, NodeKind, ProjectOverview, + SchedulePatch, SyncCursors, Task, TaskState, }; use crate::oplog::Op; use crate::ranking::RankedTask; @@ -142,6 +142,12 @@ pub trait Store { /// Working-set health — orange/active/on-deck/conflict counts (tech-spec §7). fn health(&self) -> Result; + /// Every project with its parent (via a `parent` link) and its direct + /// outstanding-task count — the shape a sidebar renders as a counted, + /// indented tree (§8.1). Read-only over existing data; no schema or sync + /// change (see [[hub-spoke-data-evolution]]). + fn project_overview(&self) -> Result>; + /// Full-text search over title + body (FTS5), owner-scoped, best-match /// first, tombstones excluded (tech-spec §6). `query` is FTS5 MATCH syntax. fn search(&self, query: &str) -> Result>; diff --git a/crates/heph-tui/src/app.rs b/crates/heph-tui/src/app.rs index 2a89cdd..3ced067 100644 --- a/crates/heph-tui/src/app.rs +++ b/crates/heph-tui/src/app.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use anyhow::Result; use chrono::NaiveDate; -use heph_core::{Attention, RankedTask, SchedulePatch, TaskState, BUILTIN_VIEWS}; +use heph_core::{Attention, ProjectOverview, RankedTask, SchedulePatch, TaskState, BUILTIN_VIEWS}; use crate::backend::{Backend, Project, SearchHit}; use crate::fmt::{days_overdue, today_local}; @@ -313,8 +313,18 @@ pub enum Focus { #[derive(Debug, Clone, PartialEq, Eq)] pub enum SidebarEntry { Header(String), - View { name: String, title: String }, - Project { id: String, title: String }, + View { + name: String, + title: String, + }, + /// A project row. `depth` is its nesting level (0 = top-level) for indent; + /// `count` is its direct outstanding-task count, shown as a trailing chip. + Project { + id: String, + title: String, + depth: u16, + count: usize, + }, } impl SidebarEntry { @@ -323,6 +333,70 @@ impl SidebarEntry { } } +/// Turn the daemon's flat (title-sorted) project overview into sidebar rows in +/// tree order — each project followed by its descendants, carrying the nesting +/// `depth` and outstanding `count` the renderer needs. +fn project_entries(overview: Vec) -> Vec { + let order = order_projects(&overview); + let mut overview: Vec> = overview.into_iter().map(Some).collect(); + order + .into_iter() + .map(|(i, depth)| { + let p = overview[i].take().expect("each index visited once"); + SidebarEntry::Project { + id: p.id, + title: p.title, + depth, + count: p.outstanding, + } + }) + .collect() +} + +/// Depth-first display order over the project forest: returns `(index, depth)` +/// pairs, each project ahead of its children, siblings in the input's title +/// order. A project whose parent is missing (tombstoned, or not in the set) +/// renders at the top level; cycles can't loop (each node is emitted once). +fn order_projects(overview: &[ProjectOverview]) -> Vec<(usize, u16)> { + use std::collections::HashSet; + let ids: HashSet<&str> = overview.iter().map(|p| p.id.as_str()).collect(); + let mut children: HashMap<&str, Vec> = HashMap::new(); + let mut roots: Vec = Vec::new(); + for (i, p) in overview.iter().enumerate() { + match &p.parent_id { + Some(pid) if ids.contains(pid.as_str()) => { + children.entry(pid.as_str()).or_default().push(i); + } + _ => roots.push(i), + } + } + let mut out = Vec::with_capacity(overview.len()); + let mut visited = vec![false; overview.len()]; + // Stack of (index, depth); push siblings reversed so we pop in title order. + let mut stack: Vec<(usize, u16)> = roots.iter().rev().map(|&i| (i, 0)).collect(); + while let Some((i, depth)) = stack.pop() { + if visited[i] { + continue; + } + visited[i] = true; + out.push((i, depth)); + if let Some(kids) = children.get(overview[i].id.as_str()) { + for &k in kids.iter().rev() { + if !visited[k] { + stack.push((k, depth + 1)); + } + } + } + } + // Defensive: any node trapped in a parent-cycle still gets one top-level row. + for (i, seen) in visited.iter().enumerate() { + if !seen { + out.push((i, 0)); + } + } + out +} + /// The selected sidebar source, as owned values (so reloads don't hold a borrow /// on `self.sidebar` while calling the backend). enum Target { @@ -376,9 +450,7 @@ impl App { }); } sidebar.push(SidebarEntry::Header("Projects".into())); - for Project { id, title } in backend.projects()? { - sidebar.push(SidebarEntry::Project { id, title }); - } + sidebar.extend(project_entries(backend.project_overview()?)); let sidebar_cursor = sidebar .iter() @@ -423,7 +495,7 @@ impl App { /// The title of a project node id, resolved from the sidebar. pub fn project_name(&self, id: &str) -> Option { self.sidebar.iter().find_map(|e| match e { - SidebarEntry::Project { id: pid, title } if pid == id => Some(title.clone()), + SidebarEntry::Project { id: pid, title, .. } if pid == id => Some(title.clone()), _ => None, }) } @@ -469,7 +541,7 @@ impl App { self.sidebar .iter() .filter_map(|e| match e { - SidebarEntry::Project { id, title } => Some((id.clone(), title.clone())), + SidebarEntry::Project { id, title, .. } => Some((id.clone(), title.clone())), _ => None, }) .collect() @@ -742,7 +814,7 @@ impl App { /// become unfiled (they move to the Inbox), not deleted. pub fn begin_delete_project(&mut self) { match self.sidebar.get(self.sidebar_cursor) { - Some(SidebarEntry::Project { id, title }) => { + Some(SidebarEntry::Project { id, title, .. }) => { self.pending_delete = Some(PendingDelete::Project { project_id: id.clone(), title: title.clone(), @@ -881,10 +953,8 @@ impl App { .filter(|e| !matches!(e, SidebarEntry::Project { .. })) .cloned() .collect(); - if let Ok(projects) = self.backend.projects() { - for Project { id, title } in projects { - rebuilt.push(SidebarEntry::Project { id, title }); - } + if let Ok(overview) = self.backend.project_overview() { + rebuilt.extend(project_entries(overview)); } self.sidebar = rebuilt; // Restore the cursor: same entry if present, else the nearest selectable @@ -923,7 +993,7 @@ impl App { self.sidebar .iter() .filter_map(|e| match e { - SidebarEntry::Project { id, title } => Some(Project { + SidebarEntry::Project { id, title, .. } => Some(Project { id: id.clone(), title: title.clone(), }), @@ -1213,4 +1283,67 @@ mod sort_tests { // Alpha group (red before blue), then Beta, then project-less tasks last. assert_eq!(ids(&tasks), vec!["a_red", "a_blue", "b_white", "none_red"]); } + + fn po(id: &str, title: &str, parent: Option<&str>, outstanding: usize) -> ProjectOverview { + ProjectOverview { + id: id.into(), + title: title.into(), + parent_id: parent.map(str::to_string), + outstanding, + } + } + + #[test] + fn project_entries_nest_children_under_parents_with_depth_and_count() { + // Input arrives title-sorted from the daemon. + let overview = vec![ + po("g", "Garden", None, 0), + po("w", "Work", None, 2), + po("ws", "Work Sub", Some("w"), 1), + po("wsx", "Work Sub Sub", Some("ws"), 5), + ]; + let rows: Vec<(String, u16, usize)> = project_entries(overview) + .into_iter() + .map(|e| match e { + SidebarEntry::Project { + title, + depth, + count, + .. + } => (title, depth, count), + _ => unreachable!("project_entries yields only Project rows"), + }) + .collect(); + assert_eq!( + rows, + vec![ + ("Garden".into(), 0, 0), + ("Work".into(), 0, 2), + ("Work Sub".into(), 1, 1), + ("Work Sub Sub".into(), 2, 5), + ] + ); + } + + #[test] + fn project_entries_treat_a_missing_parent_as_top_level() { + // A child whose parent isn't in the set (e.g. tombstoned) still shows. + let overview = vec![po("orphan", "Orphan", Some("gone"), 3)]; + let rows = project_entries(overview); + assert!(matches!( + rows.as_slice(), + [SidebarEntry::Project { + depth: 0, + count: 3, + .. + }] + )); + } + + #[test] + fn order_projects_does_not_loop_on_a_parent_cycle() { + // a→b→a is pathological but must still terminate, each row once. + let overview = vec![po("a", "A", Some("b"), 0), po("b", "B", Some("a"), 0)]; + assert_eq!(order_projects(&overview).len(), 2); + } } diff --git a/crates/heph-tui/src/backend.rs b/crates/heph-tui/src/backend.rs index 784c520..88beaaa 100644 --- a/crates/heph-tui/src/backend.rs +++ b/crates/heph-tui/src/backend.rs @@ -26,6 +26,22 @@ pub struct SearchHit { pub trait Backend { /// All project nodes (for the sidebar), title-sorted. fn projects(&mut self) -> Result>; + /// Projects enriched with parent + direct outstanding-task count, for the + /// indented, counted sidebar tree (§8.1). The default derives a flat list + /// from [`projects`](Self::projects); the real backend forwards the + /// dedicated `project.overview` RPC. + fn project_overview(&mut self) -> Result> { + Ok(self + .projects()? + .into_iter() + .map(|p| heph_core::ProjectOverview { + id: p.id, + title: p.title, + parent_id: None, + outstanding: 0, + }) + .collect()) + } /// Run a built-in filter view (`tom|ondeck|chores|work|tasks`, §8.2). fn view(&mut self, name: &str) -> Result>; /// Run a raw [`ListFilter`] (used for per-project scope). @@ -103,6 +119,11 @@ impl Backend for ClientBackend { Ok(projects) } + fn project_overview(&mut self) -> Result> { + let v = self.call("project.overview", json!({}))?; + Ok(serde_json::from_value(v)?) + } + fn view(&mut self, name: &str) -> Result> { let v = self.call("view", json!({ "name": name }))?; Ok(serde_json::from_value(v)?) diff --git a/crates/heph-tui/src/ui.rs b/crates/heph-tui/src/ui.rs index 23305c6..e6043b8 100644 --- a/crates/heph-tui/src/ui.rs +++ b/crates/heph-tui/src/ui.rs @@ -37,7 +37,7 @@ pub fn render(frame: &mut Frame, app: &App) { let panes = Layout::default() .direction(Direction::Horizontal) .constraints([ - Constraint::Length(22), + Constraint::Length(28), Constraint::Min(28), Constraint::Length(38), ]) @@ -151,8 +151,25 @@ fn pane_border(focused: bool) -> Style { } } +/// The (label, trailing-count) styles for a sidebar row given its selection +/// state: a full-width cyan bar when focus-selected, reversed when selected in +/// the unfocused pane, otherwise plain with a dimmed count. +fn sidebar_row_styles(selected: bool, focused: bool) -> (Style, Style) { + if selected { + let s = if focused { + Style::default().fg(Color::Black).bg(Color::Cyan) + } else { + Style::default().add_modifier(Modifier::REVERSED) + }; + (s, s) + } else { + (Style::default(), Style::default().fg(Color::DarkGray)) + } +} + fn render_sidebar(frame: &mut Frame, app: &App, area: Rect) { let focused = app.focus == Focus::Sidebar; + let width = area.width.saturating_sub(2) as usize; // inside borders let items: Vec = app .sidebar .iter() @@ -166,17 +183,38 @@ fn render_sidebar(frame: &mut Frame, app: &App, area: Rect) { .fg(Color::DarkGray) .add_modifier(Modifier::BOLD), ))), - SidebarEntry::View { title, .. } | SidebarEntry::Project { title, .. } => { - let mut style = Style::default(); - if selected { - style = if focused { - style.fg(Color::Black).bg(Color::Cyan) - } else { - style.add_modifier(Modifier::REVERSED) - }; - } + SidebarEntry::View { title, .. } => { + let (style, _) = sidebar_row_styles(selected, focused); ListItem::new(Line::from(Span::styled(format!(" {title}"), style))) } + SidebarEntry::Project { + title, + depth, + count, + .. + } => { + // Indent two columns per nesting level (one base level so a + // top-level project still clears the pane border). + let indent = " ".repeat(1 + *depth as usize); + // A right-aligned outstanding-task count (blank when zero). + let count_str = if *count > 0 { + format!(" {count}") + } else { + String::new() + }; + let label_w = width.saturating_sub(count_str.chars().count()); + let title_room = label_w.saturating_sub(indent.chars().count()); + let title_trunc: String = title.chars().take(title_room).collect(); + let mut label = format!("{indent}{title_trunc}"); + let pad = label_w.saturating_sub(label.chars().count()); + label.push_str(&" ".repeat(pad)); + + let (label_style, count_style) = sidebar_row_styles(selected, focused); + ListItem::new(Line::from(vec![ + Span::styled(label, label_style), + Span::styled(count_str, count_style), + ])) + } } }) .collect(); @@ -187,7 +225,29 @@ fn render_sidebar(frame: &mut Frame, app: &App, area: Rect) { .border_style(pane_border(focused)) .title(" Views "), ); - frame.render_widget(list, area); + // Drive scroll-to-visible off the cursor so projects below the fold stay + // reachable; the row's own highlight remains the selection cue. + let mut state = ListState::default(); + state.select(Some(app.sidebar_cursor)); + frame.render_stateful_widget(list, area, &mut state); + + // A scrollbar once the entries can't all fit at once (position tracks the + // cursor — an honest "where am I in the list" signal). + let inner_h = area.height.saturating_sub(2) as usize; + if app.sidebar.len() > inner_h { + let mut sb = ScrollbarState::new(app.sidebar.len()).position(app.sidebar_cursor); + let bar = Scrollbar::new(ScrollbarOrientation::VerticalRight) + .begin_symbol(None) + .end_symbol(None); + frame.render_stateful_widget( + bar, + area.inner(Margin { + vertical: 1, + horizontal: 0, + }), + &mut sb, + ); + } } /// A dimmed `──── Project ────` group header for the project sort mode, padded @@ -239,7 +299,7 @@ fn task_detail_lines( } } if let Some(rrule) = &t.recurrence { - field("recurs:", rrule.clone()); + field("recurs:", hephd::datespec::humanize_rrule(rrule)); } if let Some(d) = t.do_date { field("do:", fmt_date(d, today)); diff --git a/crates/heph-tui/tests/agenda.rs b/crates/heph-tui/tests/agenda.rs index 89399e4..917f602 100644 --- a/crates/heph-tui/tests/agenda.rs +++ b/crates/heph-tui/tests/agenda.rs @@ -206,7 +206,12 @@ fn recurring_task_shows_glyph_and_selected_detail_block() { assert!(s.contains('↻'), "recurrence glyph missing:\n{s}"); // ...and the selected task's inline detail block (cursor starts on row 0). assert!(s.contains("recurs:"), "no recurrence detail:\n{s}"); - assert!(s.contains("FREQ=DAILY"), "no rrule in detail:\n{s}"); + // The RRULE is humanized for display (§8.1), not shown raw. + assert!(s.contains("daily"), "recurrence not humanized:\n{s}"); + assert!( + !s.contains("FREQ=DAILY"), + "raw rrule leaked into detail:\n{s}" + ); assert!(s.contains("project:"), "no project detail:\n{s}"); assert!(s.contains("Routines"), "project name missing:\n{s}"); } diff --git a/crates/hephd/src/datespec.rs b/crates/hephd/src/datespec.rs index 71d23e6..fa91611 100644 --- a/crates/hephd/src/datespec.rs +++ b/crates/hephd/src/datespec.rs @@ -288,6 +288,172 @@ fn parse_month_day(s: &str) -> Option<(u32, u32)> { None } +// --------------------------------------------------------------------------- +// Reverse datespec: humanize an RRULE for display (§8.1). +// --------------------------------------------------------------------------- + +/// Render an RFC-5545 RRULE back into the compact human phrasing the owner would +/// have typed — the inverse of [`parse_recurrence`] for the forms it produces: +/// `daily`, `every 3 days`, `every other day`, `weekly`, `every other week`, +/// `weekdays`, `every Fri`, `every other Wed`, `weekly on Mon, Wed, Fri`, +/// `monthly on the 5th`, `yearly on Apr 15`. Any rule that uses parts we don't +/// model (`COUNT`, `UNTIL`, `BYSETPOS`, ordinal `BYDAY` like `2MO`, …) is +/// returned **verbatim** so nothing is silently hidden from the reader. +pub fn humanize_rrule(rrule: &str) -> String { + humanize_known(rrule).unwrap_or_else(|| rrule.trim().to_string()) +} + +/// The fallible core: `None` whenever the rule contains anything we don't model, +/// so [`humanize_rrule`] can fall back to the raw text. +fn humanize_known(rrule: &str) -> Option { + let mut freq: Option = None; + let mut interval: u32 = 1; + let mut byday: Option = None; + let mut bymonth: Option = None; + let mut bymonthday: Option = None; + for part in rrule.trim().split(';') { + let part = part.trim(); + if part.is_empty() { + continue; + } + let (k, v) = part.split_once('=')?; + match k.trim().to_uppercase().as_str() { + "FREQ" => freq = Some(v.trim().to_uppercase()), + "INTERVAL" => interval = v.trim().parse().ok()?, + "BYDAY" => byday = Some(v.trim().to_uppercase()), + "BYMONTH" => bymonth = Some(v.trim().parse().ok()?), + "BYMONTHDAY" => bymonthday = Some(v.trim().parse().ok()?), + // A part we don't render → don't risk a misleading summary. + _ => return None, + } + } + + match freq?.as_str() { + "DAILY" => { + if byday.is_some() || bymonth.is_some() || bymonthday.is_some() { + return None; + } + Some(every_unit(interval, "day", "days", "daily")) + } + "WEEKLY" => { + if bymonth.is_some() || bymonthday.is_some() { + return None; + } + match byday { + None => Some(every_unit(interval, "week", "weeks", "weekly")), + Some(days) => { + if interval == 1 && is_weekday_set(&days) { + return Some("weekdays".into()); + } + let names = weekday_names(&days)?; + if names.len() == 1 { + let day = names[0]; + Some(match interval { + 1 => format!("every {day}"), + 2 => format!("every other {day}"), + n => format!("every {n} weeks on {day}"), + }) + } else { + let joined = names.join(", "); + Some(match interval { + 1 => format!("weekly on {joined}"), + 2 => format!("every other week on {joined}"), + n => format!("every {n} weeks on {joined}"), + }) + } + } + } + } + "MONTHLY" => { + if byday.is_some() || bymonth.is_some() { + return None; + } + match bymonthday { + None => Some(every_unit(interval, "month", "months", "monthly")), + Some(d @ 1..=31) => { + let day = ordinal(d as u32); + Some(match interval { + 1 => format!("monthly on the {day}"), + 2 => format!("every other month on the {day}"), + n => format!("every {n} months on the {day}"), + }) + } + Some(_) => None, // negative / out-of-range day-of-month → raw + } + } + "YEARLY" => { + if byday.is_some() { + return None; + } + match (bymonth, bymonthday) { + (None, None) => Some(every_unit(interval, "year", "years", "yearly")), + (Some(m @ 1..=12), Some(d @ 1..=31)) => { + let mon = MONTH_ABBR[(m - 1) as usize]; + Some(match interval { + 1 => format!("yearly on {mon} {d}"), + 2 => format!("every other year on {mon} {d}"), + n => format!("every {n} years on {mon} {d}"), + }) + } + _ => None, + } + } + _ => None, + } +} + +const MONTH_ABBR: [&str; 12] = [ + "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec", +]; + +/// `preset` for `n == 1`, `every other ` for 2, `every N ` otherwise. +fn every_unit(n: u32, singular: &str, plural: &str, preset: &str) -> String { + match n { + 1 => preset.to_string(), + 2 => format!("every other {singular}"), + n => format!("every {n} {plural}"), + } +} + +/// `1st`, `2nd`, `3rd`, `4th`, … `11th`, `21st`, `22nd`. +fn ordinal(n: u32) -> String { + let suffix = match (n % 10, n % 100) { + (_, 11..=13) => "th", + (1, _) => "st", + (2, _) => "nd", + (3, _) => "rd", + _ => "th", + }; + format!("{n}{suffix}") +} + +/// `MO,TU,WE,TH,FR` in any order (and only those), the inverse of the `weekdays` +/// preset. +fn is_weekday_set(byday: &str) -> bool { + let mut days: Vec<&str> = byday.split(',').map(str::trim).collect(); + days.sort_unstable(); + days == ["FR", "MO", "TH", "TU", "WE"] +} + +/// `BYDAY` tokens → capitalized weekday abbreviations, order preserved. `None` if +/// any token isn't a bare weekday (e.g. an ordinal `2MO`), so the caller falls +/// back to the raw rule. +fn weekday_names(byday: &str) -> Option> { + byday + .split(',') + .map(|t| match t.trim() { + "MO" => Some("Mon"), + "TU" => Some("Tue"), + "WE" => Some("Wed"), + "TH" => Some("Thu"), + "FR" => Some("Fri"), + "SA" => Some("Sat"), + "SU" => Some("Sun"), + _ => None, + }) + .collect() +} + #[cfg(test)] mod tests { use super::*; @@ -404,4 +570,71 @@ mod tests { ); assert!(parse_recurrence("every blue moon").is_err()); } + + #[test] + fn humanize_inverts_the_natural_language_forms() { + let cases = [ + ("FREQ=DAILY", "daily"), + ("FREQ=DAILY;INTERVAL=2", "every other day"), + ("FREQ=DAILY;INTERVAL=3", "every 3 days"), + ("FREQ=WEEKLY", "weekly"), + ("FREQ=WEEKLY;INTERVAL=2", "every other week"), + ("FREQ=MONTHLY", "monthly"), + ("FREQ=MONTHLY;INTERVAL=6", "every 6 months"), + ("FREQ=YEARLY", "yearly"), + ("FREQ=WEEKLY;BYDAY=FR", "every Fri"), + ("FREQ=WEEKLY;INTERVAL=2;BYDAY=WE", "every other Wed"), + ("FREQ=WEEKLY;INTERVAL=3;BYDAY=WE", "every 3 weeks on Wed"), + ("FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR", "weekdays"), + ("FREQ=WEEKLY;BYDAY=MO,WE,FR", "weekly on Mon, Wed, Fri"), + ("FREQ=YEARLY;BYMONTH=4;BYMONTHDAY=15", "yearly on Apr 15"), + ("FREQ=MONTHLY;BYMONTHDAY=5", "monthly on the 5th"), + ("FREQ=MONTHLY;BYMONTHDAY=22", "monthly on the 22nd"), + ("FREQ=MONTHLY;BYMONTHDAY=1", "monthly on the 1st"), + ("FREQ=MONTHLY;BYMONTHDAY=13", "monthly on the 13th"), + ]; + for (rrule, want) in cases { + assert_eq!(humanize_rrule(rrule), want, "humanizing {rrule}"); + } + } + + #[test] + fn humanize_round_trips_through_parse_recurrence() { + // For the interval/weekday forms the display text is itself a valid input: + // owner types text → we store an RRULE → we show it back → it re-parses to + // the same rule. (The `yearly on Apr 15` / `monthly on the 5th` forms are + // tuned for reading, not re-typing — the stored RRULE, never this string, + // is what gets parsed — so they're covered by the exact-output test above.) + for input in [ + "every 3 days", + "every other day", + "every other wed", + "weekdays", + "every fri", + "every 6 months", + "every 2 weeks", + ] { + let rrule = parse_recurrence(input).unwrap(); + let shown = humanize_rrule(&rrule); + assert_eq!( + parse_recurrence(&shown).unwrap(), + rrule, + "{input:?} → {rrule:?} → shown {shown:?} must re-parse to the same rule" + ); + } + } + + #[test] + fn humanize_falls_back_to_raw_for_unmodeled_rules() { + // COUNT/UNTIL/BYSETPOS and ordinal BYDAY would be misleading if dropped. + for raw in [ + "FREQ=DAILY;COUNT=5", + "FREQ=WEEKLY;UNTIL=20261231T000000Z", + "FREQ=MONTHLY;BYDAY=2MO", + "FREQ=MONTHLY;BYMONTHDAY=-1", + "not an rrule at all", + ] { + assert_eq!(humanize_rrule(raw), raw, "should pass {raw} through"); + } + } } diff --git a/crates/hephd/src/remote.rs b/crates/hephd/src/remote.rs index 43f8ad4..45a581d 100644 --- a/crates/hephd/src/remote.rs +++ b/crates/hephd/src/remote.rs @@ -221,6 +221,10 @@ impl Store for RemoteStore { self.call_as("health", json!({})) } + fn project_overview(&self) -> Result> { + self.call_as("project.overview", json!({})) + } + fn search(&self, query: &str) -> Result> { self.call_as("search", json!({ "query": query })) } diff --git a/crates/hephd/src/rpc.rs b/crates/hephd/src/rpc.rs index bc29eca..e46f7a6 100644 --- a/crates/hephd/src/rpc.rs +++ b/crates/hephd/src/rpc.rs @@ -352,6 +352,7 @@ pub fn dispatch(store: &mut dyn Store, method: &str, params: Value) -> Result json!(store.project_overview()?), "task.create" => { let p: NewTask = parse(params)?; json!(store.create_task(p)?) diff --git a/docs/changelog.d/tui-polish-project-tree.doc.md b/docs/changelog.d/tui-polish-project-tree.doc.md new file mode 100644 index 0000000..3d607c2 --- /dev/null +++ b/docs/changelog.d/tui-polish-project-tree.doc.md @@ -0,0 +1 @@ +New explanation card [[hub-spoke-data-evolution]] covering why heph's op-based sync lets most new features ship without a coordinated migration, and the narrow case (a new required SQLite column) that does need a hub-first rollout. diff --git a/docs/changelog.d/tui-polish-project-tree.feature.md b/docs/changelog.d/tui-polish-project-tree.feature.md new file mode 100644 index 0000000..bd76951 --- /dev/null +++ b/docs/changelog.d/tui-polish-project-tree.feature.md @@ -0,0 +1 @@ +Recurring tasks now show their schedule in plain language (`every other week`, `weekdays`, `yearly on Apr 15`) instead of a raw RRULE — in both the TUI detail pane and the mobile PWA. The TUI's project sidebar gained subproject indentation, per-project outstanding-task counts, a wider pane, and scrolling when the list overflows. diff --git a/heph-pwa/src/app.js b/heph-pwa/src/app.js index 32820e0..4452c89 100644 --- a/heph-pwa/src/app.js +++ b/heph-pwa/src/app.js @@ -8,7 +8,7 @@ import { Client, loadSettings, saveSettings, RpcError } from "./rpc.js"; import * as oauth from "./oauth.js"; import { parse as quickParse } from "./quickadd.js"; -import { today, parseDate, toEpochMs } from "./datespec.js"; +import { today, parseDate, toEpochMs, humanizeRecurrence } from "./datespec.js"; import { ATTENTION_COLORS, fmtRelative, @@ -212,7 +212,7 @@ function taskRow(t) { function taskDetail(t) { const meta = []; if (t.project_id) meta.push(["project", projectTitle(t.project_id)]); - if (t.recurrence) meta.push(["recurs", t.recurrence]); + if (t.recurrence) meta.push(["recurs", humanizeRecurrence(t.recurrence)]); if (t.do_date != null) meta.push(["do", fmtRelative(t.do_date)]); if (t.late_on != null) meta.push(["late", fmtRelative(t.late_on)]); @@ -373,7 +373,7 @@ function openQuickAdd() { } if (parsed.doDate != null) preview.append(h("span", { class: "qa-tag" }, "📅 " + fmtRelative(parsed.doDate))); if (parsed.projectId) preview.append(h("span", { class: "qa-tag" }, "📁 " + projectTitle(parsed.projectId))); - if (parsed.recurrence) preview.append(h("span", { class: "qa-tag" }, "↻ " + parsed.recurrence)); + if (parsed.recurrence) preview.append(h("span", { class: "qa-tag" }, "↻ " + humanizeRecurrence(parsed.recurrence))); }; input.addEventListener("input", updatePreview); input.addEventListener("keydown", (e) => { diff --git a/heph-pwa/src/datespec.js b/heph-pwa/src/datespec.js index 7c2986a..afe798a 100644 --- a/heph-pwa/src/datespec.js +++ b/heph-pwa/src/datespec.js @@ -219,3 +219,143 @@ export function parseRecurrenceOrNull(spec) { return null; } } + +// --------------------------------------------------------------------------- +// Reverse: humanize an RRULE for display (§8.1) — a faithful port of hephd's +// `datespec::humanize_rrule`. +// --------------------------------------------------------------------------- + +const MONTH_ABBR = [ + "Jan", "Feb", "Mar", "Apr", "May", "Jun", + "Jul", "Aug", "Sep", "Oct", "Nov", "Dec", +]; +const DAY_ABBR = { MO: "Mon", TU: "Tue", WE: "Wed", TH: "Thu", FR: "Fri", SA: "Sat", SU: "Sun" }; + +function everyUnit(n, singular, plural, preset) { + if (n === 1) return preset; + if (n === 2) return `every other ${singular}`; + return `every ${n} ${plural}`; +} + +function ordinal(n) { + const tens = n % 100; + if (tens >= 11 && tens <= 13) return `${n}th`; + switch (n % 10) { + case 1: return `${n}st`; + case 2: return `${n}nd`; + case 3: return `${n}rd`; + default: return `${n}th`; + } +} + +function isWeekdaySet(byday) { + const days = byday.split(",").map((s) => s.trim()).sort(); + return days.join(",") === "FR,MO,TH,TU,WE"; +} + +/** BYDAY tokens → capitalized weekday abbreviations, order preserved, or null + * if any token isn't a bare weekday (e.g. an ordinal `2MO`). */ +function weekdayNames(byday) { + const out = []; + for (const tok of byday.split(",")) { + const name = DAY_ABBR[tok.trim()]; + if (!name) return null; + out.push(name); + } + return out; +} + +/** + * Render an RFC-5545 RRULE back into the compact phrasing `parseRecurrence` + * accepts — `daily`, `every 3 days`, `every other week`, `weekdays`, + * `every Fri`, `every other Wed`, `weekly on Mon, Wed, Fri`, `monthly on the + * 5th`, `yearly on Apr 15`. Any rule using parts we don't model (COUNT, UNTIL, + * ordinal BYDAY, …) is returned **verbatim** so nothing is silently hidden. + */ +export function humanizeRecurrence(rrule) { + const known = humanizeKnown(rrule); + return known === null ? rrule.trim() : known; +} + +function humanizeKnown(rrule) { + let freq = null; + let interval = 1; + let byday = null; + let bymonth = null; + let bymonthday = null; + for (const rawPart of rrule.trim().split(";")) { + const part = rawPart.trim(); + if (part === "") continue; + const eq = part.indexOf("="); + if (eq === -1) return null; + const k = part.slice(0, eq).trim().toUpperCase(); + const v = part.slice(eq + 1).trim(); + switch (k) { + case "FREQ": freq = v.toUpperCase(); break; + case "INTERVAL": { + const n = Number(v); + if (!Number.isInteger(n) || n < 1) return null; + interval = n; + break; + } + case "BYDAY": byday = v.toUpperCase(); break; + case "BYMONTH": { + const n = Number(v); + if (!Number.isInteger(n)) return null; + bymonth = n; + break; + } + case "BYMONTHDAY": { + const n = Number(v); + if (!Number.isInteger(n)) return null; + bymonthday = n; + break; + } + default: return null; // a part we don't render → don't risk a wrong summary + } + } + + switch (freq) { + case "DAILY": + if (byday !== null || bymonth !== null || bymonthday !== null) return null; + return everyUnit(interval, "day", "days", "daily"); + case "WEEKLY": { + if (bymonth !== null || bymonthday !== null) return null; + if (byday === null) return everyUnit(interval, "week", "weeks", "weekly"); + if (interval === 1 && isWeekdaySet(byday)) return "weekdays"; + const names = weekdayNames(byday); + if (names === null) return null; + if (names.length === 1) { + const day = names[0]; + if (interval === 1) return `every ${day}`; + if (interval === 2) return `every other ${day}`; + return `every ${interval} weeks on ${day}`; + } + const joined = names.join(", "); + if (interval === 1) return `weekly on ${joined}`; + if (interval === 2) return `every other week on ${joined}`; + return `every ${interval} weeks on ${joined}`; + } + case "MONTHLY": { + if (byday !== null || bymonth !== null) return null; + if (bymonthday === null) return everyUnit(interval, "month", "months", "monthly"); + if (bymonthday < 1 || bymonthday > 31) return null; + const day = ordinal(bymonthday); + if (interval === 1) return `monthly on the ${day}`; + if (interval === 2) return `every other month on the ${day}`; + return `every ${interval} months on the ${day}`; + } + case "YEARLY": { + if (byday !== null) return null; + if (bymonth === null && bymonthday === null) { + return everyUnit(interval, "year", "years", "yearly"); + } + if (bymonth < 1 || bymonth > 12 || bymonthday < 1 || bymonthday > 31) return null; + const mon = MONTH_ABBR[bymonth - 1]; + if (interval === 1) return `yearly on ${mon} ${bymonthday}`; + if (interval === 2) return `every other year on ${mon} ${bymonthday}`; + return `every ${interval} years on ${mon} ${bymonthday}`; + } + default: return null; + } +} diff --git a/heph-pwa/test/parsers.test.mjs b/heph-pwa/test/parsers.test.mjs index 7c006d0..cd984fc 100644 --- a/heph-pwa/test/parsers.test.mjs +++ b/heph-pwa/test/parsers.test.mjs @@ -3,7 +3,12 @@ import test from "node:test"; import assert from "node:assert/strict"; -import { parseDate, parseRecurrence, toEpochMs } from "../src/datespec.js"; +import { + parseDate, + parseRecurrence, + humanizeRecurrence, + toEpochMs, +} from "../src/datespec.js"; import { parse } from "../src/quickadd.js"; const d = (y, m, day) => new Date(y, m - 1, day); @@ -58,6 +63,60 @@ test("recurrence natural language", () => { assert.throws(() => parseRecurrence("every blue moon")); }); +// Parity with hephd's `humanize_rrule` unit tests (datespec.rs). +test("humanize inverts the natural-language forms", () => { + const cases = [ + ["FREQ=DAILY", "daily"], + ["FREQ=DAILY;INTERVAL=2", "every other day"], + ["FREQ=DAILY;INTERVAL=3", "every 3 days"], + ["FREQ=WEEKLY", "weekly"], + ["FREQ=WEEKLY;INTERVAL=2", "every other week"], + ["FREQ=MONTHLY", "monthly"], + ["FREQ=MONTHLY;INTERVAL=6", "every 6 months"], + ["FREQ=YEARLY", "yearly"], + ["FREQ=WEEKLY;BYDAY=FR", "every Fri"], + ["FREQ=WEEKLY;INTERVAL=2;BYDAY=WE", "every other Wed"], + ["FREQ=WEEKLY;INTERVAL=3;BYDAY=WE", "every 3 weeks on Wed"], + ["FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR", "weekdays"], + ["FREQ=WEEKLY;BYDAY=MO,WE,FR", "weekly on Mon, Wed, Fri"], + ["FREQ=YEARLY;BYMONTH=4;BYMONTHDAY=15", "yearly on Apr 15"], + ["FREQ=MONTHLY;BYMONTHDAY=5", "monthly on the 5th"], + ["FREQ=MONTHLY;BYMONTHDAY=22", "monthly on the 22nd"], + ["FREQ=MONTHLY;BYMONTHDAY=1", "monthly on the 1st"], + ["FREQ=MONTHLY;BYMONTHDAY=13", "monthly on the 13th"], + ]; + for (const [rrule, want] of cases) { + assert.equal(humanizeRecurrence(rrule), want, `humanizing ${rrule}`); + } +}); + +test("humanize round-trips the re-typeable forms through parseRecurrence", () => { + for (const input of [ + "every 3 days", + "every other day", + "every other wed", + "weekdays", + "every fri", + "every 6 months", + "every 2 weeks", + ]) { + const rrule = parseRecurrence(input); + assert.equal(parseRecurrence(humanizeRecurrence(rrule)), rrule, `round-trip ${input}`); + } +}); + +test("humanize falls back to raw for unmodeled rules", () => { + for (const raw of [ + "FREQ=DAILY;COUNT=5", + "FREQ=WEEKLY;UNTIL=20261231T000000Z", + "FREQ=MONTHLY;BYDAY=2MO", + "FREQ=MONTHLY;BYMONTHDAY=-1", + "not an rrule at all", + ]) { + assert.equal(humanizeRecurrence(raw), raw, `passthrough ${raw}`); + } +}); + // quickadd.rs uses 2026-06-03 as `today` with projects Work + Camano Chores. const QTODAY = d(2026, 6, 3); const PROJECTS = [ From 4bf255b2111d7f19e11ff4eaf85eb586be1024c9 Mon Sep 17 00:00:00 2001 From: Forgejo Actions Date: Sat, 6 Jun 2026 09:30:28 -0700 Subject: [PATCH 07/18] Update changelog for v1.2.2 [skip ci] --- CHANGELOG.md | 11 +++++++++++ docs/changelog.d/tui-polish-project-tree.doc.md | 1 - docs/changelog.d/tui-polish-project-tree.feature.md | 1 - 3 files changed, 11 insertions(+), 2 deletions(-) delete mode 100644 docs/changelog.d/tui-polish-project-tree.doc.md delete mode 100644 docs/changelog.d/tui-polish-project-tree.feature.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 8315268..309e228 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [v1.2.2] - 2026-06-06 + +### Features + +- Recurring tasks now show their schedule in plain language (`every other week`, `weekdays`, `yearly on Apr 15`) instead of a raw RRULE — in both the TUI detail pane and the mobile PWA. The TUI's project sidebar gained subproject indentation, per-project outstanding-task counts, a wider pane, and scrolling when the list overflows. + +### Documentation + +- New explanation card [[hub-spoke-data-evolution]] covering why heph's op-based sync lets most new features ship without a coordinated migration, and the narrow case (a new required SQLite column) that does need a hub-first rollout. + + ## [v1.2.1] - 2026-06-05 ### Features diff --git a/docs/changelog.d/tui-polish-project-tree.doc.md b/docs/changelog.d/tui-polish-project-tree.doc.md deleted file mode 100644 index 3d607c2..0000000 --- a/docs/changelog.d/tui-polish-project-tree.doc.md +++ /dev/null @@ -1 +0,0 @@ -New explanation card [[hub-spoke-data-evolution]] covering why heph's op-based sync lets most new features ship without a coordinated migration, and the narrow case (a new required SQLite column) that does need a hub-first rollout. diff --git a/docs/changelog.d/tui-polish-project-tree.feature.md b/docs/changelog.d/tui-polish-project-tree.feature.md deleted file mode 100644 index bd76951..0000000 --- a/docs/changelog.d/tui-polish-project-tree.feature.md +++ /dev/null @@ -1 +0,0 @@ -Recurring tasks now show their schedule in plain language (`every other week`, `weekdays`, `yearly on Apr 15`) instead of a raw RRULE — in both the TUI detail pane and the mobile PWA. The TUI's project sidebar gained subproject indentation, per-project outstanding-task counts, a wider pane, and scrolling when the list overflows. From 11aa25c9f47eb9269494e03f6b31630c18a9c9c2 Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Sat, 6 Jun 2026 10:19:11 -0700 Subject: [PATCH 08/18] feat(heph-tui,hephd): surface sync health (last-sync age, conflicts, auth failure) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A spoke could be silently failing to sync (expired token → 401, or hub unreachable) with the only signal buried in the daemon log. Now: - hephd tracks SyncHealth (last attempt/success time, last error, auth-failure flag) from the background sync loop and sync.now, classifying a 401 as an auth failure. sync.status returns it plus the pending merge-conflict count. - heph-tui shows a live status-line indicator (spoke only): '⟳ ' since the last good sync, red '⚠ auth' when re-login is needed, '⚠ offline' when the hub is unreachable, and '⚠ N conflicts' when conflicts are pending. The event loop polls on a 2s tick so the age advances and failures appear while idle. - docs: recommended Authentik access/refresh token validity to stop frequent re-logins (with the iOS PWA localStorage-eviction caveat). Closes the 'Add hub connection status to heph-tui' and 'Spoke sync health: surface unhealthy state instead of silent 401 spam' backlog items. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/heph-tui/src/app.rs | 15 ++- crates/heph-tui/src/backend.rs | 39 ++++++ crates/heph-tui/src/fmt.rs | 33 +++++ crates/heph-tui/src/main.rs | 16 ++- crates/heph-tui/src/ui.rs | 136 +++++++++++++++++++- crates/heph-tui/tests/agenda.rs | 6 + crates/hephd/src/server.rs | 100 +++++++++++++- docs/changelog.d/tui-sync-health.doc.md | 1 + docs/changelog.d/tui-sync-health.feature.md | 1 + docs/how-to/host-heph-pwa.md | 5 + docs/how-to/set-up-sync-hub.md | 28 +++- 11 files changed, 364 insertions(+), 16 deletions(-) create mode 100644 docs/changelog.d/tui-sync-health.doc.md create mode 100644 docs/changelog.d/tui-sync-health.feature.md diff --git a/crates/heph-tui/src/app.rs b/crates/heph-tui/src/app.rs index 3ced067..e60a969 100644 --- a/crates/heph-tui/src/app.rs +++ b/crates/heph-tui/src/app.rs @@ -9,7 +9,7 @@ use anyhow::Result; use chrono::NaiveDate; use heph_core::{Attention, ProjectOverview, RankedTask, SchedulePatch, TaskState, BUILTIN_VIEWS}; -use crate::backend::{Backend, Project, SearchHit}; +use crate::backend::{Backend, Project, SearchHit, SyncStatus}; use crate::fmt::{days_overdue, today_local}; /// How the task list is ordered (toggled in the UI, §8.1). @@ -433,6 +433,8 @@ pub struct App { undo_stack: Vec, redo_stack: Vec, pub status: String, + /// Latest sync health for the status-line indicator (refreshed on a tick). + pub sync: SyncStatus, pub should_quit: bool, } @@ -472,12 +474,23 @@ impl App { undo_stack: Vec::new(), redo_stack: Vec::new(), status: String::new(), + sync: SyncStatus::default(), should_quit: false, }; app.reload(); + app.refresh_sync(); Ok(app) } + /// Refresh the sync-health snapshot for the status line. Best-effort: a + /// failed read leaves the previous snapshot in place (a stale indicator + /// beats a flicker), so this never disrupts navigation. + pub fn refresh_sync(&mut self) { + if let Ok(status) = self.backend.sync_status() { + self.sync = status; + } + } + /// The title shown above the task list (the selected source). pub fn task_pane_title(&self) -> String { match self.sidebar.get(self.sidebar_cursor) { diff --git a/crates/heph-tui/src/backend.rs b/crates/heph-tui/src/backend.rs index 88beaaa..a52fd90 100644 --- a/crates/heph-tui/src/backend.rs +++ b/crates/heph-tui/src/backend.rs @@ -22,6 +22,35 @@ pub struct SearchHit { pub kind: String, } +/// Sync health for the status line (the `sync.status` RPC). On a standalone +/// instance `hub_url` is `None` and `health` is absent; the conflict count is +/// always present. +#[derive(Debug, Clone, Default, PartialEq, Eq, serde::Deserialize)] +pub struct SyncStatus { + /// The hub this device syncs with, or `None` if standalone (no indicator). + pub hub_url: Option, + /// Pending merge conflicts awaiting resolution. + #[serde(default)] + pub conflicts: usize, + /// Observed health of the background sync loop (spoke only). + #[serde(default)] + pub health: Option, +} + +/// The spoke's observed sync health (mirrors `hephd`'s `SyncHealth`). +#[derive(Debug, Clone, Default, PartialEq, Eq, serde::Deserialize)] +pub struct SyncHealth { + /// Epoch ms of the last successful exchange ("last synced"), if any. + pub last_success_ms: Option, + /// Epoch ms of the last attempt (success or failure), if any. + pub last_attempt_ms: Option, + /// The last error message, cleared on the next success. + pub last_error: Option, + /// Whether the most recent attempt failed authentication (needs re-login). + #[serde(default)] + pub auth_failure: bool, +} + /// Everything the agenda surface asks of the daemon. pub trait Backend { /// All project nodes (for the sidebar), title-sorted. @@ -56,6 +85,11 @@ pub trait Backend { /// A task's canonical-context doc id (where its description/checklist live), /// for opening a task search-hit at the useful node. `None` if it has none. fn context_of(&mut self, task_id: &str) -> Result>; + /// Sync health for the status line. The default is a standalone instance + /// (no hub, no conflicts); the real backend forwards `sync.status`. + fn sync_status(&mut self) -> Result { + Ok(SyncStatus::default()) + } // --- triage mutations (T2) --- @@ -170,6 +204,11 @@ impl Backend for ClientBackend { .map(|l| l.dst_id)) } + fn sync_status(&mut self) -> Result { + let v = self.call("sync.status", json!({}))?; + Ok(serde_json::from_value(v)?) + } + fn set_state(&mut self, task_id: &str, state: &str) -> Result<()> { self.call("task.set_state", json!({ "id": task_id, "state": state }))?; Ok(()) diff --git a/crates/heph-tui/src/fmt.rs b/crates/heph-tui/src/fmt.rs index 3fc7373..2383a7e 100644 --- a/crates/heph-tui/src/fmt.rs +++ b/crates/heph-tui/src/fmt.rs @@ -25,6 +25,27 @@ pub fn today_local() -> NaiveDate { Local::now().date_naive() } +/// Now, in epoch milliseconds (the reference for [`fmt_age`]). +pub fn now_ms() -> i64 { + Local::now().timestamp_millis() +} + +/// A compact "how long ago" for the sync indicator: `just now` under a minute, +/// then `Nm` / `Nh` / `Nd`. Clamped at zero so a little clock skew never shows a +/// negative age. +pub fn fmt_age(now_ms: i64, then_ms: i64) -> String { + let secs = (now_ms - then_ms).max(0) / 1000; + if secs < 60 { + "just now".into() + } else if secs < 3_600 { + format!("{}m", secs / 60) + } else if secs < 86_400 { + format!("{}h", secs / 3_600) + } else { + format!("{}d", secs / 86_400) + } +} + /// How many days past its do-date a task is (0 if not overdue, no do-date, or /// future-dated). The "how overdue" signal the agenda sort ranks on (§8.1). pub fn days_overdue(do_date: Option, today: NaiveDate) -> i64 { @@ -102,6 +123,18 @@ mod tests { assert_eq!(fmt_date(ms(day(2027, 1, 1)), today), "2027-01-01"); } + #[test] + fn age_is_compact_and_clamped() { + let now = 1_000_000_000_000; + assert_eq!(fmt_age(now, now), "just now"); + assert_eq!(fmt_age(now, now - 30_000), "just now"); + assert_eq!(fmt_age(now, now - 5 * 60_000), "5m"); + assert_eq!(fmt_age(now, now - 3 * 3_600_000), "3h"); + assert_eq!(fmt_age(now, now - 2 * 86_400_000), "2d"); + // Clock skew (then in the future) never shows a negative age. + assert_eq!(fmt_age(now, now + 10_000), "just now"); + } + #[test] fn project_color_is_stable_distinct_and_neutral_when_absent() { assert_eq!(project_color(None), Color::DarkGray); diff --git a/crates/heph-tui/src/main.rs b/crates/heph-tui/src/main.rs index 27be96e..b672d7b 100644 --- a/crates/heph-tui/src/main.rs +++ b/crates/heph-tui/src/main.rs @@ -61,14 +61,22 @@ fn run( mut app: App, socket: &std::path::Path, ) -> Result<()> { + // Poll with a timeout so the sync indicator's age advances and a sync + // failure surfaces within a couple of seconds even while the user is idle. + let tick = std::time::Duration::from_secs(2); loop { terminal.draw(|f| ui::render(f, &app))?; - if let Event::Key(key) = event::read()? { - if key.kind == KeyEventKind::Press { - if let Some(action) = handle_key(&mut app, key) { - perform(terminal, &mut app, socket, action)?; + if event::poll(tick)? { + if let Event::Key(key) = event::read()? { + if key.kind == KeyEventKind::Press { + if let Some(action) = handle_key(&mut app, key) { + perform(terminal, &mut app, socket, action)?; + } } } + } else { + // Idle tick: refresh the sync-health snapshot for the status line. + app.refresh_sync(); } if app.should_quit { return Ok(()); diff --git a/crates/heph-tui/src/ui.rs b/crates/heph-tui/src/ui.rs index e6043b8..b457818 100644 --- a/crates/heph-tui/src/ui.rs +++ b/crates/heph-tui/src/ui.rs @@ -3,7 +3,7 @@ use heph_core::Attention; use ratatui::{ - layout::{Constraint, Direction, Layout, Margin, Rect}, + layout::{Alignment, Constraint, Direction, Layout, Margin, Rect}, style::{Color, Modifier, Style}, text::{Line, Span}, widgets::{ @@ -14,8 +14,8 @@ use ratatui::{ }; use crate::app::{App, Focus, InputState, Mode, MoveOption, MoveState, SidebarEntry, SortMode}; -use crate::backend::Backend; -use crate::fmt::{fmt_date, project_color, today_local}; +use crate::backend::{Backend, SyncStatus}; +use crate::fmt::{fmt_age, fmt_date, now_ms, project_color, today_local}; // Task-pane gestures (the focused pane shows its own hints, §8.1). const HINTS: &str = @@ -538,5 +538,133 @@ fn render_status(frame: &mut Frame, app: &App, area: Rect) { } else { Style::default().fg(Color::DarkGray) }; - frame.render_widget(Paragraph::new(Line::from(Span::styled(text, style))), area); + let left = Paragraph::new(Line::from(Span::styled(text, style))); + + // A right-aligned sync indicator (spoke only); the hints take the rest. + let indicator = sync_indicator(&app.sync, now_ms()); + if indicator.is_empty() { + frame.render_widget(left, area); + return; + } + let ind_w: usize = indicator.iter().map(|s| s.content.chars().count()).sum(); + let cols = + Layout::horizontal([Constraint::Min(1), Constraint::Length(ind_w as u16 + 1)]).split(area); + frame.render_widget(left, cols[0]); + frame.render_widget( + Paragraph::new(Line::from(indicator)).alignment(Alignment::Right), + cols[1], + ); +} + +/// The status-line sync indicator (empty on a standalone instance): a sync-state +/// chip — `⚠ auth` when re-login is needed, `⟳ ` since the last successful +/// sync, `⚠ offline` when erroring, `⟳ …` before the first sync — plus a +/// conflict chip when any merge conflicts are pending. +fn sync_indicator(sync: &SyncStatus, now: i64) -> Vec> { + if sync.hub_url.is_none() { + return Vec::new(); + } + let dim = Style::default().fg(Color::DarkGray); + let red = Style::default().fg(Color::Red).add_modifier(Modifier::BOLD); + let yellow = Style::default().fg(Color::Yellow); + + let health = sync.health.clone().unwrap_or_default(); + let mut spans = vec![if health.auth_failure { + Span::styled("⚠ auth", red) + } else if let Some(ts) = health.last_success_ms { + Span::styled(format!("⟳ {}", fmt_age(now, ts)), dim) + } else if health.last_error.is_some() { + Span::styled("⚠ offline", yellow) + } else { + Span::styled("⟳ …", dim) + }]; + + if sync.conflicts > 0 { + let label = if sync.conflicts == 1 { + "1 conflict".to_string() + } else { + format!("{} conflicts", sync.conflicts) + }; + spans.push(Span::raw(" ")); + spans.push(Span::styled(format!("⚠ {label}"), red)); + } + spans +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::backend::SyncHealth; + + fn render(sync: &SyncStatus, now: i64) -> String { + sync_indicator(sync, now) + .iter() + .map(|s| s.content.to_string()) + .collect() + } + + const NOW: i64 = 1_000_000_000_000; + + fn spoke(health: SyncHealth, conflicts: usize) -> SyncStatus { + SyncStatus { + hub_url: Some("http://hub:8787".into()), + conflicts, + health: Some(health), + } + } + + #[test] + fn standalone_shows_no_indicator() { + assert!(sync_indicator(&SyncStatus::default(), NOW).is_empty()); + } + + #[test] + fn indicator_reflects_each_sync_state() { + // Recently synced → a dim age chip. + let ok = spoke( + SyncHealth { + last_success_ms: Some(NOW - 5 * 60_000), + ..Default::default() + }, + 0, + ); + assert_eq!(render(&ok, NOW), "⟳ 5m"); + + // Auth failure wins over age (it's the actionable state). + let auth = spoke( + SyncHealth { + last_success_ms: Some(NOW - 60_000), + auth_failure: true, + ..Default::default() + }, + 0, + ); + assert_eq!(render(&auth, NOW), "⚠ auth"); + + // Errored with no prior success → offline. + let offline = spoke( + SyncHealth { + last_error: Some("error sending request".into()), + ..Default::default() + }, + 0, + ); + assert_eq!(render(&offline, NOW), "⚠ offline"); + + // Before the first sync. + assert_eq!(render(&spoke(SyncHealth::default(), 0), NOW), "⟳ …"); + } + + #[test] + fn conflicts_chip_appends_and_pluralizes() { + let h = SyncHealth { + last_success_ms: Some(NOW), + ..Default::default() + }; + assert_eq!( + render(&spoke(h.clone(), 1), NOW), + "⟳ just now ⚠ 1 conflict" + ); + assert_eq!(render(&spoke(h, 3), NOW), "⟳ just now ⚠ 3 conflicts"); + } } diff --git a/crates/heph-tui/tests/agenda.rs b/crates/heph-tui/tests/agenda.rs index 917f602..84ca739 100644 --- a/crates/heph-tui/tests/agenda.rs +++ b/crates/heph-tui/tests/agenda.rs @@ -98,6 +98,12 @@ fn agenda_renders_views_projects_and_tasks() { // The red/orange tasks carry a flag glyph in the leading column (§8.1). assert!(s.contains('⚑'), "attention flag glyph missing:\n{s}"); assert!(s.contains("Preview"), "preview pane missing:\n{s}"); + // A standalone daemon (no hub) shows no sync indicator — the `sync.status` + // RPC round-trips and reports `hub_url: null`. + assert!( + !s.contains('⟳'), + "sync indicator should be hidden without a hub:\n{s}" + ); } #[test] diff --git a/crates/hephd/src/server.rs b/crates/hephd/src/server.rs index 59826ac..30c5d5a 100644 --- a/crates/hephd/src/server.rs +++ b/crates/hephd/src/server.rs @@ -10,9 +10,10 @@ //! ops with the configured hub (tech-spec §6.1, §12). use std::sync::{Arc, Mutex}; -use std::time::Duration; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use anyhow::Result; +use serde::Serialize; use serde_json::{json, Value}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::{UnixListener, UnixStream}; @@ -32,6 +33,23 @@ struct SpokeAuth { client_id: String, } +/// A spoke's observed sync health, updated after every exchange (background loop +/// or manual `sync.now`). Surfaced by `sync.status` so clients can show whether +/// sync is actually working instead of trusting silence (tech-spec §3.1 / the +/// `Spoke sync health` task). All times are epoch ms; `None` means "not yet". +#[derive(Clone, Default, Serialize)] +struct SyncHealth { + /// When we last attempted an exchange. + last_attempt_ms: Option, + /// When we last completed one without error (the "last synced" time). + last_success_ms: Option, + /// The last error message, cleared on the next success. + last_error: Option, + /// Whether the most recent attempt failed authentication (a 401) — the + /// "re-auth needed" signal, distinct from a transient network blip. + auth_failure: bool, +} + /// The shared, cheaply-cloneable context each connection serves from. #[derive(Clone)] struct Ctx { @@ -43,6 +61,41 @@ struct Ctx { auth: Option, /// Opt-in self-update config (`Some` ⇒ enabled, tech-spec self-update card). self_update: Option, + /// Live sync health, shared between the background loop and `sync.status`. + sync_health: Arc>, +} + +/// Epoch-ms wall clock (the daemon may read it; only `heph-core` is clock-pure). +fn now_ms() -> i64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_millis() as i64) + .unwrap_or(0) +} + +/// True if `e` carries an HTTP 401 — i.e. the hub rejected our bearer token. +fn is_auth_error(e: &anyhow::Error) -> bool { + e.downcast_ref::() + .and_then(|re| re.status()) + .is_some_and(|s| s == reqwest::StatusCode::UNAUTHORIZED) +} + +/// Fold one exchange outcome into the shared [`SyncHealth`]. +fn record_sync_outcome(health: &Arc>, result: &Result) { + let now = now_ms(); + let mut h = health.lock().expect("sync_health mutex poisoned"); + h.last_attempt_ms = Some(now); + match result { + Ok(_) => { + h.last_success_ms = Some(now); + h.last_error = None; + h.auth_failure = false; + } + Err(e) => { + h.auth_failure = is_auth_error(e); + h.last_error = Some(e.to_string()); + } + } } impl Ctx { @@ -87,6 +140,7 @@ impl Daemon { .expect("building the daemon HTTP client"), auth: None, self_update: None, + sync_health: Arc::new(Mutex::new(SyncHealth::default())), }, } } @@ -170,7 +224,10 @@ impl Daemon { loop { tick.tick().await; let bearer = ctx.bearer().await; - match sync::sync_once(ctx.store.clone(), &hub, &ctx.http, bearer.as_deref()).await { + let result = + sync::sync_once(ctx.store.clone(), &hub, &ctx.http, bearer.as_deref()).await; + record_sync_outcome(&ctx.sync_health, &result); + match result { Ok(report) => tracing::debug!(?report, "background sync"), Err(e) => tracing::warn!("background sync failed: {e}"), } @@ -265,7 +322,9 @@ async fn sync_now(ctx: &Ctx) -> Result { }); }; let bearer = ctx.bearer().await; - match sync::sync_once(ctx.store.clone(), &hub_url, &ctx.http, bearer.as_deref()).await { + let result = sync::sync_once(ctx.store.clone(), &hub_url, &ctx.http, bearer.as_deref()).await; + record_sync_outcome(&ctx.sync_health, &result); + match result { Ok(report) => Ok(json!(report)), Err(e) => Err(RpcError { code: INTERNAL_ERROR, @@ -274,11 +333,28 @@ async fn sync_now(ctx: &Ctx) -> Result { } } -/// `sync.status` — the hub url and the current per-hub cursors. +/// `sync.status` — the hub url, the current per-hub cursors, the observed sync +/// health (last-success time / last error / auth-failure flag), and the pending +/// merge-conflict count. A spoke that is silently failing is visible here (and, +/// via it, in the TUI status line). async fn sync_status(ctx: &Ctx) -> Result { + // Conflict count is meaningful even on a hub / standalone instance. + let store = ctx.store.clone(); + let conflicts = tokio::task::spawn_blocking(move || { + let guard = store.lock().expect("store mutex poisoned"); + guard.conflicts_list().map(|c| c.len()) + }) + .await + .map_err(|e| RpcError { + code: INTERNAL_ERROR, + message: format!("sync.status task failed: {e}"), + })? + .map_err(RpcError::from)?; + let Some(hub_url) = ctx.hub_url.clone() else { - return Ok(json!({ "hub_url": Value::Null })); + return Ok(json!({ "hub_url": Value::Null, "conflicts": conflicts })); }; + let store = ctx.store.clone(); let hub = hub_url.clone(); let cursors = tokio::task::spawn_blocking(move || { @@ -291,5 +367,17 @@ async fn sync_status(ctx: &Ctx) -> Result { message: format!("sync.status task failed: {e}"), })? .map_err(RpcError::from)?; - Ok(json!({ "hub_url": hub_url, "cursors": cursors })) + + let health = ctx + .sync_health + .lock() + .expect("sync_health mutex poisoned") + .clone(); + + Ok(json!({ + "hub_url": hub_url, + "cursors": cursors, + "conflicts": conflicts, + "health": health, + })) } diff --git a/docs/changelog.d/tui-sync-health.doc.md b/docs/changelog.d/tui-sync-health.doc.md new file mode 100644 index 0000000..1176653 --- /dev/null +++ b/docs/changelog.d/tui-sync-health.doc.md @@ -0,0 +1 @@ +[[set-up-sync-hub]] now documents recommended Authentik token-validity settings (access + refresh token lifetime) to avoid frequent re-logins, with an iOS PWA storage-eviction caveat; [[host-heph-pwa]] points the PWA's login note at it. diff --git a/docs/changelog.d/tui-sync-health.feature.md b/docs/changelog.d/tui-sync-health.feature.md new file mode 100644 index 0000000..1225721 --- /dev/null +++ b/docs/changelog.d/tui-sync-health.feature.md @@ -0,0 +1 @@ +heph-tui's status line now shows a live sync indicator for spokes: how long since the last successful sync (`⟳ 5m`), a red `⚠ auth` when the hub is rejecting the token (re-login needed), `⚠ offline` when the hub is unreachable, and a `⚠ N conflicts` chip when merge conflicts are pending. The daemon tracks this health and exposes it via `sync.status` (also visible in `heph sync --status`), so a silently-broken spoke is obvious at a glance instead of buried in the log. diff --git a/docs/how-to/host-heph-pwa.md b/docs/how-to/host-heph-pwa.md index 63b3d76..dc22359 100644 --- a/docs/how-to/host-heph-pwa.md +++ b/docs/how-to/host-heph-pwa.md @@ -102,6 +102,11 @@ app defaults its hub URL to its own origin. (A manual **Bearer token** field remains as a fallback for hubs without OIDC, or for pasting a one-off token.) + > Re-prompted for login too often? The fix is the Authentik provider's + > **refresh token validity**, not the app — see the token-lifetime note in + > [[set-up-sync-hub]]. (On iOS, Safari may also purge an un-installed PWA's + > storage after ~7 idle days; Add to Home Screen mitigates it.) + **Prerequisite — register the PWA redirect URI.** Browser PKCE needs the app's origin registered on the Authentik `heph` provider's **Redirect URIs** (Authentik also keys token-endpoint CORS off those origins). Add the PWA origin(s) with a diff --git a/docs/how-to/set-up-sync-hub.md b/docs/how-to/set-up-sync-hub.md index a0f7706..a5b56ea 100644 --- a/docs/how-to/set-up-sync-hub.md +++ b/docs/how-to/set-up-sync-hub.md @@ -51,6 +51,26 @@ need: - **Issuer** — e.g. `https://authentik.ops.eblu.me/application/o/heph/` - **Client id** — the device-code client id (this is also the token *audience*). +### Token lifetime (avoid frequent re-logins) + +Token lifetimes are set on the Authentik **provider**, not in heph — heph honors +whatever `expires_in` Authentik returns and silently refreshes using the +`offline_access` refresh token (both the CLI/daemon and the PWA do this). To +avoid re-authenticating often, set generous validities on the heph provider: + +- **Access token validity** — e.g. `hours=24`. The hub validates `exp` and keeps + no revocation list, so this is the window in which a leaked token stays usable; + on a Tailscale-only hub, 24–48h is a reasonable trade. +- **Refresh token validity** — e.g. `days=30`+. This is the setting that stops + the re-logins: while the refresh token is valid, the spoke **and** the PWA + renew silently with no browser round-trip. A short refresh window is the usual + cause of "I have to log in constantly". + +> **iOS PWA caveat:** Safari can purge an *un-installed* PWA's `localStorage` +> (where its tokens live) after ~7 idle days regardless of these settings. +> Installing the app to the home screen mitigates it, but expect the occasional +> re-login on iOS. + ## 2. Bring up the hub on `indri` **Seed it from `gilbert` (Path A).** Quiesce `gilbert` (`heph daemon stop`), @@ -98,10 +118,16 @@ and background-syncs on its interval. ## 4. Verify ```bash -heph sync --status # last push/pull cursors, hub url +heph sync --status # hub url, last push/pull cursors, sync health heph sync # force a cycle now ``` +`heph sync --status` also reports **sync health** — the time of the last +successful exchange, any last error, and whether the spoke is currently failing +to authenticate. The same signal is surfaced live in `heph-tui`'s status line +(last-sync age · pending conflicts · an auth-failure flag), so a silently-broken +spoke is visible at a glance rather than buried in the daemon log. + Make a change on `gilbert`, force a sync, and confirm it appears via the hub. ## Current gaps (finalized by the blumeops deployment) From 1a8752f124a3842c2fb7aef754b017b01fba6bff Mon Sep 17 00:00:00 2001 From: Forgejo Actions Date: Sat, 6 Jun 2026 11:03:45 -0700 Subject: [PATCH 09/18] Update changelog for v1.2.3 [skip ci] --- CHANGELOG.md | 11 +++++++++++ docs/changelog.d/tui-sync-health.doc.md | 1 - docs/changelog.d/tui-sync-health.feature.md | 1 - 3 files changed, 11 insertions(+), 2 deletions(-) delete mode 100644 docs/changelog.d/tui-sync-health.doc.md delete mode 100644 docs/changelog.d/tui-sync-health.feature.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 309e228..9799e3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [v1.2.3] - 2026-06-06 + +### Features + +- heph-tui's status line now shows a live sync indicator for spokes: how long since the last successful sync (`⟳ 5m`), a red `⚠ auth` when the hub is rejecting the token (re-login needed), `⚠ offline` when the hub is unreachable, and a `⚠ N conflicts` chip when merge conflicts are pending. The daemon tracks this health and exposes it via `sync.status` (also visible in `heph sync --status`), so a silently-broken spoke is obvious at a glance instead of buried in the log. + +### Documentation + +- [[set-up-sync-hub]] now documents recommended Authentik token-validity settings (access + refresh token lifetime) to avoid frequent re-logins, with an iOS PWA storage-eviction caveat; [[host-heph-pwa]] points the PWA's login note at it. + + ## [v1.2.2] - 2026-06-06 ### Features diff --git a/docs/changelog.d/tui-sync-health.doc.md b/docs/changelog.d/tui-sync-health.doc.md deleted file mode 100644 index 1176653..0000000 --- a/docs/changelog.d/tui-sync-health.doc.md +++ /dev/null @@ -1 +0,0 @@ -[[set-up-sync-hub]] now documents recommended Authentik token-validity settings (access + refresh token lifetime) to avoid frequent re-logins, with an iOS PWA storage-eviction caveat; [[host-heph-pwa]] points the PWA's login note at it. diff --git a/docs/changelog.d/tui-sync-health.feature.md b/docs/changelog.d/tui-sync-health.feature.md deleted file mode 100644 index 1225721..0000000 --- a/docs/changelog.d/tui-sync-health.feature.md +++ /dev/null @@ -1 +0,0 @@ -heph-tui's status line now shows a live sync indicator for spokes: how long since the last successful sync (`⟳ 5m`), a red `⚠ auth` when the hub is rejecting the token (re-login needed), `⚠ offline` when the hub is unreachable, and a `⚠ N conflicts` chip when merge conflicts are pending. The daemon tracks this health and exposes it via `sync.status` (also visible in `heph sync --status`), so a silently-broken spoke is obvious at a glance instead of buried in the log. From c9bb2cbe648b27789f24ae89a2fabe5282c54149 Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Sat, 6 Jun 2026 11:24:09 -0700 Subject: [PATCH 10/18] feat(heph-tui): show sync age in seconds under a minute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The background sync loop runs every 30s, so the last-sync age never crossed the 60s 'just now' threshold — the chip always read 'just now', which also masked the first missed sync (age 30-60s looked identical to a fresh one). Show seconds under a minute ('⟳ 26s') so the chip is a visible heartbeat and a stalled sync surfaces ~30s sooner. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/heph-tui/src/fmt.rs | 17 ++++++++++------- crates/heph-tui/src/ui.rs | 7 ++----- docs/changelog.d/+sync-age-seconds.feature.md | 1 + 3 files changed, 13 insertions(+), 12 deletions(-) create mode 100644 docs/changelog.d/+sync-age-seconds.feature.md diff --git a/crates/heph-tui/src/fmt.rs b/crates/heph-tui/src/fmt.rs index 2383a7e..8c49ac9 100644 --- a/crates/heph-tui/src/fmt.rs +++ b/crates/heph-tui/src/fmt.rs @@ -30,13 +30,15 @@ pub fn now_ms() -> i64 { Local::now().timestamp_millis() } -/// A compact "how long ago" for the sync indicator: `just now` under a minute, -/// then `Nm` / `Nh` / `Nd`. Clamped at zero so a little clock skew never shows a -/// negative age. +/// A compact "how long ago" for the sync indicator: `Ns` under a minute, then +/// `Nm` / `Nh` / `Nd`. Second-granularity under a minute makes the chip a visible +/// heartbeat (the sync loop runs every 30s) and surfaces a missed beat as the age +/// climbing, rather than hiding under a flat "just now". Clamped at zero so a +/// little clock skew never shows a negative age. pub fn fmt_age(now_ms: i64, then_ms: i64) -> String { let secs = (now_ms - then_ms).max(0) / 1000; if secs < 60 { - "just now".into() + format!("{secs}s") } else if secs < 3_600 { format!("{}m", secs / 60) } else if secs < 86_400 { @@ -126,13 +128,14 @@ mod tests { #[test] fn age_is_compact_and_clamped() { let now = 1_000_000_000_000; - assert_eq!(fmt_age(now, now), "just now"); - assert_eq!(fmt_age(now, now - 30_000), "just now"); + assert_eq!(fmt_age(now, now), "0s"); + assert_eq!(fmt_age(now, now - 30_000), "30s"); + assert_eq!(fmt_age(now, now - 59_000), "59s"); assert_eq!(fmt_age(now, now - 5 * 60_000), "5m"); assert_eq!(fmt_age(now, now - 3 * 3_600_000), "3h"); assert_eq!(fmt_age(now, now - 2 * 86_400_000), "2d"); // Clock skew (then in the future) never shows a negative age. - assert_eq!(fmt_age(now, now + 10_000), "just now"); + assert_eq!(fmt_age(now, now + 10_000), "0s"); } #[test] diff --git a/crates/heph-tui/src/ui.rs b/crates/heph-tui/src/ui.rs index b457818..6e15453 100644 --- a/crates/heph-tui/src/ui.rs +++ b/crates/heph-tui/src/ui.rs @@ -661,10 +661,7 @@ mod tests { last_success_ms: Some(NOW), ..Default::default() }; - assert_eq!( - render(&spoke(h.clone(), 1), NOW), - "⟳ just now ⚠ 1 conflict" - ); - assert_eq!(render(&spoke(h, 3), NOW), "⟳ just now ⚠ 3 conflicts"); + assert_eq!(render(&spoke(h.clone(), 1), NOW), "⟳ 0s ⚠ 1 conflict"); + assert_eq!(render(&spoke(h, 3), NOW), "⟳ 0s ⚠ 3 conflicts"); } } diff --git a/docs/changelog.d/+sync-age-seconds.feature.md b/docs/changelog.d/+sync-age-seconds.feature.md new file mode 100644 index 0000000..cf453c2 --- /dev/null +++ b/docs/changelog.d/+sync-age-seconds.feature.md @@ -0,0 +1 @@ +heph-tui's sync indicator now shows the last-sync age in seconds under a minute (`⟳ 26s`) instead of a flat `just now`, so the chip reads as a live heartbeat and a missed sync (the loop runs every 30s) shows up as the age climbing. From 626c796e6cdcf8595086457971212a341dd519ea Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Mon, 8 Jun 2026 13:25:15 -0700 Subject: [PATCH 11/18] feat(heph): bake daemon mode/hub/oidc/self-update-interval into the service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `heph daemon start`/`restart` previously hardcoded `hephd --mode local` and only wired the bare `--self-update` bool — the poll interval and all spoke/hub sync config (`--hub-url`, `--http-addr`, `--oidc-*`) could not be set on the managed service without hand-editing the plist/unit (which a later start/restart would clobber). Generate the hephd arg vector from a DaemonConfig and add the corresponding `heph daemon start/restart` flags: --mode, --hub-url, --http-addr, --oidc-issuer, --oidc-audience, --oidc-client-id, and --self-update-interval-secs. Regenerating now reads the existing service file and preserves any flags not passed (start as well as restart), so a bare invocation never silently drops baked config. Closes the "pass through --self-update-interval-secs" and "bake hub/spoke config into the generated service" backlog tasks. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/heph/src/service.rs | 492 +++++++++++++++--- .../daemon-self-update-interval.feature.md | 1 + docs/how-to/run-the-daemon.md | 45 +- docs/how-to/self-update.md | 14 +- 4 files changed, 473 insertions(+), 79 deletions(-) create mode 100644 docs/changelog.d/daemon-self-update-interval.feature.md diff --git a/crates/heph/src/service.rs b/crates/heph/src/service.rs index 7b15865..1c90924 100644 --- a/crates/heph/src/service.rs +++ b/crates/heph/src/service.rs @@ -4,12 +4,18 @@ //! be shared by the CLI, TUI, and `heph.nvim` without any one of them owning its //! lifecycle. macOS uses a launchd **LaunchAgent**, Linux a **systemd user //! service**. All verbs are idempotent. +//! +//! The service generator bakes the daemon's runtime config — mode, sync hub, +//! and OIDC — into the unit so a spoke/hub can run under the managed service +//! instead of a hand-written plist/unit. Regenerating (`start`/`restart`) +//! **preserves any config already baked into the on-disk file**, so a bare +//! invocation never silently drops flags a previous one set. use std::path::{Path, PathBuf}; use std::process::Command; use anyhow::{bail, Context, Result}; -use clap::Subcommand; +use clap::{Args, Subcommand}; use hephd::{default_db_path, default_socket_path}; @@ -19,28 +25,106 @@ const LABEL: &str = "org.hephaestus.hephd"; #[derive(Subcommand, Debug)] pub enum DaemonAction { /// Install (if needed) and start the daemon service. - Start { - /// Generate a service that runs with opt-in self-update enabled - /// (default off). The service gets a PATH that can find cargo. - #[arg(long)] - self_update: bool, - }, + Start(ServiceArgs), /// Stop the daemon now (it may restart at next login; use `uninstall` to /// stop it for good). Stop, /// Restart the daemon — run this after upgrading the binary. Preserves the - /// existing self-update setting unless `--self-update` re-enables it. - Restart { - /// Force self-update on when regenerating the service definition. - #[arg(long)] - self_update: bool, - }, + /// config already baked into the service file (pass flags to add/override). + Restart(ServiceArgs), /// Show whether the service is installed and running. Status, /// Stop and remove the service entirely. Uninstall, } +/// Config flags baked into the generated service, shared by `start`/`restart`. +/// Anything omitted falls back to what the on-disk service file already has, so +/// regenerating is non-destructive. +#[derive(Args, Debug)] +pub struct ServiceArgs { + /// Runtime mode baked into the service (default `local`). Use `server` for a + /// sync hub, `client` for an online-only proxy. + #[arg(long, value_parser = ["local", "server", "client"])] + mode: Option, + /// Hub to background-sync this replica's op-log with (makes it a spoke) — + /// bakes `--hub-url`. + #[arg(long)] + hub_url: Option, + /// Hub HTTP listen address (server mode) — bakes `--http-addr`. + #[arg(long)] + http_addr: Option, + /// OIDC issuer used to verify (server) or obtain (spoke) hub tokens — bakes + /// `--oidc-issuer`. + #[arg(long)] + oidc_issuer: Option, + /// OIDC audience hub tokens must carry (server mode) — bakes + /// `--oidc-audience`. + #[arg(long)] + oidc_audience: Option, + /// OIDC client id this device authenticates as (spoke) — bakes + /// `--oidc-client-id`. + #[arg(long)] + oidc_client_id: Option, + /// Generate a service that runs with opt-in self-update enabled (default + /// off). The service gets a PATH that can find cargo. + #[arg(long)] + self_update: bool, + /// Override the self-update poll interval, in seconds (default: 6h). Only + /// meaningful with --self-update. + #[arg(long)] + self_update_interval_secs: Option, +} + +/// The hephd flags the service generator bakes beyond the fixed `--db`/`--socket`. +#[derive(Default, Clone, PartialEq, Debug)] +struct DaemonConfig { + mode: Option, + hub_url: Option, + http_addr: Option, + oidc_issuer: Option, + oidc_audience: Option, + oidc_client_id: Option, + self_update: bool, + self_update_interval_secs: Option, +} + +impl ServiceArgs { + fn to_config(&self) -> DaemonConfig { + DaemonConfig { + mode: self.mode.clone(), + hub_url: self.hub_url.clone(), + http_addr: self.http_addr.clone(), + oidc_issuer: self.oidc_issuer.clone(), + oidc_audience: self.oidc_audience.clone(), + oidc_client_id: self.oidc_client_id.clone(), + self_update: self.self_update, + self_update_interval_secs: self.self_update_interval_secs, + } + } +} + +impl DaemonConfig { + /// CLI-provided values win; anything omitted falls back to `base` (the flags + /// already baked into the on-disk service file), so regenerating the service + /// never drops config a previous invocation set. `self_update` is sticky — + /// it stays on if either the CLI or the existing file enabled it. + fn fill_from(self, base: DaemonConfig) -> DaemonConfig { + DaemonConfig { + mode: self.mode.or(base.mode), + hub_url: self.hub_url.or(base.hub_url), + http_addr: self.http_addr.or(base.http_addr), + oidc_issuer: self.oidc_issuer.or(base.oidc_issuer), + oidc_audience: self.oidc_audience.or(base.oidc_audience), + oidc_client_id: self.oidc_client_id.or(base.oidc_client_id), + self_update: self.self_update || base.self_update, + self_update_interval_secs: self + .self_update_interval_secs + .or(base.self_update_interval_secs), + } + } +} + /// Resolved locations the service definition needs. struct Paths { hephd: PathBuf, @@ -114,6 +198,105 @@ pub fn run(action: &DaemonAction) -> Result<()> { } } +// -------------------------------------------------------------------------- +// hephd argument vector (pure — shared by both renderers and unit-tested) +// -------------------------------------------------------------------------- + +/// The full `hephd …` argument vector the service runs, given the resolved paths +/// and baked config. `--mode` defaults to `local`; the optional flags appear in +/// a stable order so regenerating an unchanged config produces an identical file. +fn hephd_args(hephd: &Path, db: &Path, socket: &Path, cfg: &DaemonConfig) -> Vec { + let mut a = vec![ + hephd.to_string_lossy().into_owned(), + "--mode".into(), + cfg.mode.clone().unwrap_or_else(|| "local".into()), + "--db".into(), + db.to_string_lossy().into_owned(), + "--socket".into(), + socket.to_string_lossy().into_owned(), + ]; + push_opt(&mut a, "--hub-url", &cfg.hub_url); + push_opt(&mut a, "--http-addr", &cfg.http_addr); + push_opt(&mut a, "--oidc-issuer", &cfg.oidc_issuer); + push_opt(&mut a, "--oidc-audience", &cfg.oidc_audience); + push_opt(&mut a, "--oidc-client-id", &cfg.oidc_client_id); + // Interval is only meaningful with --self-update, so it's nested under it. + if cfg.self_update { + a.push("--self-update".into()); + if let Some(secs) = cfg.self_update_interval_secs { + a.push("--self-update-interval-secs".into()); + a.push(secs.to_string()); + } + } + a +} + +fn push_opt(args: &mut Vec, flag: &str, val: &Option) { + if let Some(v) = val { + args.push(flag.to_string()); + args.push(v.clone()); + } +} + +/// Parse a `hephd` argument vector back into a [`DaemonConfig`] — the inverse of +/// [`hephd_args`], used to recover config already baked into an on-disk service +/// file. Unrecognized args (the binary path, `--db`, `--socket`) are ignored. +fn parse_hephd_args(args: &[String]) -> DaemonConfig { + let mut c = DaemonConfig::default(); + let mut i = 0; + while i < args.len() { + let next = || args.get(i + 1).cloned(); + match args[i].as_str() { + "--mode" => { + c.mode = next(); + i += 2; + } + "--hub-url" => { + c.hub_url = next(); + i += 2; + } + "--http-addr" => { + c.http_addr = next(); + i += 2; + } + "--oidc-issuer" => { + c.oidc_issuer = next(); + i += 2; + } + "--oidc-audience" => { + c.oidc_audience = next(); + i += 2; + } + "--oidc-client-id" => { + c.oidc_client_id = next(); + i += 2; + } + "--self-update" => { + c.self_update = true; + i += 1; + } + "--self-update-interval-secs" => { + c.self_update_interval_secs = next().and_then(|s| s.parse().ok()); + i += 2; + } + _ => i += 1, + } + } + c +} + +/// Recover the config baked into an existing service file (empty if absent). +fn existing_config(path: &Path, mgr: &Manager) -> DaemonConfig { + let Ok(s) = std::fs::read_to_string(path) else { + return DaemonConfig::default(); + }; + let args = match mgr { + Manager::Launchd => launchd_program_args(&s), + Manager::Systemd => systemd_exec_args(&s), + }; + parse_hephd_args(&args) +} + // -------------------------------------------------------------------------- // Rendering (pure — unit-tested) // -------------------------------------------------------------------------- @@ -124,17 +307,22 @@ fn xml_escape(s: &str) -> String { .replace('>', ">") } -fn launchd_plist(hephd: &Path, db: &Path, socket: &Path, log: &Path, self_update: bool) -> String { - let arg = |p: &Path| xml_escape(&p.to_string_lossy()); - // Opt-in self-update: pass the flag, and give the service a PATH/HOME that - // can find cargo + the toolchain (a LaunchAgent's default env can't), since - // the apply path shells out to `cargo install`. - let self_update_arg = if self_update { - "\n --self-update".to_string() - } else { - String::new() - }; - let cargo_env = if self_update { +fn xml_unescape(s: &str) -> String { + s.replace("<", "<") + .replace(">", ">") + .replace("&", "&") +} + +fn launchd_plist(hephd: &Path, db: &Path, socket: &Path, log: &Path, cfg: &DaemonConfig) -> String { + let args_xml = hephd_args(hephd, db, socket, cfg) + .iter() + .map(|a| format!(" {}", xml_escape(a))) + .collect::>() + .join("\n"); + // Opt-in self-update needs a PATH/HOME that can find cargo + the toolchain + // (a LaunchAgent's default env can't), since the apply path shells out to + // `cargo install`. + let cargo_env = if cfg.self_update { let (path, home) = cargo_env(); format!( "\n PATH\n {}\n HOME\n {}", @@ -153,13 +341,7 @@ fn launchd_plist(hephd: &Path, db: &Path, socket: &Path, log: &Path, self_update {label} ProgramArguments - {hephd} - --mode - local - --db - {db} - --socket - {socket}{self_update_arg} +{args_xml} RunAtLoad @@ -181,10 +363,7 @@ fn launchd_plist(hephd: &Path, db: &Path, socket: &Path, log: &Path, self_update "#, label = LABEL, - hephd = arg(hephd), - db = arg(db), - socket = arg(socket), - log = arg(log), + log = xml_escape(&log.to_string_lossy()), ) } @@ -199,20 +378,34 @@ fn cargo_env() -> (String, String) { (path, home) } -/// Whether an already-installed service file opted into self-update — so -/// `restart` (which regenerates the file) preserves the setting instead of -/// silently turning it off. -fn file_opts_into_self_update(path: &Path) -> bool { - std::fs::read_to_string(path) - .map(|s| s.contains("--self-update")) - .unwrap_or(false) +/// Extract the `ProgramArguments` strings from an existing launchd plist. +fn launchd_program_args(plist: &str) -> Vec { + let Some(k) = plist.find("ProgramArguments") else { + return vec![]; + }; + let rest = &plist[k..]; + let (Some(start), Some(end)) = (rest.find(""), rest.find("")) else { + return vec![]; + }; + let block = &rest[start..end]; + let mut out = vec![]; + let mut cur = block; + while let Some(o) = cur.find("") { + let after = &cur[o + "".len()..]; + let Some(c) = after.find("") else { + break; + }; + out.push(xml_unescape(&after[..c])); + cur = &after[c + "".len()..]; + } + out } -fn systemd_unit(hephd: &Path, db: &Path, socket: &Path, self_update: bool) -> String { - // Opt-in self-update: pass the flag and give the unit a PATH/HOME that can - // find cargo + the toolchain, since the apply path runs `cargo install`. - let su_arg = if self_update { " --self-update" } else { "" }; - let cargo_env = if self_update { +fn systemd_unit(hephd: &Path, db: &Path, socket: &Path, cfg: &DaemonConfig) -> String { + let exec = hephd_args(hephd, db, socket, cfg).join(" "); + // Opt-in self-update needs a PATH/HOME that can find cargo + the toolchain, + // since the apply path runs `cargo install`. + let cargo_env = if cfg.self_update { let (path, home) = cargo_env(); format!("Environment=PATH={path}\nEnvironment=HOME={home}\n") } else { @@ -224,19 +417,24 @@ fn systemd_unit(hephd: &Path, db: &Path, socket: &Path, self_update: bool) -> St After=default.target\n\ \n\ [Service]\n\ - ExecStart={hephd} --mode local --db {db} --socket {socket}{su_arg}\n\ + ExecStart={exec}\n\ {cargo_env}\ Restart=always\n\ RestartSec=1\n\ \n\ [Install]\n\ WantedBy=default.target\n", - hephd = hephd.display(), - db = db.display(), - socket = socket.display(), ) } +/// Extract the `ExecStart=` argument vector from an existing systemd unit. +fn systemd_exec_args(unit: &str) -> Vec { + unit.lines() + .find_map(|l| l.strip_prefix("ExecStart=")) + .map(|rest| rest.split_whitespace().map(str::to_string).collect()) + .unwrap_or_default() +} + // -------------------------------------------------------------------------- // Shared helpers // -------------------------------------------------------------------------- @@ -303,10 +501,13 @@ fn launchd(action: &DaemonAction, p: &Paths) -> Result<()> { let target = format!("gui/{uid}/{LABEL}"); match action { - DaemonAction::Start { self_update } => { + DaemonAction::Start(args) => { + let cfg = args + .to_config() + .fill_from(existing_config(&plist, &Manager::Launchd)); write_if_changed( &plist, - &launchd_plist(&p.hephd, &p.db, &p.socket, &p.log, *self_update), + &launchd_plist(&p.hephd, &p.db, &p.socket, &p.log, &cfg), )?; if launchd_loaded(&target) { println!("heph daemon already running ({LABEL})."); @@ -322,11 +523,13 @@ fn launchd(action: &DaemonAction, p: &Paths) -> Result<()> { let (_ok, _err) = run_cmd("launchctl", &["bootout", &target])?; println!("heph daemon stopped (still installed; `uninstall` to remove)."); } - DaemonAction::Restart { self_update } => { - let su = *self_update || file_opts_into_self_update(&plist); + DaemonAction::Restart(args) => { + let cfg = args + .to_config() + .fill_from(existing_config(&plist, &Manager::Launchd)); write_if_changed( &plist, - &launchd_plist(&p.hephd, &p.db, &p.socket, &p.log, su), + &launchd_plist(&p.hephd, &p.db, &p.socket, &p.log, &cfg), )?; let _ = run_cmd("launchctl", &["bootout", &target])?; let (ok, err) = run_cmd("launchctl", &["bootstrap", &domain, &plist_str(&plist)?])?; @@ -380,11 +583,11 @@ fn sc(args: &[&str]) -> Result<(bool, String)> { fn systemd(action: &DaemonAction, p: &Paths) -> Result<()> { let unit = systemd_unit_path()?; match action { - DaemonAction::Start { self_update } => { - write_if_changed( - &unit, - &systemd_unit(&p.hephd, &p.db, &p.socket, *self_update), - )?; + DaemonAction::Start(args) => { + let cfg = args + .to_config() + .fill_from(existing_config(&unit, &Manager::Systemd)); + write_if_changed(&unit, &systemd_unit(&p.hephd, &p.db, &p.socket, &cfg))?; sc(&["daemon-reload"])?; let (ok, err) = sc(&["enable", "--now", UNIT])?; if !ok { @@ -396,9 +599,11 @@ fn systemd(action: &DaemonAction, p: &Paths) -> Result<()> { sc(&["stop", UNIT])?; println!("heph daemon stopped (still enabled; `uninstall` to remove)."); } - DaemonAction::Restart { self_update } => { - let su = *self_update || file_opts_into_self_update(&unit); - write_if_changed(&unit, &systemd_unit(&p.hephd, &p.db, &p.socket, su))?; + DaemonAction::Restart(args) => { + let cfg = args + .to_config() + .fill_from(existing_config(&unit, &Manager::Systemd)); + write_if_changed(&unit, &systemd_unit(&p.hephd, &p.db, &p.socket, &cfg))?; sc(&["daemon-reload"])?; let (ok, err) = sc(&["restart", UNIT])?; if !ok { @@ -440,6 +645,18 @@ fn print_status(installed: bool, running: bool, p: &Paths, service_file: &Path) mod tests { use super::*; + fn spoke_cfg() -> DaemonConfig { + DaemonConfig { + mode: Some("local".into()), + hub_url: Some("http://hub.example:8787".into()), + oidc_issuer: Some("https://idp.example/o/heph/".into()), + oidc_client_id: Some("heph".into()), + self_update: true, + self_update_interval_secs: Some(600), + ..Default::default() + } + } + #[test] fn launchd_plist_has_label_args_and_paths() { let plist = launchd_plist( @@ -447,19 +664,21 @@ mod tests { Path::new("/home/e/.local/share/heph/heph.db"), Path::new("/tmp/heph/hephd.sock"), Path::new("/home/e/.local/share/heph/hephd.log"), - false, + &DaemonConfig::default(), ); assert!(plist.contains("org.hephaestus.hephd")); assert!(plist.contains("/usr/local/bin/hephd")); assert!(plist.contains("--mode")); + assert!(plist.contains("local")); assert!(plist.contains("/home/e/.local/share/heph/heph.db")); assert!(plist.contains("/tmp/heph/hephd.sock")); assert!(plist.contains("RunAtLoad")); assert!(plist.contains("KeepAlive")); assert!(plist.contains("hephd.log")); - // Default (no self-update): no flag, no cargo PATH baked in. + // Default (no self-update, no spoke/hub config): none of those flags. assert!(!plist.contains("--self-update")); assert!(!plist.contains(".cargo/bin")); + assert!(!plist.contains("--hub-url")); } #[test] @@ -469,12 +688,64 @@ mod tests { Path::new("/db"), Path::new("/sock"), Path::new("/log"), - true, + &DaemonConfig { + self_update: true, + ..Default::default() + }, ); assert!(plist.contains("--self-update")); assert!(plist.contains("PATH")); assert!(plist.contains(".cargo/bin")); assert!(plist.contains("HOME")); + // No interval given → no interval flag. + assert!(!plist.contains("--self-update-interval-secs")); + } + + #[test] + fn launchd_plist_self_update_interval_is_baked_under_self_update() { + let with = launchd_plist( + Path::new("/hephd"), + Path::new("/db"), + Path::new("/sock"), + Path::new("/log"), + &DaemonConfig { + self_update: true, + self_update_interval_secs: Some(3600), + ..Default::default() + }, + ); + assert!(with.contains("--self-update-interval-secs")); + assert!(with.contains("3600")); + // Interval is meaningless without --self-update, so it's not emitted. + let without = launchd_plist( + Path::new("/hephd"), + Path::new("/db"), + Path::new("/sock"), + Path::new("/log"), + &DaemonConfig { + self_update: false, + self_update_interval_secs: Some(3600), + ..Default::default() + }, + ); + assert!(!without.contains("--self-update-interval-secs")); + } + + #[test] + fn launchd_plist_bakes_spoke_config() { + let plist = launchd_plist( + Path::new("/hephd"), + Path::new("/db"), + Path::new("/sock"), + Path::new("/log"), + &spoke_cfg(), + ); + assert!(plist.contains("--hub-url")); + assert!(plist.contains("http://hub.example:8787")); + assert!(plist.contains("--oidc-issuer")); + assert!(plist.contains("https://idp.example/o/heph/")); + assert!(plist.contains("--oidc-client-id")); + assert!(plist.contains("heph")); } #[test] @@ -483,7 +754,7 @@ mod tests { Path::new("/usr/local/bin/hephd"), Path::new("/home/e/.local/share/heph/heph.db"), Path::new("/run/user/1000/heph/hephd.sock"), - false, + &DaemonConfig::default(), ); assert!(unit.contains( "ExecStart=/usr/local/bin/hephd --mode local \ @@ -507,17 +778,96 @@ mod tests { Path::new("/usr/local/bin/hephd"), Path::new("/db"), Path::new("/sock"), - true, + &DaemonConfig { + self_update: true, + self_update_interval_secs: Some(3600), + ..Default::default() + }, ); - assert!(unit.contains("--self-update")); + assert!(unit.contains("--self-update --self-update-interval-secs 3600")); assert!(unit.contains("Environment=PATH=")); assert!(unit.contains(".cargo/bin")); assert!(unit.contains("Environment=HOME=")); } #[test] - fn xml_escape_escapes_markup() { + fn systemd_unit_bakes_hub_config() { + let unit = systemd_unit( + Path::new("/hephd"), + Path::new("/db"), + Path::new("/sock"), + &DaemonConfig { + mode: Some("server".into()), + http_addr: Some("0.0.0.0:8787".into()), + oidc_issuer: Some("https://idp.example/o/heph/".into()), + oidc_audience: Some("heph".into()), + ..Default::default() + }, + ); + assert!(unit.contains("--mode server")); + assert!(unit.contains("--http-addr 0.0.0.0:8787")); + assert!(unit.contains("--oidc-issuer https://idp.example/o/heph/")); + assert!(unit.contains("--oidc-audience heph")); + } + + #[test] + fn launchd_config_round_trips_through_the_plist() { + let cfg = spoke_cfg(); + let plist = launchd_plist( + Path::new("/hephd"), + Path::new("/db"), + Path::new("/sock"), + Path::new("/log"), + &cfg, + ); + let parsed = parse_hephd_args(&launchd_program_args(&plist)); + assert_eq!(parsed, cfg); + } + + #[test] + fn systemd_config_round_trips_through_the_unit() { + let cfg = DaemonConfig { + mode: Some("server".into()), + http_addr: Some("0.0.0.0:8787".into()), + oidc_issuer: Some("https://idp.example/o/heph/".into()), + oidc_audience: Some("heph".into()), + self_update: true, + self_update_interval_secs: Some(600), + ..Default::default() + }; + let unit = systemd_unit( + Path::new("/hephd"), + Path::new("/db"), + Path::new("/sock"), + &cfg, + ); + let parsed = parse_hephd_args(&systemd_exec_args(&unit)); + assert_eq!(parsed, cfg); + } + + #[test] + fn fill_from_preserves_existing_and_lets_cli_override() { + let existing = spoke_cfg(); + // A bare invocation (no flags) preserves everything baked in the file. + assert_eq!( + DaemonConfig::default().fill_from(existing.clone()), + existing + ); + // A CLI-provided value overrides; self_update stays sticky. + let overridden = DaemonConfig { + self_update_interval_secs: Some(60), + ..Default::default() + } + .fill_from(existing.clone()); + assert_eq!(overridden.self_update_interval_secs, Some(60)); + assert_eq!(overridden.hub_url, existing.hub_url); + assert!(overridden.self_update); + } + + #[test] + fn xml_escape_round_trips() { assert_eq!(xml_escape("a & b < c > d"), "a & b < c > d"); + assert_eq!(xml_unescape("a & b < c > d"), "a & b < c > d"); } #[test] diff --git a/docs/changelog.d/daemon-self-update-interval.feature.md b/docs/changelog.d/daemon-self-update-interval.feature.md new file mode 100644 index 0000000..b5ec9b8 --- /dev/null +++ b/docs/changelog.d/daemon-self-update-interval.feature.md @@ -0,0 +1 @@ +`heph daemon start`/`restart` can now bake the daemon's full runtime config into the managed service — `--mode`, `--hub-url`, `--http-addr`, `--oidc-issuer`/`--oidc-audience`/`--oidc-client-id`, and `--self-update-interval-secs` (previously only the bare `--self-update` bool was wired). Regenerating preserves whatever is already baked into the on-disk plist/unit, so a bare `start`/`restart` no longer silently drops spoke/hub or self-update config. diff --git a/docs/how-to/run-the-daemon.md b/docs/how-to/run-the-daemon.md index 2b00dff..cb9e56d 100644 --- a/docs/how-to/run-the-daemon.md +++ b/docs/how-to/run-the-daemon.md @@ -36,14 +36,47 @@ when it's already stopped is fine. > exits cleanly to hand off to the new binary) wouldn't come back on Linux. Run > `heph daemon restart` once (it regenerates the unit) to pick up `Restart=always`. -Either way it runs `hephd --mode local` against the default store +By default it runs `hephd --mode local` against the default store (`~/.local/share/heph/heph.db`) and socket, with logs at -`~/.local/share/heph/hephd.log`. +`~/.local/share/heph/hephd.log`. Pass flags to `start`/`restart` to bake a +different runtime config into the service (see below). > **`stop` vs `uninstall`:** `stop` halts the daemon now, but the service is > still installed, so on macOS it starts again at next login. Use `uninstall` > to stop it persistently. +## Baking sync config (spoke / hub) + +By default the service runs a standalone `--mode local` daemon. To make the +managed service a **spoke** (background-syncs to a hub) or a **hub** (`--mode +server`), pass the corresponding `hephd` flags to `start` (or `restart`) — they +get baked into the generated plist/unit: + +```bash +# Spoke: sync to a hub, authenticating with OIDC +heph daemon start \ + --hub-url http://hub.example:8787 \ + --oidc-issuer https://idp.example/application/o/heph/ \ + --oidc-client-id heph + +# Hub: expose the authenticated sync endpoint +heph daemon start --mode server \ + --http-addr 0.0.0.0:8787 \ + --oidc-issuer https://idp.example/application/o/heph/ \ + --oidc-audience heph +``` + +Bakeable flags: `--mode`, `--hub-url`, `--http-addr`, `--oidc-issuer`, +`--oidc-audience`, `--oidc-client-id`, `--self-update`, +`--self-update-interval-secs`. **Regenerating preserves what's already baked +in** — `start`/`restart` read the existing service file and carry over any flags +you don't pass, so a bare `heph daemon restart` never drops your spoke/hub or +self-update config. Pass a flag again to add or override it. + +> Spoke sync is HTTP-only today (`hephd`'s sync client doesn't speak HTTPS) — a +> `--hub-url` over the tailnet or behind a TLS-terminating proxy is the usual +> setup. + ## After upgrading When you rebuild/reinstall (`cargo install … --force`), the running daemon is @@ -59,9 +92,11 @@ heph daemon restart service that polls the forge for newer releases and, when one appears, rebuilds via `cargo install` (anonymous HTTPS clone of the public repo — no credentials) and restarts onto the new binary. It is **off by default**; the generated -service also gets a `PATH` that can find cargo. `heph daemon restart` preserves -the setting (pass `--self-update` again to turn it on later). Requires the Rust -toolchain (`cargo`) installed for the service user. +service also gets a `PATH` that can find cargo. Override the 6h poll cadence with +`--self-update-interval-secs `. Both `start` and `restart` preserve an +already-baked self-update setting (and its interval), so a bare invocation won't +silently disable it — pass `--self-update` again only to turn it on later. +Requires the Rust toolchain (`cargo`) installed for the service user. ## Development isolation diff --git a/docs/how-to/self-update.md b/docs/how-to/self-update.md index d4dda1f..e7d0627 100644 --- a/docs/how-to/self-update.md +++ b/docs/how-to/self-update.md @@ -20,9 +20,17 @@ heph daemon start --self-update ``` That generates a launchd/systemd service that runs `hephd --self-update` and -gives it a `PATH` that can find `cargo`. `heph daemon restart` preserves the -setting (pass `--self-update` again to turn it on later). To run the daemon -directly instead: +gives it a `PATH` that can find `cargo`. Override the 6h poll cadence with +`--self-update-interval-secs `: + +```bash +heph daemon start --self-update # default: poll every 6h +heph daemon start --self-update --self-update-interval-secs 3600 +``` + +Both `start` and `restart` preserve an already-baked setting (the flag and its +interval), so a bare invocation won't silently disable it — pass `--self-update` +again only to turn it on later. To run the daemon directly instead: ```bash hephd --self-update # default: poll every 6h From f6b27414a8b46834a84c3b5735cc7049a4ab39b8 Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Mon, 8 Jun 2026 13:38:47 -0700 Subject: [PATCH 12/18] fix(heph): make macOS `heph daemon restart` race-free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `restart` bootstrapped immediately after `bootout`, but `launchctl bootout` is asynchronous: launchd may still be killing/reaping the job and removing its label when the command returns. Bootstrapping into that transitional domain fails with a generic `5: Input/output error`, intermittently — the odds depend on how fast hephd (sync client + SQLite + a heph-quickadd child) shuts down. - Wait for the label to actually clear (poll `launchctl print`, bounded) before re-bootstrapping, and retry the bootstrap to cover the residual settle window. - When the plist is unchanged (the common binary-upgrade restart), use `launchctl kickstart -k` to restart the loaded job atomically — no bootout/bootstrap, no race. The full reload path is reserved for genuine config changes, where launchd must re-read the plist. Start's bootstrap shares the same retry helper. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/heph/src/service.rs | 71 ++++++++++++++++--- .../changelog.d/daemon-restart-race.bugfix.md | 1 + 2 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 docs/changelog.d/daemon-restart-race.bugfix.md diff --git a/crates/heph/src/service.rs b/crates/heph/src/service.rs index 1c90924..0b8928b 100644 --- a/crates/heph/src/service.rs +++ b/crates/heph/src/service.rs @@ -13,6 +13,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; +use std::time::{Duration, Instant}; use anyhow::{bail, Context, Result}; use clap::{Args, Subcommand}; @@ -494,6 +495,51 @@ fn launchd_loaded(domain_target: &str) -> bool { .unwrap_or(false) } +/// Block until `target` is no longer loaded, up to `timeout`. `launchctl bootout` +/// is asynchronous in effect — it requests teardown and returns, but launchd may +/// still be killing/reaping the job and removing its label from the domain. +/// Bootstrapping while the label lingers fails with a generic `5: Input/output +/// error`, so we wait for the label to actually disappear before re-bootstrapping. +fn wait_until_unloaded(target: &str, timeout: Duration) { + let start = Instant::now(); + while launchd_loaded(target) { + if start.elapsed() >= timeout { + break; // fall through; bootstrap's own retry covers the residual window + } + std::thread::sleep(Duration::from_millis(100)); + } +} + +/// Bootstrap the service, retrying briefly. Even once the old instance is gone, +/// launchd can momentarily return EIO while the domain settles, so a couple of +/// short retries make `start`/`restart` reliable instead of intermittently failing. +fn launchd_bootstrap(domain: &str, plist: &str) -> Result<()> { + let mut last = String::new(); + for attempt in 0..5 { + if attempt > 0 { + std::thread::sleep(Duration::from_millis(200)); + } + let (ok, err) = run_cmd("launchctl", &["bootstrap", domain, plist])?; + if ok { + return Ok(()); + } + last = err; + } + bail!("launchctl bootstrap failed: {}", last.trim()); +} + +/// Restart an already-loaded job in place (kills it, then launchd's KeepAlive — +/// `-k` forces the kill). This restarts the *loaded* job definition, so it does +/// not pick up an edited plist — callers use it only when the on-disk plist is +/// unchanged, where it sidesteps the bootout→bootstrap race entirely. +fn launchd_kickstart(target: &str) -> Result<()> { + let (ok, err) = run_cmd("launchctl", &["kickstart", "-k", target])?; + if !ok { + bail!("launchctl kickstart failed: {}", err.trim()); + } + Ok(()) +} + fn launchd(action: &DaemonAction, p: &Paths) -> Result<()> { let plist = launchd_plist_path()?; let uid = uid()?; @@ -512,10 +558,7 @@ fn launchd(action: &DaemonAction, p: &Paths) -> Result<()> { if launchd_loaded(&target) { println!("heph daemon already running ({LABEL})."); } else { - let (ok, err) = run_cmd("launchctl", &["bootstrap", &domain, &plist_str(&plist)?])?; - if !ok { - bail!("launchctl bootstrap failed: {}", err.trim()); - } + launchd_bootstrap(&domain, &plist_str(&plist)?)?; println!("heph daemon started ({LABEL})."); } } @@ -527,14 +570,24 @@ fn launchd(action: &DaemonAction, p: &Paths) -> Result<()> { let cfg = args .to_config() .fill_from(existing_config(&plist, &Manager::Launchd)); - write_if_changed( + let changed = write_if_changed( &plist, &launchd_plist(&p.hephd, &p.db, &p.socket, &p.log, &cfg), )?; - let _ = run_cmd("launchctl", &["bootout", &target])?; - let (ok, err) = run_cmd("launchctl", &["bootstrap", &domain, &plist_str(&plist)?])?; - if !ok { - bail!("launchctl bootstrap failed: {}", err.trim()); + if !launchd_loaded(&target) { + // Not currently loaded — nothing to tear down, just bring it up. + launchd_bootstrap(&domain, &plist_str(&plist)?)?; + } else if changed { + // The plist changed, so launchd must re-read it: a full reload is + // required. bootout is async, so wait for the label to clear + // before bootstrapping (and bootstrap retries the residual EIO). + let _ = run_cmd("launchctl", &["bootout", &target])?; + wait_until_unloaded(&target, Duration::from_secs(5)); + launchd_bootstrap(&domain, &plist_str(&plist)?)?; + } else { + // Same definition (e.g. binary upgraded in place) — restart the + // loaded job atomically, sidestepping the bootout→bootstrap race. + launchd_kickstart(&target)?; } println!("heph daemon restarted ({LABEL})."); } diff --git a/docs/changelog.d/daemon-restart-race.bugfix.md b/docs/changelog.d/daemon-restart-race.bugfix.md new file mode 100644 index 0000000..c13a257 --- /dev/null +++ b/docs/changelog.d/daemon-restart-race.bugfix.md @@ -0,0 +1 @@ +`heph daemon restart` on macOS no longer intermittently fails with `launchctl bootstrap failed: 5: Input/output error`. The old code bootstrapped immediately after `bootout`, racing launchd's asynchronous teardown; it now waits for the service to fully unload and retries the bootstrap. When the plist is unchanged (e.g. a plain binary upgrade) it uses `launchctl kickstart -k` to restart the loaded job atomically, sidestepping the bootout→bootstrap dance entirely. From e943a940f147a9cf0d1f930c8a5a51e1606424e6 Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Mon, 8 Jun 2026 14:06:08 -0700 Subject: [PATCH 13/18] feat(hephd,heph,heph-tui): distinguish IdP rejection from unreachable + actionable re-auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spoke OAuth path funneled every failure into one `AuthError::Provider` whose Display was hardcoded "identity provider unreachable". So a reachable IdP returning `400 invalid_grant` on a refresh was reported as "unreachable", misdirecting incident response toward the network when the fix is re-auth. The real refresh cause was also swallowed — `bearer()` logged it and returned None, so sync health only ever showed the downstream 401 on /sync/pull. Wording fix (auth.rs / oauth.rs): - Split AuthError into Unreachable (transport), Rejected (IdP returned an HTTP error — carries the RFC 6749 §5.2 error/error_description), and Other (keyring / malformed response, previously mislabeled too). - refresh()/discover()/start()/poll() classify transport vs status; refresh reads the OAuth error body on a non-2xx. - Hub-side token verify maps IdP-infra failures → 503, token failures → 401. Recovery UX (server.rs / heph / heph-tui): - bearer() returns Result; the sync paths record the real acquisition failure (with a re-login hint when it's a rejection) instead of a masked 401. - sync health's last_error carries the exact `heph auth login --hub-url … --issuer … --client-id …` command (keyed to the configured hub); sync.status also returns issuer/client_id + the command. - New `heph auth status` prints auth health and the re-login command. - heph-tui's auth chip points at it: `⚠ auth · heph auth status`. Closes the duplicate "misleading identity provider unreachable" tasks and the "actionable re-auth guidance" task. Also corrects a now-stale set-up-sync-hub gap note (daemon config baking landed in the prior PR). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/heph-tui/src/ui.rs | 6 +- crates/heph/src/main.rs | 76 +++++++++- crates/hephd/src/auth.rs | 84 ++++++++++- crates/hephd/src/oauth.rs | 54 +++---- crates/hephd/src/server.rs | 137 +++++++++++++++--- crates/hephd/src/sync.rs | 10 +- crates/hephd/tests/oauth.rs | 66 ++++++++- docs/changelog.d/auth-error-clarity.bugfix.md | 1 + .../changelog.d/auth-error-clarity.feature.md | 1 + docs/how-to/set-up-sync-hub.md | 32 +++- 10 files changed, 393 insertions(+), 74 deletions(-) create mode 100644 docs/changelog.d/auth-error-clarity.bugfix.md create mode 100644 docs/changelog.d/auth-error-clarity.feature.md diff --git a/crates/heph-tui/src/ui.rs b/crates/heph-tui/src/ui.rs index 6e15453..f6d2f37 100644 --- a/crates/heph-tui/src/ui.rs +++ b/crates/heph-tui/src/ui.rs @@ -570,7 +570,9 @@ fn sync_indicator(sync: &SyncStatus, now: i64) -> Vec> { let health = sync.health.clone().unwrap_or_default(); let mut spans = vec![if health.auth_failure { - Span::styled("⚠ auth", red) + // Point at the recovery command — `heph auth status` prints the exact + // `heph auth login …` to run (the full command is too long for the bar). + Span::styled("⚠ auth · heph auth status", red) } else if let Some(ts) = health.last_success_ms { Span::styled(format!("⟳ {}", fmt_age(now, ts)), dim) } else if health.last_error.is_some() { @@ -639,7 +641,7 @@ mod tests { }, 0, ); - assert_eq!(render(&auth, NOW), "⚠ auth"); + assert_eq!(render(&auth, NOW), "⚠ auth · heph auth status"); // Errored with no prior success → offline. let offline = spoke( diff --git a/crates/heph/src/main.rs b/crates/heph/src/main.rs index c327f1d..28d3b5e 100644 --- a/crates/heph/src/main.rs +++ b/crates/heph/src/main.rs @@ -344,7 +344,7 @@ enum ConflictAction { }, } -#[derive(Subcommand, Debug)] +#[derive(Subcommand, Debug, Clone)] enum AuthAction { /// Log in via the device-code flow; caches the bearer token for hub sync. Login { @@ -367,6 +367,9 @@ enum AuthAction { #[arg(long)] hub_url: String, }, + /// Show this spoke's auth health and, if re-auth is needed, the exact + /// `heph auth login` command to run. Queries the daemon. + Status, } /// Run the device-code flow (or clear a token) — no daemon needed. @@ -396,10 +399,63 @@ fn run_auth(action: AuthAction) -> Result<()> { KeyringTokenStore::new(hub_url.as_str()).clear()?; println!("Logged out of {hub_url}."); } + AuthAction::Status => unreachable!("auth status is handled via the daemon"), } Ok(()) } +/// Render `heph auth status` from a `sync.status` RPC response: hub/issuer/client +/// id, whether auth is healthy or needs re-login, and — when it does — the exact +/// command to run (built daemon-side, keyed under the right hub URL). +fn print_auth_status(status: &Value) { + let Some(hub) = status.get("hub_url").and_then(Value::as_str) else { + println!("This instance is standalone (no hub configured); auth does not apply."); + return; + }; + let auth = status.get("auth"); + let issuer = auth.and_then(|a| a.get("issuer")).and_then(Value::as_str); + let client_id = auth + .and_then(|a| a.get("client_id")) + .and_then(Value::as_str); + let health = status.get("health"); + let auth_failure = health + .and_then(|h| h.get("auth_failure")) + .and_then(Value::as_bool) + .unwrap_or(false); + let last_error = health + .and_then(|h| h.get("last_error")) + .and_then(Value::as_str); + let last_success = health + .and_then(|h| h.get("last_success_ms")) + .and_then(Value::as_i64); + + println!("hub : {hub}"); + if let Some(iss) = issuer { + println!("issuer : {iss}"); + } + if let Some(cid) = client_id { + println!("client id : {cid}"); + } + println!( + "auth : {}", + if auth_failure { + "FAILED — re-authentication required" + } else if last_success.is_some() { + "ok" + } else { + "unknown (no successful sync yet)" + } + ); + if let Some(err) = last_error { + println!("last error : {err}"); + } + if auth_failure { + if let Some(cmd) = status.get("reauth_command").and_then(Value::as_str) { + println!("\nTo re-authenticate, run:\n {cmd}"); + } + } +} + fn main() -> Result<()> { let cli = Cli::parse(); @@ -407,9 +463,13 @@ fn main() -> Result<()> { if let Command::Daemon { action } = &cli.command { return service::run(action); } - // `auth` runs locally (device-code flow + keyring); it needs no daemon. - if let Command::Auth { action } = cli.command { - return run_auth(action); + // `auth login`/`logout` run locally (device-code flow + keyring); they need + // no daemon. `auth status` reads live sync health, so it falls through to the + // connected path below. + if let Command::Auth { action } = &cli.command { + if !matches!(action, AuthAction::Status) { + return run_auth(action.clone()); + } } let socket = cli.socket.unwrap_or_else(default_socket_path); @@ -790,7 +850,13 @@ fn main() -> Result<()> { let n = result.as_u64().unwrap_or(0); println!("Rewrote legacy [[Name]] links to [[id]] in {n} node(s)."); } - Command::Auth { .. } => unreachable!("auth is handled before connecting"), + Command::Auth { + action: AuthAction::Status, + } => { + let result = client.call("sync.status", json!({}))?; + print_auth_status(&result); + } + Command::Auth { .. } => unreachable!("auth login/logout handled before connecting"), Command::Daemon { .. } => unreachable!("daemon is handled before connecting"), } Ok(()) diff --git a/crates/hephd/src/auth.rs b/crates/hephd/src/auth.rs index c601d90..6b80e95 100644 --- a/crates/hephd/src/auth.rs +++ b/crates/hephd/src/auth.rs @@ -38,9 +38,45 @@ pub enum AuthError { /// The token was present but failed validation. #[error("invalid token: {0}")] Invalid(String), - /// The identity provider could not be reached to fetch keys. + /// The identity provider could not be reached at all (DNS, TLS, connection + /// refused, timeout) — a transport failure, distinct from a rejection. #[error("identity provider unreachable: {0}")] - Provider(String), + Unreachable(String), + /// The identity provider *was* reached but returned an HTTP error response — + /// e.g. `400 invalid_grant` on a refresh, meaning the token was rejected + /// (expired/rotated/session-invalidated), not that the IdP was down. The + /// distinction matters: "unreachable" sends debugging toward the network; + /// this points at the token/authorization. + #[error("identity provider rejected the request: {0}")] + Rejected(String), + /// Some other failure in the auth path that is neither a transport failure + /// nor an HTTP rejection — a malformed/unparseable IdP response, or a local + /// credential-store (keyring) error. Kept distinct so neither is mislabeled + /// as "unreachable". + #[error("auth error: {0}")] + Other(String), +} + +impl AuthError { + /// Build a [`AuthError::Rejected`] from an HTTP status and the OAuth error + /// body (RFC 6749 §5.2), e.g. `HTTP 400 (invalid_grant): Token is expired`. + pub fn rejected(status: u16, error: Option<&str>, description: Option<&str>) -> AuthError { + let mut msg = format!("HTTP {status}"); + if let Some(e) = error.filter(|e| !e.is_empty()) { + msg.push_str(&format!(" ({e})")); + } + if let Some(d) = description.filter(|d| !d.is_empty()) { + msg.push_str(&format!(": {d}")); + } + AuthError::Rejected(msg) + } + + /// Whether this is an authorization-level rejection (the IdP refused the + /// grant) rather than a transport failure — i.e. re-authentication is the + /// likely fix, not network troubleshooting. + pub fn is_rejection(&self) -> bool { + matches!(self, AuthError::Rejected(_)) + } } /// Verifies a bearer token and returns its [`Claims`]. A trait so the hub can be @@ -92,16 +128,13 @@ impl OidcVerifier { .http .get(url) .call() - .map_err(|e| AuthError::Provider(e.to_string()))?; + .map_err(|e| AuthError::Unreachable(e.to_string()))?; if !resp.status().is_success() { - return Err(AuthError::Provider(format!( - "{url} returned {}", - resp.status() - ))); + return Err(AuthError::rejected(resp.status().as_u16(), None, None)); } resp.body_mut() .read_json() - .map_err(|e| AuthError::Provider(e.to_string())) + .map_err(|e| AuthError::Unreachable(e.to_string())) } /// Resolve the JWKS URI from the provider's discovery document. @@ -169,3 +202,38 @@ impl TokenVerifier for OidcVerifier { Some((&self.issuer, &self.audience)) } } + +#[cfg(test)] +mod tests { + use super::AuthError; + + #[test] + fn rejected_formats_status_error_and_description() { + let e = AuthError::rejected(400, Some("invalid_grant"), Some("Token is not active")); + assert!(e.is_rejection()); + assert_eq!( + e.to_string(), + "identity provider rejected the request: HTTP 400 (invalid_grant): Token is not active" + ); + } + + #[test] + fn rejected_omits_absent_or_empty_oauth_fields() { + // No OAuth body (e.g. a bare 503) → just the status. + assert_eq!( + AuthError::rejected(503, None, None).to_string(), + "identity provider rejected the request: HTTP 503" + ); + // Empty strings are treated as absent, not rendered as "()" / ": ". + assert_eq!( + AuthError::rejected(400, Some(""), Some("")).to_string(), + "identity provider rejected the request: HTTP 400" + ); + } + + #[test] + fn unreachable_is_not_a_rejection() { + assert!(!AuthError::Unreachable("connection refused".into()).is_rejection()); + assert!(!AuthError::Other("keyring locked".into()).is_rejection()); + } +} diff --git a/crates/hephd/src/oauth.rs b/crates/hephd/src/oauth.rs index 53ee5f0..4af704f 100644 --- a/crates/hephd/src/oauth.rs +++ b/crates/hephd/src/oauth.rs @@ -109,7 +109,7 @@ impl KeyringTokenStore { } }); keyring_core::Entry::new(&self.service, &self.account) - .map_err(|e| AuthError::Provider(e.to_string())) + .map_err(|e| AuthError::Other(e.to_string())) } } @@ -119,16 +119,16 @@ impl TokenStore for KeyringTokenStore { serde_json::from_str(&secret).ok() } fn save(&self, token: &StoredToken) -> Result<(), AuthError> { - let json = serde_json::to_string(token).map_err(|e| AuthError::Provider(e.to_string()))?; + let json = serde_json::to_string(token).map_err(|e| AuthError::Other(e.to_string()))?; self.entry()? .set_password(&json) - .map_err(|e| AuthError::Provider(e.to_string())) + .map_err(|e| AuthError::Other(e.to_string())) } fn clear(&self) -> Result<(), AuthError> { match self.entry()?.delete_credential() { Ok(()) => Ok(()), Err(keyring_core::Error::NoEntry) => Ok(()), - Err(e) => Err(AuthError::Provider(e.to_string())), + Err(e) => Err(AuthError::Other(e.to_string())), } } } @@ -187,6 +187,9 @@ impl TokenResponse { #[derive(Debug, Deserialize)] struct TokenErrorBody { error: String, + /// Human-readable detail the provider may include (RFC 6749 §5.2). + #[serde(default)] + error_description: Option, } /// Drives the OAuth 2.0 device-code flow against one provider. @@ -208,17 +211,14 @@ impl DeviceFlow { let mut resp = http .get(&url) .call() - .map_err(|e| AuthError::Provider(e.to_string()))?; + .map_err(|e| AuthError::Unreachable(e.to_string()))?; if !resp.status().is_success() { - return Err(AuthError::Provider(format!( - "discovery returned {}", - resp.status() - ))); + return Err(AuthError::rejected(resp.status().as_u16(), None, None)); } let doc: DiscoveryDoc = resp .body_mut() .read_json() - .map_err(|e| AuthError::Provider(e.to_string()))?; + .map_err(|e| AuthError::Other(e.to_string()))?; Ok(DeviceFlow { client_id: client_id.to_string(), http, @@ -233,16 +233,13 @@ impl DeviceFlow { .http .post(&self.device_authorization_endpoint) .send_form([("client_id", self.client_id.as_str()), ("scope", scope)]) - .map_err(|e| AuthError::Provider(e.to_string()))?; + .map_err(|e| AuthError::Unreachable(e.to_string()))?; if !resp.status().is_success() { - return Err(AuthError::Provider(format!( - "device authorization returned {}", - resp.status() - ))); + return Err(AuthError::rejected(resp.status().as_u16(), None, None)); } resp.body_mut() .read_json() - .map_err(|e| AuthError::Provider(e.to_string())) + .map_err(|e| AuthError::Other(e.to_string())) } /// Poll the token endpoint until the user authorizes, the code expires, or @@ -267,13 +264,13 @@ impl DeviceFlow { ("device_code", auth.device_code.as_str()), ("client_id", self.client_id.as_str()), ]) - .map_err(|e| AuthError::Provider(e.to_string()))?; + .map_err(|e| AuthError::Unreachable(e.to_string()))?; if response.status().is_success() { let token: TokenResponse = response .body_mut() .read_json() - .map_err(|e| AuthError::Provider(e.to_string()))?; + .map_err(|e| AuthError::Other(e.to_string()))?; return Ok(token.into_stored()); } @@ -281,7 +278,7 @@ impl DeviceFlow { let body: TokenErrorBody = response .body_mut() .read_json() - .map_err(|e| AuthError::Provider(e.to_string()))?; + .map_err(|e| AuthError::Other(e.to_string()))?; match body.error.as_str() { "authorization_pending" => {} "slow_down" => interval += 5, @@ -301,17 +298,24 @@ impl DeviceFlow { ("refresh_token", refresh_token), ("client_id", self.client_id.as_str()), ]) - .map_err(|e| AuthError::Provider(e.to_string()))?; + .map_err(|e| AuthError::Unreachable(e.to_string()))?; if !response.status().is_success() { - return Err(AuthError::Provider(format!( - "token refresh returned {}", - response.status() - ))); + // The IdP was reached and refused the grant (typically a `400 + // invalid_grant` once the refresh token is expired/rotated). Report + // it as a *rejection* with the OAuth error body — not "unreachable", + // which would misdirect debugging toward the network. + let status = response.status().as_u16(); + let body = response.body_mut().read_json::().ok(); + return Err(AuthError::rejected( + status, + body.as_ref().map(|b| b.error.as_str()), + body.as_ref().and_then(|b| b.error_description.as_deref()), + )); } let mut token: StoredToken = response .body_mut() .read_json::() - .map_err(|e| AuthError::Provider(e.to_string()))? + .map_err(|e| AuthError::Other(e.to_string()))? .into_stored(); // Providers may omit the refresh token on refresh — keep the old one. if token.refresh_token.is_none() { diff --git a/crates/hephd/src/server.rs b/crates/hephd/src/server.rs index 30c5d5a..89dee78 100644 --- a/crates/hephd/src/server.rs +++ b/crates/hephd/src/server.rs @@ -20,6 +20,7 @@ use tokio::net::{UnixListener, UnixStream}; use heph_core::Store; +use crate::auth::AuthError; use crate::oauth::{self, TokenStore}; use crate::rpc::{self, Request, Response, RpcError, INTERNAL_ERROR, PARSE_ERROR}; use crate::selfupdate::{self, SelfUpdateConfig}; @@ -80,10 +81,25 @@ fn is_auth_error(e: &anyhow::Error) -> bool { .is_some_and(|s| s == reqwest::StatusCode::UNAUTHORIZED) } -/// Fold one exchange outcome into the shared [`SyncHealth`]. -fn record_sync_outcome(health: &Arc>, result: &Result) { +/// The exact `heph auth login …` command that re-authenticates this spoke, built +/// from the hub URL + issuer + client id the daemon is configured with — so the +/// surfaced error tells the user *what to run*, not just that auth failed. +/// `None` for an unauthenticated / standalone instance. The hub-URL string must +/// match what the credential store is keyed under, which is exactly `hub_url`. +fn reauth_command(hub_url: Option<&str>, auth: Option<&SpokeAuth>) -> Option { + let (hub, auth) = (hub_url?, auth?); + Some(format!( + "heph auth login --hub-url {hub} --issuer {} --client-id {}", + auth.issuer, auth.client_id + )) +} + +/// Fold one exchange outcome into the shared [`SyncHealth`]. On an auth failure +/// (a 401 from the hub) the recorded error carries the actionable re-login +/// command, so `heph sync --status` / `heph auth status` / the TUI show the fix. +fn record_sync_outcome(ctx: &Ctx, result: &Result) { let now = now_ms(); - let mut h = health.lock().expect("sync_health mutex poisoned"); + let mut h = ctx.sync_health.lock().expect("sync_health mutex poisoned"); h.last_attempt_ms = Some(now); match result { Ok(_) => { @@ -92,28 +108,67 @@ fn record_sync_outcome(health: &Arc>, result: &Result { - h.auth_failure = is_auth_error(e); - h.last_error = Some(e.to_string()); + let auth_failure = is_auth_error(e); + h.auth_failure = auth_failure; + h.last_error = Some(annotate_reauth( + e.to_string(), + auth_failure, + ctx.hub_url.as_deref(), + ctx.auth.as_ref(), + )); } } } +/// Record a failure to obtain a bearer token (the refresh step, before any hub +/// request). A *rejection* (the IdP refused the refresh) is an auth failure and +/// gets the re-login hint; a transport failure stays a transient error. Surfacing +/// this here means `last_error` reflects the real cause (e.g. `invalid_grant`) +/// instead of only the downstream 401 on `/sync/pull`. +fn record_bearer_failure(ctx: &Ctx, err: &AuthError) { + let now = now_ms(); + let auth_failure = err.is_rejection(); + let mut h = ctx.sync_health.lock().expect("sync_health mutex poisoned"); + h.last_attempt_ms = Some(now); + h.auth_failure = auth_failure; + h.last_error = Some(annotate_reauth( + format!("could not obtain bearer token: {err}"), + auth_failure, + ctx.hub_url.as_deref(), + ctx.auth.as_ref(), + )); +} + +/// Append the actionable re-login command to `msg` when this is an auth failure +/// and the spoke has auth configured. +fn annotate_reauth( + msg: String, + auth_failure: bool, + hub_url: Option<&str>, + auth: Option<&SpokeAuth>, +) -> String { + match reauth_command(hub_url, auth) { + Some(cmd) if auth_failure => format!("{msg} — re-authenticate: {cmd}"), + _ => msg, + } +} + impl Ctx { - /// The current bearer token for hub sync (refreshing if expired), or `None` - /// if this spoke has no auth configured / no usable token. - async fn bearer(&self) -> Option { - let auth = self.auth.clone()?; - let result = tokio::task::spawn_blocking(move || { + /// The current bearer token for hub sync (refreshing if expired). `Ok(None)` + /// means this spoke has no auth configured / no token stored (it syncs + /// unauthenticated); `Err` means token acquisition genuinely failed (the + /// caller records it and skips the attempt rather than 401ing the hub). + async fn bearer(&self) -> Result, AuthError> { + let Some(auth) = self.auth.clone() else { + return Ok(None); + }; + match tokio::task::spawn_blocking(move || { oauth::current_bearer(auth.store.as_ref(), &auth.issuer, &auth.client_id) }) - .await; - match result { - Ok(Ok(token)) => token, - Ok(Err(e)) => { - tracing::warn!("could not obtain bearer token: {e}"); - None - } - Err(_) => None, + .await + { + Ok(res) => res, + Err(_join) => Ok(None), // the blocking task panicked; treat as no token } } } @@ -223,10 +278,20 @@ impl Daemon { let mut tick = tokio::time::interval(interval); loop { tick.tick().await; - let bearer = ctx.bearer().await; + let bearer = match ctx.bearer().await { + Ok(b) => b, + Err(e) => { + // Couldn't get a token — record the real cause (e.g. a + // rejected refresh) and skip; sending an unauthenticated + // request would only 401 and mask it. + record_bearer_failure(&ctx, &e); + tracing::warn!("background sync: could not obtain bearer token: {e}"); + continue; + } + }; let result = sync::sync_once(ctx.store.clone(), &hub, &ctx.http, bearer.as_deref()).await; - record_sync_outcome(&ctx.sync_health, &result); + record_sync_outcome(&ctx, &result); match result { Ok(report) => tracing::debug!(?report, "background sync"), Err(e) => tracing::warn!("background sync failed: {e}"), @@ -321,9 +386,25 @@ async fn sync_now(ctx: &Ctx) -> Result { message: "no hub_url configured; this instance is standalone".into(), }); }; - let bearer = ctx.bearer().await; + let bearer = match ctx.bearer().await { + Ok(b) => b, + Err(e) => { + // Token acquisition failed — record the real cause (with a re-login + // hint when it's a rejection) and surface it instead of a downstream 401. + record_bearer_failure(ctx, &e); + return Err(RpcError { + code: INTERNAL_ERROR, + message: annotate_reauth( + format!("sync failed: could not obtain bearer token: {e}"), + e.is_rejection(), + ctx.hub_url.as_deref(), + ctx.auth.as_ref(), + ), + }); + } + }; let result = sync::sync_once(ctx.store.clone(), &hub_url, &ctx.http, bearer.as_deref()).await; - record_sync_outcome(&ctx.sync_health, &result); + record_sync_outcome(ctx, &result); match result { Ok(report) => Ok(json!(report)), Err(e) => Err(RpcError { @@ -374,10 +455,22 @@ async fn sync_status(ctx: &Ctx) -> Result { .expect("sync_health mutex poisoned") .clone(); + // Non-secret OIDC params (issuer/client-id) + the exact re-login command, so + // `heph auth status` can show the fix without reconstructing it client-side + // (and keyed under the right hub URL — see the per-URL token-keying gotcha). + let auth = ctx.auth.as_ref().map(|a| { + json!({ + "issuer": a.issuer, + "client_id": a.client_id, + }) + }); + Ok(json!({ "hub_url": hub_url, "cursors": cursors, "conflicts": conflicts, "health": health, + "auth": auth, + "reauth_command": reauth_command(Some(&hub_url), ctx.auth.as_ref()), })) } diff --git a/crates/hephd/src/sync.rs b/crates/hephd/src/sync.rs index bfaa323..9beac05 100644 --- a/crates/hephd/src/sync.rs +++ b/crates/hephd/src/sync.rs @@ -261,8 +261,14 @@ async fn require_auth( .await .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)? .map_err(|e| match e { - AuthError::Provider(_) => StatusCode::SERVICE_UNAVAILABLE, - _ => StatusCode::UNAUTHORIZED, + // The token itself is missing/bad → tell the client it's unauthorized. + AuthError::Missing | AuthError::Invalid(_) => StatusCode::UNAUTHORIZED, + // We couldn't reach/process the IdP to fetch verification keys — a + // transient hub-side problem, not the client's token. Ask them to + // retry rather than claiming their token is invalid. + AuthError::Unreachable(_) | AuthError::Rejected(_) | AuthError::Other(_) => { + StatusCode::SERVICE_UNAVAILABLE + } })?; // Multi-tenancy seam: resolve the token's identity to the owner it may act diff --git a/crates/hephd/tests/oauth.rs b/crates/hephd/tests/oauth.rs index f61c872..0a1c709 100644 --- a/crates/hephd/tests/oauth.rs +++ b/crates/hephd/tests/oauth.rs @@ -90,11 +90,25 @@ async fn token(State(s): State, Form(form): Form Json(json!({ - "access_token": "access-2", - "expires_in": 3600, - })) - .into_response(), + Some("refresh_token") => { + // A rotated/expired refresh token is refused with `400 invalid_grant` + // (RFC 6749 §5.2) — the case that used to be mislabeled "unreachable". + if form.get("refresh_token").map(String::as_str) == Some("refresh-expired") { + return ( + StatusCode::BAD_REQUEST, + Json(json!({ + "error": "invalid_grant", + "error_description": "Token is not active", + })), + ) + .into_response(); + } + Json(json!({ + "access_token": "access-2", + "expires_in": 3600, + })) + .into_response() + } _ => ( StatusCode::BAD_REQUEST, Json(json!({ "error": "unsupported_grant_type" })), @@ -129,6 +143,48 @@ fn refresh_keeps_the_old_refresh_token_when_omitted() { assert_eq!(refreshed.refresh_token.as_deref(), Some("refresh-1")); } +#[test] +fn refresh_rejected_by_idp_is_a_rejection_not_unreachable() { + let issuer = start_idp(); + let flow = DeviceFlow::discover(&issuer, "heph-cli").unwrap(); + let err = flow.refresh("refresh-expired").unwrap_err(); + // The whole point of the fix: a reachable IdP that returns 400 is a + // *rejection*, carrying the OAuth error body — not "unreachable". + assert!(err.is_rejection(), "expected a rejection, got: {err}"); + let msg = err.to_string(); + assert!( + msg.contains("rejected"), + "message should say rejected: {msg}" + ); + assert!( + msg.contains("invalid_grant"), + "should include the OAuth error: {msg}" + ); + assert!( + msg.contains("Token is not active"), + "should include error_description: {msg}" + ); + assert!( + !msg.contains("unreachable"), + "must NOT claim the IdP was unreachable: {msg}" + ); +} + +#[test] +fn discovery_against_a_dead_idp_is_unreachable_not_a_rejection() { + use hephd::AuthError; + // Port 1 refuses the connection → a genuine transport failure. + let err = match DeviceFlow::discover("http://127.0.0.1:1/application/o/heph/", "heph-cli") { + Ok(_) => panic!("discovery should fail against a dead IdP"), + Err(e) => e, + }; + assert!( + matches!(err, AuthError::Unreachable(_)), + "a connection failure must be Unreachable, got: {err}" + ); + assert!(!err.is_rejection()); +} + #[test] fn memory_token_store_round_trips_and_reports_expiry() { let store = MemoryTokenStore::default(); diff --git a/docs/changelog.d/auth-error-clarity.bugfix.md b/docs/changelog.d/auth-error-clarity.bugfix.md new file mode 100644 index 0000000..83ba854 --- /dev/null +++ b/docs/changelog.d/auth-error-clarity.bugfix.md @@ -0,0 +1 @@ +hephd no longer reports a rejected OAuth refresh as "identity provider unreachable". A reachable IdP that returns an HTTP error (e.g. `400 invalid_grant` once a refresh token expires/rotates) is now surfaced as a *rejection* — `identity provider rejected the request: HTTP 400 (invalid_grant): …` — with the OAuth error body, distinct from a genuine transport failure. This stops the wording from misdirecting incident response toward the network when the real fix is re-authentication. diff --git a/docs/changelog.d/auth-error-clarity.feature.md b/docs/changelog.d/auth-error-clarity.feature.md new file mode 100644 index 0000000..ab67867 --- /dev/null +++ b/docs/changelog.d/auth-error-clarity.feature.md @@ -0,0 +1 @@ +Spoke auth failures now tell you how to recover. When a refresh token is rejected or the hub returns 401, `hephd` records the real cause plus the exact `heph auth login --hub-url … --issuer … --client-id …` command (keyed to this spoke's hub) in its sync health. A new `heph auth status` prints that health and the re-login command, `heph sync --status`'s `last_error` carries it, and `heph-tui`'s status line points at it with a `⚠ auth · heph auth status` chip. diff --git a/docs/how-to/set-up-sync-hub.md b/docs/how-to/set-up-sync-hub.md index a5b56ea..4d654a9 100644 --- a/docs/how-to/set-up-sync-hub.md +++ b/docs/how-to/set-up-sync-hub.md @@ -130,19 +130,41 @@ spoke is visible at a glance rather than buried in the daemon log. Make a change on `gilbert`, force a sync, and confirm it appears via the hub. +### When sync stops authenticating + +A spoke's refresh token can expire or be rotated (e.g. the IdP session lapses). +The spoke then can't refresh on its own and needs a re-login — but this is +**visible, not silent**: + +- `heph-tui` shows a red `⚠ auth · heph auth status` chip in the status line. +- `heph auth status` prints the auth health and the **exact** re-login command, + pre-filled with this spoke's hub URL / issuer / client id: + + ```bash + heph auth status + ``` + +- `heph sync --status`'s `last_error` names the real cause — a refresh + *rejection* (e.g. `HTTP 400 (invalid_grant)`), not a misleading "identity + provider unreachable" — and carries the same `heph auth login …` hint. + +Run the printed `heph auth login …` command to restore sync. + ## Current gaps (finalized by the blumeops deployment) -The flag-level flow above works today; two enablers make it a clean, managed +The flag-level flow above works today; one enabler makes it a clean, managed deployment rather than a hand-run process — tracked in the `Hephaestus` project: -- **`heph daemon` only generates a `--mode local` service** (no `--hub-url` / - `--oidc-*`). So for now the hub and the spoke config are expressed as `hephd` - flags (run directly, or via the blumeops-managed systemd unit), not via - `heph daemon start`. - **Path A seeding is manual** (copy the store + reset the device origin). A small enabler — seed a hub from a snapshot with a fresh origin, or `hephd --owner-id` — would make this one step. +> `heph daemon start`/`restart` can now bake the spoke/hub config (`--hub-url`, +> `--mode server`, `--http-addr`, `--oidc-*`) into the generated service (see +> [[run-the-daemon]]). The canonical hub on `indri` is still provisioned via the +> blumeops-managed systemd unit by deployment choice, not because `heph daemon` +> can't express it. + ## Related - [[run-the-daemon]] — manage the local daemon as an OS service From 2ca1e246f0b178de046bb781c60c595a7fdaab16 Mon Sep 17 00:00:00 2001 From: Forgejo Actions Date: Mon, 8 Jun 2026 14:15:03 -0700 Subject: [PATCH 14/18] Update changelog for v1.3.0 [skip ci] --- CHANGELOG.md | 14 ++++++++++++++ docs/changelog.d/+sync-age-seconds.feature.md | 1 - docs/changelog.d/auth-error-clarity.bugfix.md | 1 - docs/changelog.d/auth-error-clarity.feature.md | 1 - docs/changelog.d/daemon-restart-race.bugfix.md | 1 - .../daemon-self-update-interval.feature.md | 1 - 6 files changed, 14 insertions(+), 5 deletions(-) delete mode 100644 docs/changelog.d/+sync-age-seconds.feature.md delete mode 100644 docs/changelog.d/auth-error-clarity.bugfix.md delete mode 100644 docs/changelog.d/auth-error-clarity.feature.md delete mode 100644 docs/changelog.d/daemon-restart-race.bugfix.md delete mode 100644 docs/changelog.d/daemon-self-update-interval.feature.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 9799e3f..1acdbe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,20 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [v1.3.0] - 2026-06-08 + +### Features + +- Spoke auth failures now tell you how to recover. When a refresh token is rejected or the hub returns 401, `hephd` records the real cause plus the exact `heph auth login --hub-url … --issuer … --client-id …` command (keyed to this spoke's hub) in its sync health. A new `heph auth status` prints that health and the re-login command, `heph sync --status`'s `last_error` carries it, and `heph-tui`'s status line points at it with a `⚠ auth · heph auth status` chip. +- `heph daemon start`/`restart` can now bake the daemon's full runtime config into the managed service — `--mode`, `--hub-url`, `--http-addr`, `--oidc-issuer`/`--oidc-audience`/`--oidc-client-id`, and `--self-update-interval-secs` (previously only the bare `--self-update` bool was wired). Regenerating preserves whatever is already baked into the on-disk plist/unit, so a bare `start`/`restart` no longer silently drops spoke/hub or self-update config. +- heph-tui's sync indicator now shows the last-sync age in seconds under a minute (`⟳ 26s`) instead of a flat `just now`, so the chip reads as a live heartbeat and a missed sync (the loop runs every 30s) shows up as the age climbing. + +### Bug Fixes + +- hephd no longer reports a rejected OAuth refresh as "identity provider unreachable". A reachable IdP that returns an HTTP error (e.g. `400 invalid_grant` once a refresh token expires/rotates) is now surfaced as a *rejection* — `identity provider rejected the request: HTTP 400 (invalid_grant): …` — with the OAuth error body, distinct from a genuine transport failure. This stops the wording from misdirecting incident response toward the network when the real fix is re-authentication. +- `heph daemon restart` on macOS no longer intermittently fails with `launchctl bootstrap failed: 5: Input/output error`. The old code bootstrapped immediately after `bootout`, racing launchd's asynchronous teardown; it now waits for the service to fully unload and retries the bootstrap. When the plist is unchanged (e.g. a plain binary upgrade) it uses `launchctl kickstart -k` to restart the loaded job atomically, sidestepping the bootout→bootstrap dance entirely. + + ## [v1.2.3] - 2026-06-06 ### Features diff --git a/docs/changelog.d/+sync-age-seconds.feature.md b/docs/changelog.d/+sync-age-seconds.feature.md deleted file mode 100644 index cf453c2..0000000 --- a/docs/changelog.d/+sync-age-seconds.feature.md +++ /dev/null @@ -1 +0,0 @@ -heph-tui's sync indicator now shows the last-sync age in seconds under a minute (`⟳ 26s`) instead of a flat `just now`, so the chip reads as a live heartbeat and a missed sync (the loop runs every 30s) shows up as the age climbing. diff --git a/docs/changelog.d/auth-error-clarity.bugfix.md b/docs/changelog.d/auth-error-clarity.bugfix.md deleted file mode 100644 index 83ba854..0000000 --- a/docs/changelog.d/auth-error-clarity.bugfix.md +++ /dev/null @@ -1 +0,0 @@ -hephd no longer reports a rejected OAuth refresh as "identity provider unreachable". A reachable IdP that returns an HTTP error (e.g. `400 invalid_grant` once a refresh token expires/rotates) is now surfaced as a *rejection* — `identity provider rejected the request: HTTP 400 (invalid_grant): …` — with the OAuth error body, distinct from a genuine transport failure. This stops the wording from misdirecting incident response toward the network when the real fix is re-authentication. diff --git a/docs/changelog.d/auth-error-clarity.feature.md b/docs/changelog.d/auth-error-clarity.feature.md deleted file mode 100644 index ab67867..0000000 --- a/docs/changelog.d/auth-error-clarity.feature.md +++ /dev/null @@ -1 +0,0 @@ -Spoke auth failures now tell you how to recover. When a refresh token is rejected or the hub returns 401, `hephd` records the real cause plus the exact `heph auth login --hub-url … --issuer … --client-id …` command (keyed to this spoke's hub) in its sync health. A new `heph auth status` prints that health and the re-login command, `heph sync --status`'s `last_error` carries it, and `heph-tui`'s status line points at it with a `⚠ auth · heph auth status` chip. diff --git a/docs/changelog.d/daemon-restart-race.bugfix.md b/docs/changelog.d/daemon-restart-race.bugfix.md deleted file mode 100644 index c13a257..0000000 --- a/docs/changelog.d/daemon-restart-race.bugfix.md +++ /dev/null @@ -1 +0,0 @@ -`heph daemon restart` on macOS no longer intermittently fails with `launchctl bootstrap failed: 5: Input/output error`. The old code bootstrapped immediately after `bootout`, racing launchd's asynchronous teardown; it now waits for the service to fully unload and retries the bootstrap. When the plist is unchanged (e.g. a plain binary upgrade) it uses `launchctl kickstart -k` to restart the loaded job atomically, sidestepping the bootout→bootstrap dance entirely. diff --git a/docs/changelog.d/daemon-self-update-interval.feature.md b/docs/changelog.d/daemon-self-update-interval.feature.md deleted file mode 100644 index b5ec9b8..0000000 --- a/docs/changelog.d/daemon-self-update-interval.feature.md +++ /dev/null @@ -1 +0,0 @@ -`heph daemon start`/`restart` can now bake the daemon's full runtime config into the managed service — `--mode`, `--hub-url`, `--http-addr`, `--oidc-issuer`/`--oidc-audience`/`--oidc-client-id`, and `--self-update-interval-secs` (previously only the bare `--self-update` bool was wired). Regenerating preserves whatever is already baked into the on-disk plist/unit, so a bare `start`/`restart` no longer silently drops spoke/hub or self-update config. From 5c2b4bde2cf18f0ba45b1edff7c474cf092d3b9f Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Mon, 8 Jun 2026 14:35:10 -0700 Subject: [PATCH 15/18] Relabel changelog v1.3.0 section as v1.4.0 [skip ci] A double workflow_dispatch produced both v1.3.0 and an empty duplicate v1.4.0 (the version actually deployed via self-update). Move the release notes onto v1.4.0 to match what shipped; v1.3.0 release+tag are being removed. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1acdbe1..aa29354 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). -## [v1.3.0] - 2026-06-08 +## [v1.4.0] - 2026-06-08 ### Features From b04a71421ed437320080e1d9cd1a64f6439f3c01 Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Mon, 8 Jun 2026 15:19:10 -0700 Subject: [PATCH 16/18] fix(hephd): reconnect the socket client across daemon restarts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Client` connected to the unix socket once and never reconnected, so after an opt-in self-update or `heph daemon restart` dropped the socket, every later `call()` failed — `heph-tui` would sit on errors until relaunched (and the work we just shipped makes restarts more frequent). `Client` now stores the socket path and reconnects on a dropped connection, classifying the failure to stay safe: - write-side failure (request never reached the daemon) → reconnect + retry once; - reply lost after sending (daemon closed mid-request) → reconnect for next time but surface this one, so a mutation is never silently double-applied; - genuine RPC errors are passed through untouched. heph-tui and the CLI use `Client` unchanged, so the TUI self-heals on its next refresh tick. Adds an integration test driving a mock daemon that drops the connection after each request. Closes the "heph-tui: reconnect on a dropped daemon socket" backlog task. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/hephd/src/client.rs | 112 +++++++++++++++++--- crates/hephd/tests/client_reconnect.rs | 96 +++++++++++++++++ docs/changelog.d/client-reconnect.bugfix.md | 1 + docs/how-to/run-the-daemon.md | 8 ++ 4 files changed, 204 insertions(+), 13 deletions(-) create mode 100644 crates/hephd/tests/client_reconnect.rs create mode 100644 docs/changelog.d/client-reconnect.bugfix.md diff --git a/crates/hephd/src/client.rs b/crates/hephd/src/client.rs index c3c008b..8a2bd5d 100644 --- a/crates/hephd/src/client.rs +++ b/crates/hephd/src/client.rs @@ -2,59 +2,145 @@ //! //! Used by the `heph` CLI and by tests. Surfaces never touch SQLite directly //! (tech-spec §3) — they go through the daemon socket, which this wraps. +//! +//! The connection self-heals across daemon restarts (opt-in self-update, `heph +//! daemon restart`): a [`call`](Client::call) that finds the socket dropped +//! reconnects. It only auto-retries when the request provably never reached the +//! daemon (a write-side failure); a reply lost *after* sending is surfaced +//! rather than retried, so a mutation is never silently double-applied. use std::io::{BufRead, BufReader, Write}; use std::os::unix::net::UnixStream; -use std::path::Path; +use std::path::{Path, PathBuf}; -use anyhow::{bail, Context, Result}; +use anyhow::{anyhow, Context, Result}; use serde_json::{json, Value}; use crate::rpc::Response; /// A connected client. One request/response per [`call`](Client::call). pub struct Client { + socket_path: PathBuf, reader: BufReader, writer: UnixStream, next_id: u64, } +/// How a single request/response exchange failed — drives the retry decision. +enum ExchangeError { + /// The request could not be written (broken pipe, reset): it never reached + /// the daemon, so retrying on a fresh connection is safe. + Send(anyhow::Error), + /// The request was sent but no reply came back (the daemon closed mid-flight, + /// e.g. it restarted): it may or may not have applied — do not retry. + Recv(anyhow::Error), + /// A well-formed RPC-level error (or an unparseable reply): the connection is + /// fine; nothing to reconnect. + Rpc(anyhow::Error), +} + +impl ExchangeError { + fn into_inner(self) -> anyhow::Error { + match self { + ExchangeError::Send(e) | ExchangeError::Recv(e) | ExchangeError::Rpc(e) => e, + } + } +} + impl Client { /// Connect to a daemon listening at `socket_path`. pub fn connect(socket_path: &Path) -> Result { - let stream = UnixStream::connect(socket_path) - .with_context(|| format!("connecting to hephd at {}", socket_path.display()))?; - let reader = BufReader::new(stream.try_clone()?); + let (reader, writer) = Self::open(socket_path)?; Ok(Client { + socket_path: socket_path.to_path_buf(), reader, - writer: stream, + writer, next_id: 1, }) } + /// Open a fresh reader/writer pair on the socket. + fn open(socket_path: &Path) -> Result<(BufReader, UnixStream)> { + let stream = UnixStream::connect(socket_path) + .with_context(|| format!("connecting to hephd at {}", socket_path.display()))?; + let reader = BufReader::new(stream.try_clone()?); + Ok((reader, stream)) + } + + /// Re-establish the connection (after the daemon restarted and dropped it). + fn reconnect(&mut self) -> Result<()> { + let (reader, writer) = Self::open(&self.socket_path)?; + self.reader = reader; + self.writer = writer; + Ok(()) + } + /// Call `method` with `params`, returning the `result` value (or an error /// carrying the RPC error's code and message). + /// + /// If the daemon has restarted and dropped the socket, this reconnects: it + /// retries transparently when the request never went out, and otherwise + /// reconnects for the next call while surfacing an error for this one (so a + /// mutation whose reply was lost is not silently re-applied). pub fn call(&mut self, method: &str, params: Value) -> Result { let id = self.next_id; self.next_id += 1; - let mut line = serde_json::to_string(&json!({ "id": id, "method": method, "params": params, }))?; line.push('\n'); - self.writer.write_all(line.as_bytes())?; - self.writer.flush()?; + + match self.exchange(&line) { + Ok(v) => Ok(v), + Err(ExchangeError::Rpc(e)) => Err(e), + Err(ExchangeError::Send(_)) => { + // The request never reached the daemon — reconnect and retry once. + self.reconnect() + .context("hephd connection lost and reconnect failed")?; + self.exchange(&line) + .map_err(ExchangeError::into_inner) + .with_context(|| format!("retrying `{method}` after reconnect")) + } + Err(ExchangeError::Recv(e)) => { + // Sent but no reply: the daemon likely restarted mid-request. Don't + // retry (a mutation may have applied); reconnect for next time and + // surface this one. + let _ = self.reconnect(); + Err(e).context( + "hephd closed the connection mid-request (it likely restarted); \ + reconnected — re-run the action if it didn't take effect", + ) + } + } + } + + /// One request/response over the current connection, classifying failures. + fn exchange(&mut self, line: &str) -> std::result::Result { + self.writer + .write_all(line.as_bytes()) + .map_err(|e| ExchangeError::Send(e.into()))?; + self.writer + .flush() + .map_err(|e| ExchangeError::Send(e.into()))?; let mut response_line = String::new(); - let read = self.reader.read_line(&mut response_line)?; + let read = self + .reader + .read_line(&mut response_line) + .map_err(|e| ExchangeError::Recv(e.into()))?; if read == 0 { - bail!("hephd closed the connection"); + return Err(ExchangeError::Recv(anyhow!("hephd closed the connection"))); } - let response: Response = serde_json::from_str(&response_line)?; + let response: Response = + serde_json::from_str(&response_line).map_err(|e| ExchangeError::Rpc(e.into()))?; if let Some(err) = response.error { - bail!("rpc error {}: {}", err.code, err.message); + return Err(ExchangeError::Rpc(anyhow!( + "rpc error {}: {}", + err.code, + err.message + ))); } Ok(response.result.unwrap_or(Value::Null)) } diff --git a/crates/hephd/tests/client_reconnect.rs b/crates/hephd/tests/client_reconnect.rs new file mode 100644 index 0000000..a4d0074 --- /dev/null +++ b/crates/hephd/tests/client_reconnect.rs @@ -0,0 +1,96 @@ +//! [`Client`] survives the daemon dropping the socket (opt-in self-update, `heph +//! daemon restart`). A mock daemon serves exactly one request per connection +//! then closes it, forcing the client to reconnect — without auto-reconnect, +//! every call after the first would fail forever. + +use std::io::{BufRead, BufReader, Write}; +use std::os::unix::net::UnixListener; +use std::path::PathBuf; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Arc; +use std::thread; +use std::time::Duration; + +use hephd::Client; +use serde_json::{json, Value}; + +/// A mock daemon that handles ONE request per connection then closes it, looping +/// to accept the next connection. `served` counts total requests answered. +fn spawn_one_shot_daemon(socket: PathBuf, served: Arc) { + thread::spawn(move || { + let listener = UnixListener::bind(&socket).unwrap(); + for conn in listener.incoming() { + let Ok(mut stream) = conn else { continue }; + let mut reader = BufReader::new(stream.try_clone().unwrap()); + let mut line = String::new(); + if reader.read_line(&mut line).unwrap_or(0) == 0 { + continue; // client opened then went away; wait for the next one + } + let req: Value = serde_json::from_str(&line).unwrap(); + let n = served.fetch_add(1, Ordering::SeqCst) + 1; + let mut out = serde_json::to_string(&json!({ + "id": req["id"], + "result": { "served": n }, + })) + .unwrap(); + out.push('\n'); + let _ = stream.write_all(out.as_bytes()); + let _ = stream.flush(); + // `stream` drops here → the connection closes after one request. + } + }); +} + +fn wait_for(socket: &std::path::Path) { + for _ in 0..400 { + if socket.exists() { + return; + } + thread::sleep(Duration::from_millis(5)); + } + panic!("mock daemon socket never appeared"); +} + +#[test] +fn client_reconnects_after_the_daemon_drops_the_socket() { + let dir = tempfile::tempdir().unwrap(); + let socket = dir.path().join("d.sock"); + let served = Arc::new(AtomicUsize::new(0)); + spawn_one_shot_daemon(socket.clone(), served.clone()); + wait_for(&socket); + + let mut c = Client::connect(&socket).unwrap(); + + // First call works on the initial connection. + let r1 = c.call("ping", json!({})).unwrap(); + assert_eq!(r1["served"], 1); + + // The daemon has now closed that connection. With reconnect, the client + // recovers within a call or two (depending on whether the dead socket fails + // on write or on read); without it, every further call would fail forever. + let mut recovered = None; + for _ in 0..2 { + if let Ok(v) = c.call("ping", json!({})) { + recovered = Some(v); + break; + } + } + let r = recovered.expect("client should reconnect after the socket was dropped"); + // The recovered call was served exactly once on the new connection — no + // double-serve from a spurious retry. + assert_eq!(r["served"], 2); + assert_eq!(served.load(Ordering::SeqCst), 2); + + // And it keeps working across subsequent drops. + let r3 = { + let mut got = None; + for _ in 0..2 { + if let Ok(v) = c.call("ping", json!({})) { + got = Some(v); + break; + } + } + got.expect("client should keep reconnecting") + }; + assert_eq!(r3["served"], 3); +} diff --git a/docs/changelog.d/client-reconnect.bugfix.md b/docs/changelog.d/client-reconnect.bugfix.md new file mode 100644 index 0000000..ae987b8 --- /dev/null +++ b/docs/changelog.d/client-reconnect.bugfix.md @@ -0,0 +1 @@ +The `heph` CLI and `heph-tui` now survive a daemon restart. Previously the unix-socket client connected once and never reconnected, so an opt-in self-update or `heph daemon restart` left every subsequent call failing — `heph-tui` would sit on errors until relaunched. The client now reconnects on a dropped socket: a request that never went out is retried transparently, while a reply lost mid-request is surfaced (not silently retried) so a mutation is never double-applied. A long-running TUI self-heals on its next refresh tick. diff --git a/docs/how-to/run-the-daemon.md b/docs/how-to/run-the-daemon.md index cb9e56d..545b3be 100644 --- a/docs/how-to/run-the-daemon.md +++ b/docs/how-to/run-the-daemon.md @@ -86,6 +86,14 @@ still the old binary until you restart it: heph daemon restart ``` +A restart (or an opt-in self-update) drops the daemon's unix socket out from +under any connected surface. The CLI and `heph-tui` **reconnect automatically**: +a read transparently retries on a fresh connection, and a long-running TUI +self-heals on its next tick — so a daemon restart no longer leaves the agenda +view stuck on errors. (A mutating action whose reply is lost mid-restart reports +"reconnected — re-run the action if it didn't take effect" rather than risk +applying twice.) + ## Self-update (opt-in) `hephd` can keep itself current: `heph daemon start --self-update` generates a From 470ef1de0e5551183649df45b8118a63d9f44450 Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Mon, 8 Jun 2026 20:08:07 -0700 Subject: [PATCH 17/18] fix(quickadd): return focus to the previous app when the popover hides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global ⌘' quick-add overlay is a borderless, transparent, always-on-top accessory window that winit hides with `Visible(false)`. That orders the window out visually but leaves heph-quickadd the *active* application — so after a capture (or Esc / toggle) keyboard focus never returns to the app the user was in, and the lingering overlay can keep intercepting clicks where it used to sit. Hide at the application level instead via `NSApplication.hide:`, which fully orders our windows out and activates the next app in line (the previously focused one). On re-show, `unhide:` clears that hidden flag before the existing viewport `Focus` command makes the field key again. Both are macOS-only no-ops elsewhere, wired through new `app_yield_focus`/`app_take_focus` helpers backed by objc2 / objc2-app-kit (unified to the 0.6/0.3 line global-hotkey already pulls). Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 2 + crates/heph-quickadd/Cargo.toml | 11 ++++- crates/heph-quickadd/src/app.rs | 43 +++++++++++++++++++ .../feature-quickadd-focus-return.bugfix.md | 1 + 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 docs/changelog.d/feature-quickadd-focus-return.bugfix.md diff --git a/Cargo.lock b/Cargo.lock index be8f974..cc9b3a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2237,6 +2237,8 @@ dependencies = [ "heph-core", "hephd", "libc", + "objc2 0.6.4", + "objc2-app-kit 0.3.2", "serde_json", "winit", ] diff --git a/crates/heph-quickadd/Cargo.toml b/crates/heph-quickadd/Cargo.toml index 5b1889b..57bbb98 100644 --- a/crates/heph-quickadd/Cargo.toml +++ b/crates/heph-quickadd/Cargo.toml @@ -19,7 +19,16 @@ global-hotkey = "0.8" # macOS-only: winit for the accessory-mode activation policy (no Dock icon), # pinned to the same minor eframe carries so cargo unifies to one winit; libc -# for getppid() (orphan detection — self-exit when the supervising daemon dies). +# for getppid() (orphan detection — self-exit when the supervising daemon dies); +# objc2 + objc2-app-kit to hand keyboard focus back to the previously active app +# when the popover hides (NSApplication.hide:/unhide:). Pinned to the 0.6/0.3 +# line global-hotkey already pulls in, so cargo unifies to one copy. [target.'cfg(target_os = "macos")'.dependencies] winit = "0.30" libc = "0.2" +objc2 = "0.6" +objc2-app-kit = { version = "0.3", default-features = false, features = [ + "std", + "NSApplication", + "NSResponder", +] } diff --git a/crates/heph-quickadd/src/app.rs b/crates/heph-quickadd/src/app.rs index b08bf03..a334b22 100644 --- a/crates/heph-quickadd/src/app.rs +++ b/crates/heph-quickadd/src/app.rs @@ -226,6 +226,9 @@ impl QuickAdd { } fn show(&mut self, ctx: &egui::Context) { + // Undo the app-level hide from the previous `hide()` so we can take focus + // again (no-op the first time / off macOS). + app_take_focus(); self.visible = true; self.focus_pending = true; self.current_hint = random_hint(self.current_hint); @@ -256,6 +259,13 @@ impl QuickAdd { ctx.send_viewport_cmd(egui::ViewportCommand::InnerSize(egui::vec2(WIN_W, BASE_H))); self.win_h_applied = BASE_H; } + // Hand keyboard focus back to the app underneath us. winit's + // `Visible(false)` alone leaves *us* the active application, so focus + // never returns and the borderless always-on-top overlay can keep eating + // clicks where it used to sit. `NSApplication.hide:` orders our windows + // fully out and activates the next app in line — exactly the one the user + // was in (no-op off macOS). + app_yield_focus(); } /// Optimistic submit: hide now, create in the background. @@ -596,6 +606,39 @@ impl QuickAdd { } } +/// Hide the popover at the *application* level so macOS hands keyboard focus +/// back to the previously active app. `NSApplication.hide:` orders all our +/// windows out and activates the next app in line — the one the user was in — +/// which a plain winit `Visible(false)` does not do. No-op off macOS. +#[cfg(target_os = "macos")] +fn app_yield_focus() { + use objc2::MainThreadMarker; + use objc2_app_kit::NSApplication; + // eframe's `update` runs on the main thread, so this marker is always Some. + if let Some(mtm) = MainThreadMarker::new() { + NSApplication::sharedApplication(mtm).hide(None); + } +} + +#[cfg(not(target_os = "macos"))] +fn app_yield_focus() {} + +/// Undo [`app_yield_focus`]: clear the app-level hidden flag before re-showing, +/// so the window the viewport `Focus` command then makes key actually appears. +/// (`unhide:` also re-activates us; the per-window `Focus`/`Visible` viewport +/// commands do the rest.) No-op off macOS. +#[cfg(target_os = "macos")] +fn app_take_focus() { + use objc2::MainThreadMarker; + use objc2_app_kit::NSApplication; + if let Some(mtm) = MainThreadMarker::new() { + NSApplication::sharedApplication(mtm).unhide(None); + } +} + +#[cfg(not(target_os = "macos"))] +fn app_take_focus() {} + /// The current parent process id, for orphan detection. `None` off macOS (where /// hephd does not supervise a helper — there is no Aqua session to inherit). fn current_parent_pid() -> Option { diff --git a/docs/changelog.d/feature-quickadd-focus-return.bugfix.md b/docs/changelog.d/feature-quickadd-focus-return.bugfix.md new file mode 100644 index 0000000..6835eb5 --- /dev/null +++ b/docs/changelog.d/feature-quickadd-focus-return.bugfix.md @@ -0,0 +1 @@ +Quick-add popover (⌘'): hand keyboard focus back to the previously active app when it hides, and stop the (now invisible) overlay from intercepting clicks where it used to sit. From b34371af873f2d9a6ba3f097093b5efef22e07ef Mon Sep 17 00:00:00 2001 From: Forgejo Actions Date: Mon, 8 Jun 2026 20:24:38 -0700 Subject: [PATCH 18/18] Update changelog for v1.4.1 [skip ci] --- CHANGELOG.md | 8 ++++++++ docs/changelog.d/client-reconnect.bugfix.md | 1 - docs/changelog.d/feature-quickadd-focus-return.bugfix.md | 1 - 3 files changed, 8 insertions(+), 2 deletions(-) delete mode 100644 docs/changelog.d/client-reconnect.bugfix.md delete mode 100644 docs/changelog.d/feature-quickadd-focus-return.bugfix.md diff --git a/CHANGELOG.md b/CHANGELOG.md index aa29354..1900ad9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [v1.4.1] - 2026-06-08 + +### Bug Fixes + +- The `heph` CLI and `heph-tui` now survive a daemon restart. Previously the unix-socket client connected once and never reconnected, so an opt-in self-update or `heph daemon restart` left every subsequent call failing — `heph-tui` would sit on errors until relaunched. The client now reconnects on a dropped socket: a request that never went out is retried transparently, while a reply lost mid-request is surfaced (not silently retried) so a mutation is never double-applied. A long-running TUI self-heals on its next refresh tick. +- Quick-add popover (⌘'): hand keyboard focus back to the previously active app when it hides, and stop the (now invisible) overlay from intercepting clicks where it used to sit. + + ## [v1.4.0] - 2026-06-08 ### Features diff --git a/docs/changelog.d/client-reconnect.bugfix.md b/docs/changelog.d/client-reconnect.bugfix.md deleted file mode 100644 index ae987b8..0000000 --- a/docs/changelog.d/client-reconnect.bugfix.md +++ /dev/null @@ -1 +0,0 @@ -The `heph` CLI and `heph-tui` now survive a daemon restart. Previously the unix-socket client connected once and never reconnected, so an opt-in self-update or `heph daemon restart` left every subsequent call failing — `heph-tui` would sit on errors until relaunched. The client now reconnects on a dropped socket: a request that never went out is retried transparently, while a reply lost mid-request is surfaced (not silently retried) so a mutation is never double-applied. A long-running TUI self-heals on its next refresh tick. diff --git a/docs/changelog.d/feature-quickadd-focus-return.bugfix.md b/docs/changelog.d/feature-quickadd-focus-return.bugfix.md deleted file mode 100644 index 6835eb5..0000000 --- a/docs/changelog.d/feature-quickadd-focus-return.bugfix.md +++ /dev/null @@ -1 +0,0 @@ -Quick-add popover (⌘'): hand keyboard focus back to the previously active app when it hides, and stop the (now invisible) overlay from intercepting clicks where it used to sit.