Skip to content

feat: runtime config management API + supporting backend work#24

Open
roziscoding wants to merge 16 commits into
mainfrom
feat/ui
Open

feat: runtime config management API + supporting backend work#24
roziscoding wants to merge 16 commits into
mainfrom
feat/ui

Conversation

@roziscoding

@roziscoding roziscoding commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Context

Adds a runtime config management API so peers, servers, and settings can be managed live (no restart), plus the supporting backend work this branch accumulated. jack's config stays a hand-editable, version-controllable config.jsonc (the file is the source of truth) — the management API mutates it through a single serialized writer and applies changes to the live connector map.

What's included

Config management API (the main deliverable)

  • A separate management surface on its own port (getManagementApp + a 2nd Bun.serve on MANAGEMENT_PORT), guarded by X-Management-Key (constant-time compare), started only when MANAGEMENT_KEY is set. The public peer port never exposes /config.
  • ConfigService: one in-process serialized write queue; rollback-safe atomic file writes (tmprename, randomized temp name, preserved perms); persists the raw config so {env}/{file} secret refs are never resolved into the file.
  • Live peers CRUD: add / remove (disable-as-soft-delete, in-flight drains) / rename / URL-change rekey with a peer_id download cascade.
  • Live servers CRUD (+ source/destination list reconciliation).
  • Live connector visibility: search fan-out (/torznab, /peer, /items, /servers, qB) reads connectors live, so add/remove is searchable without restart.
  • Migration write-back: a migrated config is persisted with a .bak; also fixes a pre-existing crash where an up-to-date config failed to parse on the next boot.

Supporting backend work (earlier commits on this branch)

  • Sensitive-field redaction in logs + a span-attribute redacting funnel.
  • Versioned config migration system; TypeScript 6 bump (noUnusedLocals/Parameters).
  • Connector-lifecycle refactor (decorator rename; ConnectorManager-shaped deps).

Notes

  • No UI yet — the management API is the backend foundation; UI/CLI/BFF are out of scope.
  • MANAGEMENT_KEY is operator-set via env (no first-boot key generation).

Testing

  • bun test: 223 pass / 0 fail · tsc --noEmit: clean · eslint .: clean

Greptile Summary

This PR adds a runtime config management API on a separate port (MANAGEMENT_PORT), guarded by X-Management-Key with a constant-time comparison. It introduces a ConnectorManager class that replaces the old initializeConnectors function, a ConfigService with a serialized write queue and rollback-safe atomic file writes, and a versioned config migration system. Live CRUD for peers and servers is fully wired through controller, router, and service layers.

  • Config management API: A distinct Hono app on its own port reads and mutates config.jsonc through ConfigService, which uses an async mutex to serialize writes and tmp→rename for atomic persistence. Mutation routes are dynamically registered only when a ConfigService is injected.
  • ConnectorManager: Replaces the previous flat arrays with a map-based manager whose servers/peers getters are read live on each request, enabling add/remove without restart. Soft-delete (disable rather than evict) lets in-flight downloads drain.
  • Config migration: migrateConfig applies versioned transformations on startup and writes the migrated file atomically after backing up the original; a pre-existing crash on up-to-date configs is fixed.

Confidence Score: 4/5

Safe to merge for new deployments; a downgrade after first boot risks silently emptying the config file due to a mis-scoped Zod .catch.

The migration's .catch({ version: 0 }) is placed on the entire z.looseObject(...) rather than on just the version field. Any future config (version number above the current maximum) collapses the whole object to { version: 0 }, runs all migrations against the resulting empty structure, and writes the stripped result back to disk — permanently overwriting all peer and server configuration with no visible warning beyond a routine 'Config migrated' log line. A .bak is created so recovery is possible, but operators won't know they need it until their connectors are gone. Everything else — the constant-time key comparison, the serialized write queue, atomic tmp→rename persistence, and the soft-delete drain strategy — is correct and well-tested.

apps/backend/src/lib/config.ts — specifically the migrateConfig function's .catch placement on line 188.

Important Files Changed

