sledtools/pika branch #117

pika-git-simplify-2

pika-git: remove legacy auth config and stamp deploy revs

Target branch: master

Merge Commit: 3cf44b467c4908bfd9ee1d48270f7343d4538cdc

branch: merged tutorial: ready ci: success

Open CI Details

Continuous Integration

CI: success

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

Open CI Details

Latest run #144 success

4 passed

head 1e57f8f03d0b5ff2543a3b0f75f55552a2ff617f · queued 2026-03-26 21:38:05 · 4 lane(s)

queued 6s · ran 29s

check-pika-rust · success check-pika-followup · success check-notifications · success check-agent-contracts · success

Summary

This branch removes legacy authentication and configuration fields from pika-git, streamlining the codebase around the newer forge-based architecture. The repos list, merged_lookback_hours, and allowed_npubs config fields are dropped from production code (retained only behind #[cfg(test)] for test deserialization). The AuthState no longer tracks a separate legacy_allowed_npubs set—chat and forge-write access now flows exclusively through bootstrap_admin_npubs and the runtime chat allowlist stored in SQLite. The admin UI panel for legacy chat access is removed, the Nix deployment config is cleaned up, the deploy script switches from git archive | tar to git bundle for more reliable transfers, and the startup log line now prints the single forge repo label instead of a repo-list count.

Tutorial Steps

Remove legacy config fields from the TOML schema and defaults

Intent: Eliminate the `repos`, `merged_lookback_hours`, and `allowed_npubs` fields from the production Config struct, retaining them only under `#[cfg(test)]` so existing tests that deserialize these fields still compile. Delete the `effective_bootstrap_admin_npubs` wrapper and the `default_merged_lookback_hours` helper since they are no longer needed.

Affected files: crates/pika-git/src/config.rs, crates/pika-git/pika-git.example.toml

Evidence

@@ -22,6 +21,8 @@ pub struct Config {
+    #[cfg(test)]
+    #[serde(default)]
     pub repos: Vec<String>,

@@ -33,7 +34,8 @@ pub struct Config {
-    #[serde(default = "default_merged_lookback_hours")]
+    #[cfg(test)]
+    #[serde(default)]
     pub merged_lookback_hours: u64,

@@ -45,6 +47,7 @@ pub struct Config {
+    #[cfg(test)]
     #[serde(default)]
     pub allowed_npubs: Vec<String>,

@@ -73,10 +76,6 @@ pub struct ForgeRepoConfig {
-    pub fn effective_bootstrap_admin_npubs(&self) -> Vec<String> {
-        self.bootstrap_admin_npubs.clone()
-    }

@@ -158,10 +157,6 @@ fn default_github_token_env() -> String {
-fn default_merged_lookback_hours() -> u64 {
-    DEFAULT_MERGED_LOOKBACK_HOURS
-}

@@ -8,7 +8,6 @@ pub const DEFAULT_MODEL: &str = ...
-pub const DEFAULT_MERGED_LOOKBACK_HOURS: u64 = 72;

@@ -1,10 +1,7 @@
-repos = ["sledtools/pika"]
-
 poll_interval_secs = 60
-merged_lookback_hours = 72

The Config struct previously carried three fields that were holdovers from the pre-forge GitHub-polling architecture:

repos – a list of "owner/repo" slugs used by the old GitHub PR poller.
merged_lookback_hours – controlled how far back the poller searched for merged PRs.
allowed_npubs – a legacy Nostr pubkey list that granted chat access but not admin rights.

All three are now gated behind #[cfg(test)] with #[serde(default)], so they vanish from production builds entirely but remain available to unit tests that deserialize TOML snippets containing them.

The effective_bootstrap_admin_npubs() method was a trivial clone wrapper around bootstrap_admin_npubs; callers now access the field directly. The DEFAULT_MERGED_LOOKBACK_HOURS constant and its serde default function are deleted.

The example TOML (pika-git.example.toml) is updated in lockstep: repos and merged_lookback_hours lines are removed so new deployments never see them.

Strip legacy_allowed_npubs from AuthState

Intent: Remove the entire `legacy_allowed_npubs` path from authentication. Chat access and forge-write permission now derive solely from `bootstrap_admin_npubs` (config) and the runtime chat allowlist (SQLite). This simplifies the authorization model to two clear sources of truth.

Affected files: crates/pika-git/src/auth.rs

Evidence

@@ -24,23 +24,14 @@ pub struct AuthState {
-    legacy_allowed_npubs: HashSet<String>,
 }

@@ -31,10 +31,7 @@ impl AuthState {
-    pub fn new(
-        bootstrap_admin_npubs: &[String],
-        legacy_allowed_npubs: &[String],
-        store: Store,
-    ) -> Self {
+    pub fn new(bootstrap_admin_npubs: &[String], store: Store) -> Self {

@@ -127,14 +117,12 @@ impl AuthState {
-            || self.legacy_allowed_npubs.contains(&normalized)
             || self
                 .store
                 .is_chat_allowlist_forge_writer(&normalized)

@@ -151,17 +139,8 @@ impl AuthState {
-    pub fn is_legacy_allowed(&self, npub: &str) -> bool { ... }
     pub fn is_config_managed_chat_principal(&self, npub: &str) -> bool {
-        self.is_admin(npub) || self.is_legacy_allowed(npub)
+        self.is_admin(npub)
     }

@@ -170,15 +149,8 @@ impl AuthState {
-    pub fn legacy_allowed_npubs(&self) -> Vec<String> { ... }
     pub fn chat_enabled(&self) -> bool {
         !self.bootstrap_admin_npubs.is_empty()
-            || !self.legacy_allowed_npubs.is_empty()

The AuthState struct previously maintained two HashSet<String> collections for config-managed identities:

bootstrap_admin_npubs – full admin access.
legacy_allowed_npubs – chat + forge-write access but not admin.

The second set is now removed entirely. Key changes:

Constructor drops the legacy_allowed_npubs parameter; only bootstrap_admin_npubs and the Store are needed.
access_for_npub no longer checks legacy_allowed_npubs when computing can_chat or can_forge_write. These flags now come from either bootstrap admin membership or the SQLite chat allowlist.
is_legacy_allowed and legacy_allowed_npubs methods are deleted.
is_config_managed_chat_principal simplifies to just is_admin.
chat_enabled no longer considers the legacy set when deciding whether chat is active.

Two test cases (legacy_allowed_user_can_chat_but_is_not_admin and hex_legacy_allowed_user_normalizes_for_chat_access) are removed. All remaining tests update their AuthState::new calls to the two-argument signature.

Update web server initialization and admin API

Intent: Align the web layer with the simplified AuthState constructor and remove the legacy allowlist from the admin JSON API response and the admin HTML template.

Affected files: crates/pika-git/src/web.rs, crates/pika-git/templates/admin.html

Evidence

@@ -593,19 +593,7 @@ pub async fn serve(
-    let bootstrap_admin_npubs = config.effective_bootstrap_admin_npubs();
-    let legacy_allowed_npubs = config.allowed_npubs.clone();
-    let auth = Arc::new(AuthState::new(
-        &bootstrap_admin_npubs,
-        &legacy_allowed_npubs,
-        store.clone(),
-    ));
+    let auth = Arc::new(AuthState::new(&config.bootstrap_admin_npubs, store.clone()));

@@ -3336,16 +3324,14 @@ async fn api_admin_allowlist_handler(
-    let legacy_allowed_npubs = state.auth.legacy_allowed_npubs();
-        Ok::<_, anyhow::Error>((entries, bootstrap_admin_npubs, legacy_allowed_npubs))
+        Ok::<_, anyhow::Error>((entries, bootstrap_admin_npubs))

@@ -62,12 +62,6 @@
-        <section class="panel" id="legacy-allowed-section">
-          <h2>Legacy Chat Access</h2>
-          ...
-        </section>

@@ -191,17 +183,6 @@
-        function renderLegacyAllowed(npubs) { ... }

`web.rs`

The serve function previously extracted both effective_bootstrap_admin_npubs() and config.allowed_npubs to pass into AuthState::new. It also printed a warning when bootstrap_admin_npubs was empty but legacy keys existed. All of this is replaced with a single-line construction using &config.bootstrap_admin_npubs.

The api_admin_allowlist_handler no longer fetches or returns legacy_allowed_npubs in its JSON payload. The response shape drops from {bootstrap_admin_npubs, legacy_allowed_npubs, entries} to {bootstrap_admin_npubs, entries}.

Test helper build_test_state is similarly simplified.

`admin.html`

The entire "Legacy Chat Access" panel (#legacy-allowed-section) is removed from the HTML. The corresponding renderLegacyAllowed JavaScript function and its DOM references (legacyAllowedSectionEl, legacyAllowedEl) are deleted. The loadAllowlist fetch callback no longer references data.legacy_allowed_npubs.

Simplify branch inbox recipient collection in the worker

Intent: Replace the two-loop pattern (effective_bootstrap_admin_npubs + allowed_npubs) with a single iteration over `config.bootstrap_admin_npubs`, matching the new config shape.

Affected files: crates/pika-git/src/worker.rs

Evidence

@@ -188,12 +188,7 @@ fn branch_inbox_recipients(store: &Store, config: &Config) -> anyhow::Result<Vec<String>> {
-    for npub in config.effective_bootstrap_admin_npubs() {
-        if let Ok(normalized) = normalize_npub(&npub) {
-            recipients.insert(normalized);
-        }
-    }
-    for npub in &config.allowed_npubs {
+    for npub in &config.bootstrap_admin_npubs {

@@ -321,16 +312,15 @@ mod tests {
-            allowed_npubs: vec![legacy_npub.clone()],
+            allowed_npubs: vec![],
             ...
-            4
+            3

branch_inbox_recipients builds the set of Nostr pubkeys that receive branch tutorial notifications. Previously it iterated effective_bootstrap_admin_npubs() and then config.allowed_npubs, merging both into a BTreeSet. Now it iterates only config.bootstrap_admin_npubs.

The test branch_inbox_populated_for_all_principals is updated:

The legacy_npub key generation is removed.
allowed_npubs is set to an empty vec.
The expected inbox count drops from 4 to 3 (admin + allowlisted chat user + forge writer).

Print forge repo label instead of repo count on startup

Intent: The startup log line previously printed the length of the now-removed `repos` list. Replace it with the forge repo slug (or "disabled") for meaningful operational output.

Affected files: crates/pika-git/src/main.rs

Evidence

@@ -47,6 +47,10 @@ fn main() -> anyhow::Result<()> {
+            let repo_label = config
+                .effective_forge_repo()
+                .map(|forge| forge.repo)
+                .unwrap_or_else(|| "disabled".to_string());

@@ -55,10 +59,10 @@ fn main() -> anyhow::Result<()> {
-                "serve: bind={} db={} repos={} poll_interval={}s model={}",
+                "serve: bind={} db={} repo={} poll_interval={}s model={}",
                 ...
-                config.repos.len(),
+                repo_label,

The main.rs startup banner previously logged repos=N using config.repos.len(). Since the repos field no longer exists in production builds, this is replaced with repo=sledtools/pika (or whatever the configured forge_repo.repo value is), falling back to repo=disabled when no forge repo is configured. The format key is also singularized from repos to repo.

Clean up Nix deployment config

Intent: Remove the now-invalid `repos`, `merged_lookback_hours`, and `allowed_npubs` keys from the generated pika-git.toml in the NixOS module so the deployed config matches the new schema.

Affected files: infra/nix/modules/pika-git.nix

Evidence

@@ -32,7 +32,6 @@ let
-    repos = [ "sledtools/pika" ];

@@ -46,12 +45,10 @@ let
-    merged_lookback_hours = 72;
     ...
-    allowed_npubs = [ ];

The NixOS module generates pika-git.toml via tomlFormat.generate. Three attributes are removed from the generated config:

repos = [ "sledtools/pika" ] — no longer a valid field.
merged_lookback_hours = 72 — deleted from the schema.
allowed_npubs = [ ] — legacy auth list removed.

The remaining fields (forge_repo, bootstrap_admin_npubs, github_token_env, etc.) are unchanged and sufficient for the forge-based architecture.

Switch deploy script from git-archive to git-bundle

Intent: Replace the `git archive --format=tar HEAD | ssh tar -x` pipeline with `git bundle create` + `scp` + remote `git clone` from the bundle. This produces a proper Git repository on the remote, which Nix flake evaluation may require (e.g., for `builtins.fetchGit` or flake metadata).

Affected files: infra/scripts/remote-self-deploy

Evidence

@@ -19,16 +19,23 @@
+local_bundle="$(mktemp "${TMPDIR:-/tmp}/pika-deploy-bundle.XXXXXX")"
 cleanup() {
+  rm -f "${local_bundle}" >/dev/null 2>&1 || true
   "${ssh_cmd[@]}" "rm -rf '${remote_tmp}'" >/dev/null 2>&1 || true
 }

@@ -25,8 +30,12 @@
+echo "==> Bundling repo snapshot for ${target_host}..."
 (
   cd "${flake_root}"
-  git archive --format=tar HEAD
-) | "${ssh_cmd[@]}" "mkdir -p '${remote_tmp}' && tar -xf - -C '${remote_tmp}'"
+  git bundle create "${local_bundle}" HEAD
+)
+echo "==> Uploading repo snapshot to ${target_host}:${remote_tmp}..."
+scp "${ssh_opts[@]}" "${local_bundle}" "root@${target_host}:${remote_tmp}/repo.bundle"
+"${ssh_cmd[@]}" "git clone '${remote_tmp}/repo.bundle' '${remote_tmp}/repo' >/dev/null && cd '${remote_tmp}/repo' && git checkout --detach HEAD >/dev/null"

@@ -33,4 +38,4 @@
-"${ssh_cmd[@]}" "cd '${remote_tmp}' && nix run nixpkgs#nixos-rebuild ..."
+"${ssh_cmd[@]}" "cd '${remote_tmp}/repo' && nix run nixpkgs#nixos-rebuild ..."

The remote-self-deploy script previously piped git archive --format=tar HEAD over SSH and extracted it into a flat directory. This produced a plain file tree without .git metadata, which can cause issues with Nix flake evaluation that expects a Git repo.

The new approach:

Local: git bundle create writes a single-file bundle containing the HEAD commit and all reachable objects to a temp file.
Transfer: scp copies the bundle to the remote temp directory.
Remote: git clone reconstructs a full repo from the bundle, then git checkout --detach HEAD ensures a clean working tree.
Build: nixos-rebuild switch now runs from ${remote_tmp}/repo instead of ${remote_tmp}.

The cleanup trap is extended to remove the local bundle file as well.

Update dev script and documentation

Intent: Keep the local dev helper script and README in sync with the removed config fields.

Affected files: scripts/pika-git, crates/pika-git/README.md

Evidence

@@ -34,13 +34,10 @@ write_dev_config() {
-repos = ["sledtools/pika"]
-
 poll_interval_secs = 60
-merged_lookback_hours = 72

@@ -10,12 +10,10 @@ Hosted forge:
-repos = ["sledtools/pika"]
-
 poll_interval_secs = 60
-merged_lookback_hours = 72
+github_token_env = "GITHUB_TOKEN"

@@ -32,7 +30,6 @@
-- `repos`: legacy repo slug list; keep `["sledtools/pika"]`.

@@ -42,15 +39,13 @@
-- `merged_lookback_hours`: retained legacy setting; ...
+- `github_token_env`: environment variable containing the GitHub token ...

@@ -52,8 +47,6 @@
-`allowed_npubs` remains as a legacy chat-access list ...

`scripts/pika-git`

The write_dev_config function generates a local pika-git.toml for development. The repos and merged_lookback_hours lines are removed from the heredoc template.

`README.md`

The example config block drops repos and replaces merged_lookback_hours with github_token_env. Three documentation bullets are removed:

The repos field description.
The merged_lookback_hours field description (replaced with github_token_env documentation).
The paragraph explaining that allowed_npubs is a legacy chat-access list.

This ensures that anyone reading the README sees only the fields that actually exist in the current config schema.

Update config tests for the simplified schema

Intent: Adjust unit tests in config.rs to stop setting or asserting on removed fields, and rename tests to reflect the new semantics.

Affected files: crates/pika-git/src/config.rs

Evidence

@@ -177,13 +172,11 @@ mod tests {
-repos = ["sledtools/pika", "openclaw/openclaw"]
 poll_interval_secs = 30
-merged_lookback_hours = 48

@@ -198,13 +191,11 @@
-        assert_eq!(parsed.repos.len(), 2);
         assert_eq!(parsed.poll_interval_secs, 30);
-        assert_eq!(parsed.merged_lookback_hours, 48);

@@ -221,7 +212,7 @@
-        let raw = r#"repos = ["test/repo"]"#;
+        let raw = "";

@@ -277,26 +265,18 @@
-    fn bootstrap_admins_do_not_fall_back_to_allowed_npubs() {
-        let raw = r#"
-repos = ["test/repo"]
-allowed_npubs = ["npub1legacy"]
-"#;
+    fn bootstrap_admins_default_to_empty() {
+        let raw = "";

@@ -286,14 +274,10 @@
-    fn explicit_bootstrap_admins_override_legacy_allowed_npubs() {
+    fn parses_explicit_bootstrap_admins() {
         let raw = r#"
-repos = ["test/repo"]
-allowed_npubs = ["npub1legacy"]
 bootstrap_admin_npubs = ["npub1admin"]
-        assert_eq!(
-            parsed.effective_bootstrap_admin_npubs(),
-            vec!["npub1admin".to_string()]
-        );
+        assert_eq!(parsed.bootstrap_admin_npubs, vec!["npub1admin".to_string()]);

Several config tests are updated:

parses_repo_config_contract: The TOML input no longer contains repos or merged_lookback_hours, and the assertions for those fields are removed.
webhook_secret_env_defaults: The minimal config changes from repos = ["test/repo"] to an empty string, since repos is no longer required.
forge_repo_defaults_hook_url_and_ci_command, mirror_remote_defaults_poll_interval_and_timeout, explicit_zero_mirror_timeout_uses_default: All drop their repos = [...] lines.
bootstrap_admins_do_not_fall_back_to_allowed_npubs is renamed to bootstrap_admins_default_to_empty and simplified to parse an empty config and assert the vec is empty.
explicit_bootstrap_admins_override_legacy_allowed_npubs is renamed to parses_explicit_bootstrap_admins and asserts directly on parsed.bootstrap_admin_npubs instead of calling the deleted effective_bootstrap_admin_npubs() method.

pika-git-simplify-2

CI: success

Summary

Tutorial Steps

Remove legacy config fields from the TOML schema and defaults

Strip legacy_allowed_npubs from AuthState

Update web server initialization and admin API

web.rs

admin.html

Simplify branch inbox recipient collection in the worker

Print forge repo label instead of repo count on startup

Clean up Nix deployment config

Switch deploy script from git-archive to git-bundle

Update dev script and documentation

scripts/pika-git

README.md

Update config tests for the simplified schema

Diff

`web.rs`

`admin.html`

`scripts/pika-git`

`README.md`