Refactor all files over 900 LOC into cohesive modules - #35
Merged
Conversation
Extract the ~1000-line index.ts into focused domain modules (rbac, audit, nodes, controller-settings, channel-maps, schedules, recordings, uploads, retention, health-events) matching the existing base.ts/channels.ts leaf pattern. index.ts is now a pure re-export barrel and the shared leaf primitives (isoDateSchema, timeOfDaySchema, audioCaptureBackendSchema) move into base.ts. Public export surface is unchanged; room-capabilities imports Permission from rbac.ts directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Break the ~960-line schema.ts into cohesive modules under src/schema/ (enums, auth, access, rooms, nodes, node-credentials, audio, switchers, channel-maps, settings, schedules, recordings, uploads, room-roster, health, audit). Cross-table FK references form a clean import DAG; schema.ts re-exports every module so drizzle-kit (schema: ./src/schema.ts) and all consumers still see the full table set. drizzle-kit generate confirms no schema changes / no migration drift. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
apps/api/src/index.ts (921 LOC) drops to a thin wiring file by moving the audit/authorization closures (recordAuditEvent, requirePermission, permissionDecision, resourceScopeDecision, roomCapabilityAuthorizes, roster helpers, currentAuth/currentUser, requestContext) into a createAuthorization factory in index-authorization.ts and the /readyz DB probe into createReadinessProbe in index-readiness.ts — mirroring the existing createScopedResources/createResourceScopeTargets pattern. Exports (app, permissionDecision, runners) and startup side-effect order are unchanged; all 643 API tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 917-line inventory.rs becomes a ~135-line facade (public NodeInventory shapes + collect/heartbeat_snapshot/heartbeat_health_details) delegating to submodules under src/inventory/: alsa.rs (ALSA interface discovery, parsing, metadata, identity, serial), runtime.rs (host runtime facts + backend detection), and net.rs (IP-address collection). Unit tests move next to the code they cover. Public API is unchanged; cargo check/clippy/fmt clean and all 19 inventory tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
settings-routes, schedule-routes, recording-routes, agent-routes, recording-job-export, and upload-runner tests (all 960–999 LOC) each split into a co-located non-test harness module (env-before-import ordering + shared helpers + test.after cleanup preserved) plus focused sibling *.test.ts files discovered by the test/**/*.test.ts glob. Every test case moved verbatim; the full suite still reports 662 tests / 643 pass / 19 skipped with 0 failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Break the three oversized fake-controller smoke files into cohesive modules: - health (923) -> a re-export facade over -health-system (system/node lifecycle scenarios) and -health-meters (meter/monitor data-plane scenarios); - recovery (961) -> the upload-boundary orchestrator + its four scenarios, with the disk/device scenarios and their df-command writers moved to -recovery-disk; - the smoke entry (924) -> pure builders/diagnostics (createObserved, retention builders, recorderCachePoliciesForScenario, createJob via makeCreateJob, fakeDfCommandPath, daemonScenarioDiagnostics) moved to -scenario-helpers. Function bodies are byte-identical; public export surfaces are unchanged so importers keep working. All modules import cleanly and check:loc passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The permission catalog moved from packages/shared/src/index.ts into rbac.ts when the shared barrel was split, so verify-rbac-audit-baseline.mjs extracted an empty permission set and flagged every API permission literal as unknown. Read the catalog from packages/shared/src/rbac.ts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The refactor moved invariant-bearing snippets (schema columns, shared schemas, Rust fns, smoke helpers, and split-out test cases) into new files. The baseline verifiers scan hardcoded file lists, so nine of them (scheduler, settings, recording-library, generic-device, first-reliable-recording, health-watchdog, operations, storage-upload, node-lifecycle) lost sight of that evidence. Append the new file locations to each verifier's source list (and, where a verifier asserts the baseline doc references every file / every test file, add the matching path to the baseline .md). Add-only: no snippet, phrase, or existing entry was removed or weakened. All 14 baseline verifiers pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
runRuntimeDiskRecoveryScenario asserts the agent's during-capture disk monitor observes an exhausted disk and runs the runtime-recovery path. The fake capture only lived ~750ms, but the recording-job disk monitor polls once per job-poll-second (1s). On a slower CI runner the capture could be spawned and exit inside a single poll interval, so the monitor never saw the full-disk window: the job just completed normally and the asserted disk_space_runtime_recovered / capture_segments_stitched events never fired (the idle cache sweep, which only runs when no job is active, then cleared the seeded stale cache after completion). Real captures far outlast the poll interval, so this was purely a too-short fake capture. Give this scenario a dedicated capture command that lingers 3.5s (parameterized writeFakeCaptureCommandScript with a lingerMs; default 750ms unchanged for all other scenarios), so the disk monitor deterministically polls df while the capture is still in flight. Smoke passes locally; scenario assertions unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Splits every source file that was over 900 LOC into cohesive modules. Pure reorganization — public export surfaces, behavior, and (for the DB) the generated schema snapshot are all unchanged.
What changed (6 checkpoints)
packages/shared/src/index.tsrbac,audit,nodes,controller-settings,channel-maps,schedules,recordings,uploads,retention,health-events); shared leaf primitives moved intobase.tspackages/db/src/schema.tssrc/schema/; FK graph forms a clean import DAGapps/api/src/index.tsindex-authorization.ts(audit/permission factory) +index-readiness.ts(readiness probe)crates/recorder-agent/src/inventory.rsinventory/{alsa,runtime,net}.rstest.afterpreserved) + focused sibling*.test.tshealthfacade over-health-{system,meters};recoveryorchestrator +-recovery-disk; smoke entry +-scenario-helpersNo file in the repo now exceeds 900 LOC.
Verification
tscclean for shared / db / api / webdrizzle-kit generatereports no schema changes (no migration drift)fmt/check/clippyclean; Miri 138 pass / 28 ignoredagent:fake-controller-smokepasses end-to-endcheck:locpasses🤖 Generated with Claude Code