Filename Overview
apps/backend/src/lib/config.ts Adds versioned config migration, RawPeerConfig/RawServerConfig schemas, and a version field to AppConfig. The .catch({ version: 0 }) in migrateConfig discards all config content when the version is out of range.
apps/backend/src/modules/config/config.service.ts New ConfigService with a serialized write queue, rollback-safe persist-then-commit ordering, and generic CRUD over peers/servers. Logic is sound; queue poisoning prevention is correctly handled.
apps/backend/src/middleware/require-management-key.ts New constant-time key comparison via SHA-256 hash normalization and a manual XOR loop. Implementation is correct; hashing to a fixed-width digest avoids the length-mismatch issue with timingSafeEqual.
apps/backend/src/lib/servers/index.ts ConnectorManager replaces the old initializeConnectors function with a map-based, live-queryable design. Soft-delete (disable without eviction) is intentionally traded against memory accumulation on high-churn setups.
apps/backend/src/index.ts Wires ConnectorManager, ConfigService, and management server. Port-collision guard is correct but uses logger.fatal without exiting, which is semantically misleading.
apps/backend/src/lib/atomic-write.ts New utility for atomic tmp→rename file writes with randomized temp name, preserved permissions, and cleanup on failure. Correct and well-commented.
apps/backend/src/management-app.ts New Hono app on the management port; globally applies secureHeaders, requireManagementKey, and handleError. No public routes are exposed here.
apps/backend/src/modules/config/config.controller.ts New ConfigController funnels all mutations through a private #mutate helper that guards ConfigService presence and maps ZodErrors to BadRequestError. Clean design.
apps/backend/src/modules/config/config.router.ts Mutation routes are conditionally registered at construction time based on controller.canMutate. Missing routes return 404 rather than 500 when no ConfigService is wired.
apps/backend/src/lib/span-attributes.ts New centralized span attribute funnel with redaction, serialization, size-capping, and sensitive-URL query-param masking. Cleanly replaces scattered span.setAttribute calls.

Sequence Diagram

sequenceDiagram
    participant Client as API Client
    participant MgmtApp as Management App (MANAGEMENT_PORT)
    participant Middleware as requireManagementKey
    participant Controller as ConfigController
    participant Service as ConfigService (queue)
    participant Manager as ConnectorManager
    participant Disk as config.jsonc

    Client->>MgmtApp: "POST /config/peers {X-Management-Key}"
    MgmtApp->>Middleware: validate key (SHA-256 constant-time)
    alt invalid key
        Middleware-->>Client: 401 Unauthorized
    end
    Middleware->>Controller: addPeer(body)
    Controller->>Service: addPeer(input)
    Service->>Service: "#enqueue(task) — serialize write"
    Service->>Service: validate (PeerConfig + RawPeerConfig)
    Service->>Service: check url/name uniqueness
    Service->>Disk: atomicWrite (tmp → rename)
    Service->>Service: "this.#raw = next"
    Service->>Manager: addPeerConnector(resolved)
    Manager->>Manager: PeerConnector.init()
    Controller-->>Client: "201 { ok: true }"

    Note over Client,Disk: Public app reads connectorManager.peers live on next request
Loading

Fix All in Claude Code Fix All in Codex

Reviews (1): Last reviewed commit: "refactor: complete connector-lifecycle r..." | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

Introduce a dedicated management surface (getManagementApp) served on its own
MANAGEMENT_PORT via a second Bun.serve, guarded by X-Management-Key
(constant-time compare) and started only when MANAGEMENT_KEY is set. Adds
GET /config, /config/peers, /config/servers reading the live ConnectorManager.
ConfigService is the single serialized writer: holds the raw (ref-preserving)
config object, persists atomically (tmp + rename) through an async-mutex queue,
and reconciles the live connector map. addPeer validates + resolves secrets,
rejects duplicate url/name (409), strips unknown keys before persisting, and adds
a live PeerConnector. POST /config/peers wired into the management app.
Connector-map getters now honor the enabled flag (peers/servers/sources/
destinations/connectors skip disabled connectors; sources/destinations also gate
on canSource/canDestination). removePeer persists the removal then disables the
live connector (in-flight drains, restart prunes). Same-URL updatePeer replaces
the connector under its stable id. Adds NotFoundError (404); DELETE/PATCH
/config/peers/:id routes.
updatePeer now handles a changed URL inside the serialized write: persist the
file, add the connector under the new id, drain the old one (disable), and cascade
download rows via DownloadsRepository.reassignPeerId (manual ON UPDATE CASCADE) so
downloads follow the peer. Rejects a URL that collides with another peer (409).
addServer/removeServer/updateServer mirror the peer lifecycle through the same
serialized write queue. addServerConnector now reconciles _sourceIds/_destinationIds
so a live-added server is immediately a usable source/destination, and a toggled
capability drops out. URL change rekeys the connector (no download cascade; *arr
re-registration needs a restart). POST/DELETE/PATCH /config/servers.
… crash

