196 lines
18 KiB
Markdown
196 lines
18 KiB
Markdown
|
|
# Codebase Concerns
|
|||
|
|
|
|||
|
|
**Analysis Date:** 2026-07-29
|
|||
|
|
|
|||
|
|
## Tech Debt
|
|||
|
|
|
|||
|
|
**Federation node removal tombstone gap:**
|
|||
|
|
- Issue: `federation::remove_node()` (`core/archipelago/src/federation/storage.rs:180-197`) calls `tombstone_did()` at line 193 but explicitly drops the error with `let _ = …`. If tombstone write fails (disk I/O, permission, transient), the peer is removed from `nodes.json` but never actually recorded as removed, so the next background sync/notify-join silently re-adds it.
|
|||
|
|
- Files: `core/archipelago/src/federation/storage.rs:180-197`, `core/archipelago/src/api/rpc/federation/handlers.rs:272-300`
|
|||
|
|
- Impact: Federation peers marked for removal can reappear after the next sync cycle, confusing the operator and potentially re-establishing unwanted connections.
|
|||
|
|
- Fix approach: Surface the tombstone-write failure instead of swallowing it; consider retry logic with backoff; add integration test via `tests/multinode/smoke.sh` to verify removal sticks across sync cycles.
|
|||
|
|
|
|||
|
|
**Container reconciler observability gap:**
|
|||
|
|
- Issue: No metrics distinguish "settling after restart" from "flapping" — container thrashing is invisible until anecdotal reports. No per-app restart counter or log line when an app restarts >N times in M minutes.
|
|||
|
|
- Files: `core/archipelago/src/container/prod_orchestrator.rs` (reconciler loop), `core/archipelago/src/health_monitor.rs`
|
|||
|
|
- Impact: Silent restart storms go unnoticed; users see frequent service interruptions without diagnostics; operator can't distinguish normal convergence from a crash loop.
|
|||
|
|
- Fix approach: Add per-app restart counter + log line when threshold exceeded; emit metric on each restart; wire restart count into health/status RPC output.
|
|||
|
|
|
|||
|
|
**Failed systemd unit self-healing gap:**
|
|||
|
|
- Issue: When a Quadlet-backed app's `.service` unit enters `failed` state (e.g., exit 255), the reconciler does not automatically `reset-failed` + `start` it. The unit sits failed until the operator manually intervenes or the service restarts.
|
|||
|
|
- Files: `core/archipelago/src/container/prod_orchestrator.rs` (reconcile loop)
|
|||
|
|
- Impact: Apps with transient failures go down and stay down; no automatic recovery; operator must manually reset or restart the orchestrator.
|
|||
|
|
- Fix approach: Add reconcile step: quadlet-backed app whose `.service` is `failed` and not user-stopped → call `systemctl --user reset-failed <unit>` + `start`; add backoff to avoid busy-loop on persistent failures.
|
|||
|
|
|
|||
|
|
**Bitcoin RPC credentials not retrieved from config/secrets:**
|
|||
|
|
- Issue: `core/container/src/bitcoin_simulator.rs:158` has a TODO marking hardcoded (or missing) RPC credentials in the Bitcoin simulator real-mode path. Credentials should be fetched from the secret store.
|
|||
|
|
- Files: `core/container/src/bitcoin_simulator.rs:155-165`
|
|||
|
|
- Impact: Bitcoin simulator in real mode (Testnet/Mainnet) cannot authenticate to the node; RPC calls fail.
|
|||
|
|
- Fix approach: Inject `SecretsProvider` into `BitcoinSimulator::new()` or pass credentials as constructor args; fetch via `config/secrets` at runtime; handle credential rotation.
|
|||
|
|
|
|||
|
|
**Container security policies not wired in:**
|
|||
|
|
- Issue: `core/security/src/container_policies.rs` generates AppArmor/SELinux profiles but the `apply_profile()` function has a TODO at line 71: "Configure Podman to use the profile" — the profiles are generated but never applied to running containers.
|
|||
|
|
- Files: `core/security/src/container_policies.rs:63-75`
|
|||
|
|
- Impact: Security profiles exist but provide zero protection; containers run without the intended isolation constraints.
|
|||
|
|
- Fix approach: Pass `--security-opt apparmor=<profile>` (or SELinux equivalent) to Podman at container creation; verify profile loads via `apparmor_status`; add CI check that profiles compile cleanly.
|
|||
|
|
|
|||
|
|
**Dynamic resource adjustment not implemented:**
|
|||
|
|
- Issue: `core/performance/src/resource_manager.rs:86` has a TODO for dynamic resource adjustment based on usage. The allocator is static; no adaptive rebalancing when load patterns shift.
|
|||
|
|
- Files: `core/performance/src/resource_manager.rs:86-88`
|
|||
|
|
- Impact: Resource allocation is rigid; a node with skewed usage (e.g., one app consuming all memory) has no mechanism to rebalance dynamically.
|
|||
|
|
- Fix approach: Monitor per-app resource usage via cgroup stats; implement feedback loop to adjust limits; gate on production deployment (likely Phase 3+).
|
|||
|
|
|
|||
|
|
## Known Bugs
|
|||
|
|
|
|||
|
|
**Multinode RPC robustness gap:**
|
|||
|
|
- Symptoms: The `node_rpc()` function in `tests/multinode/lib/multinode.bash` lacks `--max-time` on curl calls — a slow server-side RPC can hang the test suite indefinitely with zero feedback.
|
|||
|
|
- Files: `tests/multinode/lib/multinode.bash` (exact line TBD; see grep for `node_rpc`)
|
|||
|
|
- Trigger: Run multinode federation/mesh test against a slow or overloaded node; curl will block forever.
|
|||
|
|
- Workaround: Manually kill the test process and diagnose the hanging RPC manually; no automatic timeout recovery.
|
|||
|
|
- Fix approach: Add `--max-time 30` to all curl calls in `node_rpc()`; re-run `tests/multinode/smoke.sh` to verify.
|
|||
|
|
|
|||
|
|
## Security Considerations
|
|||
|
|
|
|||
|
|
**Secrets environment variable exposure risk:**
|
|||
|
|
- Risk: Bitcoin and other service credentials are materialized as env vars in `ARCHIPELAGO_*` (e.g., `BITCOIN_RPC_PASSWORD`). Env vars are visible via `/proc/<pid>/environ` and potentially logged.
|
|||
|
|
- Files: `core/archipelago/src/container/prod_orchestrator.rs`, `core/container/src/manifest.rs`, `core/archipelago/src/api/rpc/package/config.rs`
|
|||
|
|
- Current mitigation: Secrets are declared as `generated_secrets` in manifests and materialized 0600/rootless; the orchestrator avoids logging values.
|
|||
|
|
- Recommendations: Audit all env-var passing to containers; consider switching high-sensitivity secrets (bitcoin RPC, LND macaroons) to file-based secrets mounted read-only; add audit logging for secret access.
|
|||
|
|
|
|||
|
|
**Federation DID validation incomplete:**
|
|||
|
|
- Risk: Federation peer DIDs are added via the RPC without cryptographic verification of ownership. A compromised peer could advertise arbitrary DIDs.
|
|||
|
|
- Files: `core/archipelago/src/api/rpc/federation/handlers.rs` (add-node path), `core/archipelago/src/federation/storage.rs`
|
|||
|
|
- Current mitigation: DIDs are stored locally; transitive federation discovery uses the tombstone list to block removed peers.
|
|||
|
|
- Recommendations: Add DID-ownership proof (e.g., signed proof-of-identity) before accepting a peer's advertised DID; document the trust model; consider user warnings when adding peers.
|
|||
|
|
|
|||
|
|
**AppArmor profiles overly permissive:**
|
|||
|
|
- Risk: Generated AppArmor profiles use blanket `network,` instead of per-port/protocol rules. Readonly flag is checkbox only, not enforced per actual app needs.
|
|||
|
|
- Files: `core/security/src/container_policies.rs:46-54`
|
|||
|
|
- Current mitigation: None (profiles not applied).
|
|||
|
|
- Recommendations: Refine per-app capabilities based on manifest's declared needs; add integration test verifying readonly mounts are enforced; apply profiles in development before prod.
|
|||
|
|
|
|||
|
|
## Performance Bottlenecks
|
|||
|
|
|
|||
|
|
**Container thrashing during reconcile:**
|
|||
|
|
- Problem: Restarting `archipelago.service` SIGKILLs every container, forcing a full rebuild over several minutes. Uninstall + reinstall loops can cascade-trigger restarts.
|
|||
|
|
- Files: `core/archipelago/src/container/prod_orchestrator.rs` (the reconciler's desired-state machine)
|
|||
|
|
- Cause: Pre-Phase-3 architecture: containers run in systemd cgroup, not as independent Quadlet units.
|
|||
|
|
- Improvement path: Phase-3 Quadlet default-flip (`config.rs:256`) — each app becomes an independent `.container` unit; restart only the affected app, not the entire cgroup.
|
|||
|
|
|
|||
|
|
**Reconciler churn on boot:**
|
|||
|
|
- Problem: Boot reconciler makes multiple passes reconciling drift; during each pass, containers may be recreated. Post-OTA health checks deliberately skip per-app container assertions because of restart-storm unpredictability.
|
|||
|
|
- Files: `core/archipelago/src/container/prod_orchestrator.rs`, `core/archipelago/src/bootstrap.rs`
|
|||
|
|
- Cause: Multi-pass reconciliation + no incremental diff detection.
|
|||
|
|
- Improvement path: Consolidate reconciler into single pass for boot; cache manifest/config diffs to avoid redundant comparisons; add boot-only fast-path.
|
|||
|
|
|
|||
|
|
**Bitcoin IBD on .198 stalled (disk I/O):**
|
|||
|
|
- Problem: .198 bitcoin is mid-IBD with only 21% progress; disk is 448GB (below 1TB archival threshold); load is high (~3–5).
|
|||
|
|
- Files: `tests/multinode-testing-plan.md` (documented issue)
|
|||
|
|
- Cause: Undersized/slow disk; concurrent workload.
|
|||
|
|
- Improvement path: User decision required: swap in a different node (already done for gate run, using .5 instead) or add storage + wait for sync. Not a code issue.
|
|||
|
|
|
|||
|
|
## Fragile Areas
|
|||
|
|
|
|||
|
|
**Uninstall + reinstall lifecycle:**
|
|||
|
|
- Files: `core/archipelago/src/api/rpc/package/install.rs`, `core/archipelago/src/container/quadlet.rs:disable_remove()`, `neode-ui/src/components/AppCard.vue`
|
|||
|
|
- Why fragile: Pre-2026-07-26, `quadlet::disable_remove()` called systemd + podman with no timeouts, causing hangs. Fixed by commit `71cc9ac4` (added `QUADLET_STOP_TIMEOUT`, SIGKILL escalation, reset-failed). AppCard was hardcoding uninstall bar to "stuck full-red" (fixed `9f17ba68`). Tests for reinstall/cascade are still opt-in.
|
|||
|
|
- Safe modification: Any changes to the uninstall path must be tested via `cascade-uninstall.bats` (7/7 on .228); extend coverage to multi-container stacks (immich, btcpay). Verify on .228 before fleet roll.
|
|||
|
|
- Test coverage: `tests/lifecycle/bats/cascade-uninstall.bats` exists but not in canonical gate; must opt-in with `ARCHY_GATE_CASCADE=1`.
|
|||
|
|
|
|||
|
|
**Production orchestrator state machine:**
|
|||
|
|
- Files: `core/archipelago/src/container/prod_orchestrator.rs` (6291 lines)
|
|||
|
|
- Why fragile: Largest file in the codebase; owns install/start/stop/restart/remove/upgrade for every app; per-app mutex + RwLock concurrency model; complex dependency resolution, adoption scan, Quadlet rendering, and host-port-wait logic interleaved.
|
|||
|
|
- Safe modification: Understand the per-app mutex protocol before touching state mutation; test all changes via the lifecycle gate on .228; use the adoption scan + manifest merge logic for any new manifest evolution.
|
|||
|
|
- Test coverage: 667 unit tests green (2026-07-01); lifecycle gate covers ~8 core apps; ~30 apps untested in gate.
|
|||
|
|
|
|||
|
|
**Mesh radio configuration + boot race:**
|
|||
|
|
- Files: `core/archipelago/src/mesh/meshtastic.rs`, `core/archipelago/src/mesh/mod.rs`, tests at `tests/lifecycle/bats/meshtastic.bats`
|
|||
|
|
- Why fragile: Radio boot-race fixed (2026-07-28, `a8c4694c`/`3f76b496`); on-air config apply must finish before device is used. Earlier versions had probe-boot-race + live config propagation issues. Must verify on real hardware.
|
|||
|
|
- Safe modification: Any mesh changes require E2E test on real LoRa radios (dev-box ↔ x250-dev, or fleet broadcast); unit tests alone won't catch RF timing issues.
|
|||
|
|
- Test coverage: 8-stage on-air smoke test in `tests/multinode/meshtastic.sh` (run manually; not in canonical gate).
|
|||
|
|
|
|||
|
|
**Lightning payment state machine:**
|
|||
|
|
- Files: `core/archipelago/src/api/rpc/lnd/wallet.rs:payinvoice()`
|
|||
|
|
- Why fragile: Slow multi-hop payments (>15s) previously surfaced as "failed" while settling in background; client-side 15s timeout was aborting the wait. Fixed by commit `614a0f5a` (120s wait, pending status, lnd.paymentstatus poll). Must verify on Framework PT with real multi-hop.
|
|||
|
|
- Safe modification: Any lnd state changes must test full payment lifecycle: invoice creation, encoding, send, multi-hop wait, settlement confirmation. Verify on Framework PT before release.
|
|||
|
|
- Test coverage: Local LND payinvoice smoke test; no multinode lightning routing test in gate.
|
|||
|
|
|
|||
|
|
## Scaling Limits
|
|||
|
|
|
|||
|
|
**Uninstall progress bar truthfulness:**
|
|||
|
|
- Current capacity: Uninstall now has timeouts (fixed 2026-07-26) but progress-bar still reports fake stages (full-red full-opacity).
|
|||
|
|
- Limit: Long uninstalls (>30s) show no real progress; bar claims "uninstalling" for the full duration.
|
|||
|
|
- Scaling path: Backend must emit real progress events (% complete, stage name); UI must poll + display truthfully; integrate into all 5 gate iterations (not just 1 throw-away app).
|
|||
|
|
|
|||
|
|
**Federation node list deduplication on disk bloat:**
|
|||
|
|
- Current capacity: `federation/storage.rs:dedup_nodes_by_onion()` reads entire nodes.json into memory each time a node is added/synced. At N federated peers, O(N) memory + O(N²) comparisons per operation.
|
|||
|
|
- Limit: No hard limit measured; scales fine up to hundreds of peers. Beyond 1000+ peers, memory/time may become visible.
|
|||
|
|
- Scaling path: Switch to a disk-backed database (e.g., rocksdb) for federation state if peer count grows; or implement incremental dedup on disk writes (preserve dedup state, only recompute on load).
|
|||
|
|
|
|||
|
|
**Lifecycle gate iteration count:**
|
|||
|
|
- Current capacity: `ARCHY_ITERATIONS=5` runs 5 full cycles (stop/start/restart/survive per app). Entire run takes ~8–12 hours on .228.
|
|||
|
|
- Limit: Cannot easily scale to 10+ iterations without timeout risks; per-app timeout tuning is manual.
|
|||
|
|
- Scaling path: Add per-app timeout tuning (manifest field); parallelize per-app tests where safe (currently serial to avoid contention).
|
|||
|
|
|
|||
|
|
## Dependencies at Risk
|
|||
|
|
|
|||
|
|
**Reticulum transport daemon process group:**
|
|||
|
|
- Risk: Pre-fix (before `be50c886`), process group wasn't cleaned up on drop. Fork-bombs or dangling processes possible under error conditions.
|
|||
|
|
- Impact: Stale reticulum processes accumulating over time; resource leaks on node.
|
|||
|
|
- Migration plan: Code fix already deployed (commit `7a7fec21`); no active risk. Monitor fleet for stale python processes post-deployment.
|
|||
|
|
|
|||
|
|
**Podman socket mount security model:**
|
|||
|
|
- Risk: Apps mounting `/run/podman/podman.sock` get full container-management access. Not restricted by the security policy (AppArmor profiles not applied).
|
|||
|
|
- Files: `core/archipelago/src/container/prod_orchestrator.rs:135-137` (detection), manifests for apps with podman mounts (e.g., portainer)
|
|||
|
|
- Impact: A compromised app with podman socket access can start/stop/delete any container on the node.
|
|||
|
|
- Recommendation: Restrict podman socket mounts to admin-only apps (portainer, docker-api tools); document risk; consider socket filtering layer (selinux context, etc.) once AppArmor is wired.
|
|||
|
|
|
|||
|
|
**Bitcoin version multi-version branch not fleet-wide:**
|
|||
|
|
- Risk: Branch `bitcoin-version-bulletproof` (base `095a76cd`) carries multi-version support but hasn't been deployed fleet-wide yet. .228 carries it; others still run single version.
|
|||
|
|
- Impact: Users on single-version nodes can't switch versions; version mismatch across fleet breaks federation.
|
|||
|
|
- Migration plan: Coordinated OTA + catalog publish + `:latest` repoint sequencing per `docs/bitcoin-version-bulletproof-rollout.md`. Awaiting user decision on timing.
|
|||
|
|
|
|||
|
|
## Missing Critical Features
|
|||
|
|
|
|||
|
|
**Developer tooling CLI suite:**
|
|||
|
|
- Problem: Third-party developers need `archy app validate/render/local-install/lifecycle-test` tooling before external registry launches.
|
|||
|
|
- Blocks: External marketplace (workstream C); external developer onboarding.
|
|||
|
|
- Status: Not yet built; documented in APP-PACKAGING-MIGRATION-PLAN.md step 5.
|
|||
|
|
|
|||
|
|
**Manifest-distributed registry flip:**
|
|||
|
|
- Problem: Manifests still travel via OTA disk rsync. The signed catalog currently distributes only image overrides, not full manifests. Workstream B phases 1+2 done; not yet fleet-deployed.
|
|||
|
|
- Blocks: Cannot confidently add/bump apps without re-signing the catalog.
|
|||
|
|
- Status: Code ready; flip awaits authorization + timing call from user.
|
|||
|
|
|
|||
|
|
**Phase-3 Quadlet default-flip:**
|
|||
|
|
- Problem: Orchestrator still uses legacy cgroup-based container management; Phase-3 `use_quadlet_backends` switch exists but is opt-in only.
|
|||
|
|
- Blocks: Resolves container thrashing; unlocks independent app restarts; unblocks lifecycle perfection (workstream F).
|
|||
|
|
- Status: Code validated on .228/.198 (commit pending); ready to flip when multinode gate passes.
|
|||
|
|
|
|||
|
|
## Test Coverage Gaps
|
|||
|
|
|
|||
|
|
**~30 apps with zero app-specific assertions:**
|
|||
|
|
- What's not tested: Apps like grafana, jellyfin, vaultwarden, penpot, nextcloud, photoprism, uptime-kuma, homeassistant, etc. have no app-specific health checks beyond "container running."
|
|||
|
|
- Files: `tests/lifecycle/bats/all-apps-matrix.bats`, `tests/lifecycle/bats/all-apps-lifecycle.bats` (generic baseline coverage)
|
|||
|
|
- Risk: App-specific bugs (API down, data corruption, dependency failure) go unnoticed until user encounters them.
|
|||
|
|
- Priority: Medium — baseline coverage is a real safety net; app-specific assertions are a "nice to harden" backlog item, not a gate blocker.
|
|||
|
|
- Approach: Add per-app health RPC endpoints or HTTP probes; wire into the gate as opt-in per-app test suites.
|
|||
|
|
|
|||
|
|
**Progress UI assertions incomplete:**
|
|||
|
|
- What's not tested: Install + uninstall must report monotonic, truthful progress. No stage/percentage assertions in the gate.
|
|||
|
|
- Files: `neode-ui/src/components/AppCard.vue`, `core/archipelago/src/api/rpc/package/install.rs` (backend progress events)
|
|||
|
|
- Risk: Silent hangs or fake progress bars are invisible to the gate.
|
|||
|
|
- Priority: High — immich/grafana uninstall was stuck full-red (fixed); progress truthfulness is part of definition of done for workstream F.
|
|||
|
|
- Approach: Backend must emit real progress events; UI must display & test them; integrate into canonical gate (currently opt-in).
|
|||
|
|
|
|||
|
|
**All-apps matrix in cascade gate:**
|
|||
|
|
- What's not tested: `ARCHY_GATE_CASCADE=1` runs ONE throwaway app's uninstall/reinstall. Must extend to multi-container stacks (immich, btcpay, mempool) and all ~40 installed apps.
|
|||
|
|
- Files: `tests/lifecycle/bats/cascade-uninstall.bats` (single-app variant)
|
|||
|
|
- Risk: Multi-container app uninstall bugs (e.g., orphan postgres container) go undetected.
|
|||
|
|
- Priority: High — part of workstream F definition of done.
|
|||
|
|
- Approach: Parametrize cascade test over all manifest IDs; run 5 cascades total (not 5 per app to save time); gate-pass requires zero ghost containers post-uninstall.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
*Analysis based on codebase state 2026-07-29. Issues tracked in `docs/UNIFIED-TASK-TRACKER.md` (day-to-day) and `docs/PRODUCTION-MASTER-PLAN.md` (historical narrative).*
|