getAppConfig now returns a shared { appConfig, raw } so ConfigService is seeded
from the same parsed object (no divergent second read). On a real migration it
backs up the original bytes to <path>.bak (comments intact) and atomically
rewrites the file. Using 'migrated ?? fileContent' also fixes a pre-existing crash
where an already-current config threw AppConfig.parse(undefined) on the next boot.
Fan-out consumers now read connectors live from ConnectorManager instead of
snapshot arrays captured at boot, so management-API add/remove is visible without
restart. Object-taking controllers (Servers/Items/qB) get lazy getter objects;
array-taking consumers (PeerController, TorznabController, getDownloadRouter)
take () => Connector[] providers.
- ConfigService: collapse the 6 near-identical add/remove/update methods behind
  generic #addEntry/#removeEntry/#updateEntry helpers parameterized by slice + an
  optional onRekey hook (peers cascade download rows; servers don't).
- ConfigController: funnel all mutations through one #mutate helper (service-presence
  guard + ZodError->400); expose canMutate.
- Router: mount mutation routes only when a ConfigService is wired, so an
  unconfigured management surface returns 404 instead of 500.
- atomic-write: randomized temp name, preserve target perms (new files 0o600),
  cleanup-on-error.
- Document removeConnector's resident-until-restart trade-off.
Finishes the in-progress refactor and fixes its mechanical fallout:
- DownloadsService and getApp accept the structural { peers } / { servers, peers }
  shape they actually use, so a real ConnectorManager (live) or a lightweight test
  object both satisfy them.
- Widen the base ServerConnector input so PeerConnector's type: 'jack' plumbs
  through ConnectorType; ArrServerConnector tolerates omitted headers (base defaults
  to {}), fixing the radarr/sonarr ctors.
- Rename the init decorator require->requiresInitialization and fix its docstring.
- Update tests to the new signatures; peer-download's markInitialized now resolves
  the initialization promise the @requiresInitialization guard awaits.

Full suite: 223 pass / 0 fail; tsc clean; lint clean.
@github-actions

Copy link
Copy Markdown

🐳 Docker image published

This PR has been built and pushed to GHCR:

ghcr.io/roziscoding/jack:pr-24

Pull and run it locally:

docker pull ghcr.io/roziscoding/jack:pr-24
docker run --rm ghcr.io/roziscoding/jack:pr-24

Last built from commit 1f1b143. Heads up: this image is automatically deleted when the PR is closed.

Comment on lines +186 to +189
const configObject = z
.looseObject({ version: z.number().max(LATEST_MIGRATION).min(0).default(0) })
.catch({ version: 0 })
.parse(rawConfigObject)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 The .catch({ version: 0 }) on the entire z.looseObject(...) parse fires when the version field fails validation — including when version > LATEST_MIGRATION (a downgrade scenario). When it fires, configObject becomes { version: 0 }, discarding every other field (servers, peers, jack, etc.). All migrations then run against that empty object, and the stripped result is atomically written back to disk with a .bak of the original. The backup means data is recoverable, but the production config is silently emptied on first boot after a downgrade — operators may not notice until all their peers and servers are gone.

Applying the catch only to the version field, not the entire schema, keeps the rest of the config intact regardless of the version value.

Suggested change
const configObject = z
.looseObject({ version: z.number().max(LATEST_MIGRATION).min(0).default(0) })
.catch({ version: 0 })
.parse(rawConfigObject)
const configObject = z
.looseObject({ version: z.number().max(LATEST_MIGRATION).min(0).default(0).catch(0) })
.parse(rawConfigObject)

Fix in Claude Code Fix in Codex

Comment thread apps/backend/src/index.ts
let managementServer: ReturnType<typeof Bun.serve> | undefined
if (envs.MANAGEMENT_KEY) {
if (envs.MANAGEMENT_PORT === server.port) {
logger.fatal({ port: envs.MANAGEMENT_PORT }, 'MANAGEMENT_PORT collides with the public port; not starting the management API')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 logger.fatal conventionally signals that the process is about to terminate. Here the application continues serving on the public port — only the management listener is skipped. Using logger.error (or logger.warn) avoids a false alarm in log aggregators and on-call alerting that are configured to page on fatal-level entries.

Suggested change
logger.fatal({ port: envs.MANAGEMENT_PORT }, 'MANAGEMENT_PORT collides with the public port; not starting the management API')
logger.error({ port: envs.MANAGEMENT_PORT }, 'MANAGEMENT_PORT collides with the public port; not starting the management API')

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant