Skip to content

feat(multiuser): Phase 1 schema, core auth infrastructure, and ownership plumbing - #154

Merged
wpfleger96 merged 13 commits into
mainfrom
will/multiuser-phase1
Aug 3, 2026
Merged

feat(multiuser): Phase 1 schema, core auth infrastructure, and ownership plumbing#154
wpfleger96 merged 13 commits into
mainfrom
will/multiuser-phase1

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Add the data model and core infrastructure for SNORE multiuser support (Phase 1 of the gate-cleared multiuser plan). This is a breaking schema change — the old two-migration history is replaced with a single baseline migration; existing DBs should be rebuilt from scratch.

New tables (users, auth_identities, invites, oauth_attempts) and ownership chain (profiles.user_id, devices.profile_id NOT NULL) establish the User 1—N Profile 1—N Device hierarchy. All session/day/waveform/event/statistics data hangs off devices, so a single join predicates every query to a profile. The Day model gains UNIQUE(id, device_id) to support a future composite FK from Session.

  • auth/actor.py: immutable ActorContext (StrEnum Role/AuthMode; can_write derived from role, never supplied at construction)
  • auth/factory.py: single ActorContextFactory — the only place profile ownership is validated; fallback resolution: requested → default → first live
  • services/profile_service.py: ProfileService (CRUD) + DeletionSaga (tombstone → same-filesystem quarantine rename → cascade → purge, startup recovery, CLI-only under exclusive writer lease)
  • services/writer_lease.py: cross-process advisory flock WriterLeaseManager acquired inside BackupService.backup_via_parser() — the sole raw-mutation boundary; API lifetime shared hold via startup lifespan; snore profile delete / snore db purge-quarantine require exclusive non-blocking
  • database/txn.py: run_txn[T] callback-style retry for idempotent units only (invite redemption + uniqueness-protected import chunks; analysis storage explicitly excluded, returns 503 on contention)
  • api/import_jobs.py: PENDING_UPLOAD admission reservation taken before any body bytes; one counter/state machine for ingress + pending + running; publish-terminal → cleanup → release-capacity ordering
  • api/routers/profiles.py: GET/POST/PATCH /api/v1/profiles (no DELETE — CLI-only offline operation)
  • cli/groups/profile.py + cli/groups/user.py: snore profile and snore user command groups; _resolve_user fails instead of first-row guessing when multiple users exist
  • Backup root namespaced ~/.snore/raw/<profile_id>/ in upload flow; SessionImporter threads profile_id for device find-or-create
  • All 1153 tests pass; just check (mypy + ruff lint + ruff format) green

npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq and others added 5 commits August 2, 2026 18:01
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
feat: Phase 1 multiuser schema, new services, and infrastructure
44e8da0 
Add multiuser data model and core infrastructure for SNORE Phase 1:

Schema (breaking — migration regenerated in place):
- New tables: users, auth_identities, invites, oauth_attempts with proper FK
  relationships and uniqueness constraints
- profiles.user_id FK, name, deleting_at (tombstone); UNIQUE(user_id, name)
- devices.profile_id NOT NULL FK; UNIQUE(profile_id, serial_number)
- Day UNIQUE(id, device_id) to support composite FK target
- Regenerated Alembic baseline migration (drops old two-step history)

New modules:
- auth/actor.py: immutable ActorContext (StrEnum Role/AuthMode, can_write derived)
- auth/factory.py: single ActorContextFactory with fallback resolution
- services/profile_service.py: ProfileService + DeletionSaga (tombstone saga)
- services/writer_lease.py: cross-process flock WriterLeaseManager at
  BackupService.backup_via_parser() boundary; API lifetime shared hold
- services/profile_service.py: DeletionSaga (tombstone → quarantine → cascade)
- database/txn.py: run_txn[T] callback-style retry (invite + import chunks only)
- api/routers/profiles.py: GET/POST/PATCH profile API (no DELETE — CLI-only)
- cli/groups/profile.py: snore profile create/list/rename/set-default/delete
- cli/groups/user.py: snore user create/list/disable/invite/invite-revoke

Infrastructure changes:
- import_jobs.py: PENDING_UPLOAD admission reservation state; JobStore interface
  with per-user/global caps; publish-terminal → cleanup → release-capacity order
- backup_service.py: writer lease acquired inside backup_via_parser()
- import_data.py: profile-namespaced backup root (raw/<profile_id>/)
- importers.py: profile_id threaded through SessionImporter._import_single_session
- app.py: profiles router registered; writer lease lifetime hold at lifespan
- cli/__init__.py: profile and user command groups registered
- cli/groups/db.py: snore db purge-quarantine command

Test fixes (all 1153 passing):
- conftest.py: async_test_user + async_test_profile fixtures
- All test call sites that create Device rows now create User+Profile first
- Type annotations added where mypy required them

just check green (mypy + ruff lint + ruff format).

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
test: adversarial infra tests for admission caps, writer lease, and d…
ccb4b2e 
…eletion saga

Step 1 + Step 2 of Phase 1 multiuser work.

Step 1 — composite FK + baseline regen:
- Add ForeignKeyConstraint Session(day_id, device_id) → Day(id, device_id) with
  use_alter=True so SQLite deferred enforcement works correctly.
- Add Day.UNIQUE(id, device_id) (uq_day_id_device) to make the composite FK
  enforceable; both ownership joins are provably identical.
- Add overlaps= annotations to Day.sessions, Session.device, Device.sessions
  relationships to silence the SQLAlchemy column-copy warning introduced by
  the composite FK sharing sessions.device_id across two relationships.
- Regenerate baseline migration in-place (dab8ad625898) to include all of the
  above; no second migration file added.

Step 2 — adversarial infra tests (1184 total, +31 from prior checkpoint):
- tests/unit/test_import_jobs_admission.py (25 tests):
    Per-user cap enforcement, global cap enforcement, slot-reuse-at-cap for all
    5 release paths (try_cancel on PENDING_UPLOAD, try_cancel on PENDING,
    mark_failed, cleanup_files, release_capacity), atomic reservation→job
    conversion invariant, cleanup ordering, concurrent admission under global cap,
    run_txn idempotency / no-duplicate-on-retry.
- tests/integration/test_writer_lease_and_deletion.py (16 tests):
    Writer lease unit: shared/exclusive context managers, nested refcount,
    release idempotency, shared-holds-blocks-exclusive in-process,
    pre-tombstone failure releases exclusive lock.
    Cross-process: subprocess backup blocked during exclusive hold; shared
    acquire succeeds after exclusive released.
    Deletion saga fault injection: pre-tombstone failure leaves no orphans,
    last-profile rejection raises ProfileLastError, tombstone-only leaves
    recovery path, rename-only leaves recovery path, cascade-only leaves
    recovery path (quarantine purge), full recovery with two tombstoned
    profiles leaves no orphaned raw files.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
feat: thread ActorContext through all services and fix CLI plumbing
fd9a9d7 
Step 3+4+5 of Phase 1 multiuser work.

Services threading (Step 3):
- Add profile_id to SessionService, DayService, DatabaseService,
  EventService, DeviceService, WaveformService, StatsService,
  ReportService, ExportService, AnalysisFacade constructors.
- Each service's _profile_filter() / _profile_filters() method now
  scopes queries to the active profile via Device.profile_id.
- profile_id=None is accepted by WaveformService and AnalysisFacade
  for internal callers (WaveformInspector, BatchValidator) that have
  already validated ownership externally; ownership checks are skipped.
- WaveformInspector gains profile_id parameter and threads it to its
  WaveformService instance.
- Import service threads profile_id through ImportJob → device
  find-or-create path.

CLI plumbing (Step 4):
- resolve_local_profile_id() added at the top of every CLI command
  that creates a service directly: stats, analysis delete/batch/single/
  list, waveform list/show/compare, report summary/comparison,
  export csv/json.
- _resolve_session_id() in waveform CLI now returns (session_id,
  profile_id) tuple so callers do not re-query the DB.

Test fixes (Step 5):
- test_waveform_parallel: add ActorContextFactory.make_local AsyncMock
  to all three multi-type test classes so MagicMock DB sessions do
  not hit real async awaits.
- test_analysis_batch: patch resolve_local_profile_id to return 1 so
  the mock session does not need to satisfy the full make_local query
  chain.
- test_api/test_waveforms: update test_list_waveforms_not_found_session
  expectation to 404 — profile-scoped _assert_session_owned now
  returns 404 for sessions outside the profile, which is correct.

Type safety:
- _profile_filter() / _profile_filters() return ColumnElement[bool] /
  list[ColumnElement[bool]] instead of object / list[object].
- ColumnElement added to sqlalchemy imports in all affected services.
- service_dep generic uses Callable[[AsyncSession, int | None], T].
- AnalysisFacade and WaveformService __init__ profile_id typed as
  int | None = None (required int callers still pass it explicitly).

All 1184 tests pass; just check green (mypy + ruff).

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
feat: add --user/--profile CLI flags to all data commands
d687c83 
Add SNORE_USER / SNORE_PROFILE env vars and --user / --profile flags
to every profile-scoped data command (stats, session, analysis, report,
export, waveform, db stats).  Resolution is via the same
ActorContextFactory.make_from_cli() — fails with a clear error on
missing user or profile, never silently picks the first row.

Changes:
- factory.py: add make_from_cli(), _resolve_profile_by_ref(), and
  resolve_cli_profile_id() wrapper (ValueError → ClickException)
- decorators.py: add actor_options() decorator (--user/SNORE_USER +
  --profile/SNORE_PROFILE; param names actor_user/actor_profile)
- 8 CLI files: apply @actor_options, replace resolve_local_profile_id
  calls with resolve_cli_profile_id(db, actor_user, actor_profile)
- test_analysis_batch.py: update mocks from resolve_local_profile_id
  to resolve_cli_profile_id (no behavior change)

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
feat: multi-user ambiguity guard, import actor wiring, isolation tests
b2b5569 
Thufir Pass 4 contract defects addressed:

- make_from_cli: count non-disabled users before falling through to
  make_local; raise ValueError with --user guidance when count > 1
- resolve_local_profile_id: route through make_from_cli so the guard
  propagates to all remaining single-arg CLI callers
- import_data: add actor_options (--user/--profile/SNORE_USER/
  SNORE_PROFILE); thread user_ref/profile_ref through ImportService
  and _import_sources_async; use resolve_cli_profile_id when refs
  are supplied, resolve_local_profile_id otherwise
- analysis._list_sessions: remove dead profile_id=None fallback
  (all callers now supply a resolved id from resolve_cli_profile_id)
- test_waveform_parallel: patch make_from_cli not make_local so the
  unit tests bypass the DB count query entirely
- test_factory_ambiguity: 8 cases covering the guard, disabled-user
  exclusion, count in error message, and resolve_local_profile_id
  propagation
- test_profile_isolation: 9 cases for DeviceService and SessionService
  list and point-lookup isolation across profile boundaries

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 marked this pull request as ready for review August 2, 2026 23:25
@wpfleger96
wpfleger96 marked this pull request as draft August 3, 2026 00:19
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq and others added 4 commits August 2, 2026 20:58
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
fix: make profile isolation non-optional; address Thufir pass 1 findi…
3e4199b 
…ngs 1–7

Profile isolation was optional (None default + fallback to global queries),
allowing any entry point that forgot to pass context to silently leak data
across profiles.  This commit removes all optional paths and makes
profile_id a required constructor argument at every data boundary.

Finding 1 — profiles.py Request annotation:
Removed the defunct 'request: object' annotation that caused FastAPI to
expose 'request' as a required query parameter (422 on every call).
Refactored all three profile route handlers to use ActorDep exclusively.

Findings 2–3 — Import targeting + device scoping:
_run_import now consumes job.target_profile_id so DB writes land in the
correct profile even if the default profile changes after job creation.
_import_single_session device lookup is scoped by (profile_id, serial_number)
so same-serial devices owned by different profiles cannot collide.

Finding 3 — Job control authorization:
cancel_import and import_progress now use ActorDep and authorize against
owner_user_id (foreign or missing job → 404, no information leak).
cancel_import also enforces can_write.

Finding 4 — Export isolation:
ExportService.__init__ now requires profile_id: int (no None default,
no global-query fallback). Backup root is always DEFAULT_RAW_BACKUP_DIR /
profile_id — never client-supplied. API router and all three CLI export
commands resolved via actor/resolve_cli_profile_id.

Finding 5 — Validation isolation:
BatchValidator.__init__ requires profile_id: int. API router and CLI
validate command both resolve profile_id before constructing the validator.

Finding 6 — Rx isolation:
RxTracker.__init__ requires profile_id: int. All device/day queries use
Device.profile_id == self.profile_id. API router and all four CLI rx
commands resolve profile_id via actor/resolve_cli_profile_id.

Finding 7 — analysis show isolation:
All analysis CLI commands (run, list, show, delete) now carry actor_options
and resolve profile_id before any DB query. The show command's date and
numeric-ID lookups both include Device.profile_id == profile_id in WHERE.

Finding 8 — run_txn wired at import chunks:
import_service._import_sources_async now uses run_txn for each import chunk.
UNIQUE(device_id, device_session_id) makes chunk replay safe under contention.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
test: add run_txn fault-injection tests; extend isolation matrix
a32691f 
run_txn idempotency (finding 8):
- test_no_duplicate_session_on_import_chunk_retry: unit_of_work raises on
  attempt 1 (simulated SQLITE_BUSY), succeeds on attempt 2; exactly one
  Session row persisted — UNIQUE(device_id, device_session_id) makes it safe
- test_non_contention_exception_propagates_immediately: non-contention error
  never retried; call_count == 1 on first ValueError
- test_exhausted_contention_raises_last_error: max_attempts=3 exhausted;
  call_count == 3 before RuntimeError raised
- asyncio.sleep patched out to keep tests fast

Isolation matrix extension (finding 8 + Paul synthesis point 4):
- TestExportServiceIsolation: CSV export with profile A returns 0 nights
  when sessions belong to profile B; own profile returns 1; two profiles
  see exactly their own count
- TestBatchValidatorIsolation: validate_date_range finds 0 sessions for
  foreign profile; own-profile query is scoped and never crosses boundaries
- TestRxTrackerIsolation: get_history returns [] for foreign profile; own
  devices + settings produce at least 1 period; two-profile partition check
- TestAnalysisSessionIsolation: direct query (mirrors analysis show
  --session-id) returns None for foreign session_id scoped by profile_id;
  own-profile lookup succeeds

Job control isolation (finding 3):
- test_cancel_foreign_job_returns_404: job owned by user 9999, actor is
  local auto-provisioned (user_id != 9999) → DELETE returns 404
- test_progress_foreign_job_returns_404: same setup for GET progress
- test_cancel_own_job_not_404: unowned job (owner_user_id=None, local mode)
  → DELETE returns 204 or 404

profiles.py regression guard (finding 1):
- test_profiles.py: 12 tests covering list/create/patch without 'request='
  query param (422 regression guard), correct status codes, 404 for foreign
  profile IDs, 409 for duplicate names, and 422 for missing fields

Updated test fixtures for required profile_id constructors:
- test_export_service.py: ExportService(1, ...) and ExportService(1)
- test_rx_tracker.py: RxTracker(1) everywhere
- test_import_state_machine.py: get_actor stub returns profile_id=1
  (required by cancel/progress ActorDep)

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
chore: regenerate ui/src/types/generated.ts
d0dc8e4 
Remove malformed profiles-router parameters (request: unknown was
exposed as a required query param due to the 'request: object'
annotation bug fixed in the previous commit).  Regenerated via:
  uv run python scripts/export_openapi.py ui/openapi.json
  cd ui && pnpm run generate:types

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
fix: scope import backup root to profile; fix e2e raw export test
46c1559 
BackupService was initialized with backup_root=None from the CLI import
path, causing it to fall back to DEFAULT_RAW_BACKUP_DIR without the
profile namespace. ExportService always reads from DEFAULT_RAW_BACKUP_DIR
/ str(profile_id), so the two paths disagreed and export raw could not
find files written by CLI import.

After profile_id is resolved in import_service._import_source_async,
default backup_root to DEFAULT_RAW_BACKUP_DIR / str(profile_id) when the
caller did not provide an explicit path. This mirrors what the API import
path already does (import_data.py:259-261) and ensures CLI import and
export raw share the same directory layout.

The e2e test is updated to match the new contract: no --backup-dir on
either side; both commands use the same --db so they resolve the same
profile_id and agree on the backup root.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 marked this pull request as ready for review August 3, 2026 01:08
@wpfleger96
wpfleger96 marked this pull request as draft August 3, 2026 01:32
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
fix(multiuser): address all 7 pass-2 isolation findings
c2f0293 
Finding 1 (CRITICAL): bulk delete ignores profile ownership
- SessionService.delete_sessions() now carries ownership predicate inside
  the DELETE: subquery on Device.profile_id, no separate SELECT
- AnalysisFacade.delete_analysis() scopes both branches (all_versions and
  single) through an owned_sessions_subq predicate
- Added two-profile isolation matrix tests in test_profile_isolation.py
  proving foreign session/analysis rows survive the delete call

Finding 2 (IMPORTANT): import routes read request.state.actor instead of ActorDep
- import_files() and import_from_path() now declare actor: ActorDep; both
  enforce actor.can_write and snapshot non-null owner_user_id /
  target_profile_id
- CLI import_data: derive raw backup root from resolved profile_id;
  removed --backup-dir client override

Finding 3 (IMPORTANT): AnalysisFacade accepts profile_id=None, fresh scopes unscoped
- AnalysisFacade.__init__ now requires profile_id: int (no None default)
- run_analysis() validates session ownership before I/O phase
- get_analysis_result() validates ownership; returns None for foreign IDs
- BatchValidator._validate_session() constructs AnalysisFacade(db, profile_id)
  for both lookup and run paths

Finding 4 (IMPORTANT): WaveformService/WaveformInspector accept profile_id=None
- Both constructors now require profile_id: int
- WaveformService._assert_session_owned() no longer has a None bypass
- WaveformInspector arg order: profile_id positional before service
- waveform show CLI: unpacks resolved profile_id from _resolve_session_id
  and threads it into WaveformInspector and parallel fresh scopes
- SessionService.resolve_session_id() validates ownership for explicit IDs
- Waveform parallel unit tests: also mock _resolve_session_id (hits DB now)

Finding 5 (IMPORTANT): recover() misses orphaned quarantine dirs post-cascade
- _recover_async() Case 2: enumerate quarantine dirs with no surviving
  tombstone row and purge them directly
- Updated test to call public saga.recover() instead of private helper

Finding 6 (IMPORTANT): lifespan lease can double-acquire shared hold on failure
- Restructured app.py lifespan: exclusive acquire in isolated try/except
  (exclusive_held flag); shared acquire called exactly once unconditionally
  outside both blocks
- Added lifespan unit test asserting _refcount == 1 after injected recovery
  failure and _refcount == 0 after release

Finding 7 (IMPORTANT): run_txn contention test doesn't exercise rollback+replay
- _insert_session UoW: attempt 1 does db.add + await db.flush then raises
  contention, so run_txn session_scope rolls back the transaction
- Attempt 2 inserts cleanly; exactly-one-row assertion validates replay
- Added test_invite_redemption_contention for invite idempotency

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 marked this pull request as ready for review August 3, 2026 02:58
@wpfleger96
wpfleger96 marked this pull request as draft August 3, 2026 03:01
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
fix(analysis): move run_analysis ownership check into read session_sc…
52f47f2 
…ope; scope CLI lookups

Regression (c2f0293): run_analysis() executed its ownership check against
self.db_session, which the CLI closes before calling the facade. This caused
InvalidRequestError on every 'snore analysis run' invocation. Fix: move the
ownership check inside the existing session_scope() read block so it always
runs against a live connection.

Also scoped the CLI pre-lookup queries at analysis.py:479-512: both the date
branch and the ID branch now join through Device and apply
Device.profile_id == profile_id, preventing a foreign session from being
resolved before the facade can reject it.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 marked this pull request as ready for review August 3, 2026 03:06
@wpfleger96
wpfleger96 marked this pull request as draft August 3, 2026 03:18
@wpfleger96
wpfleger96 marked this pull request as ready for review August 3, 2026 03:32
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
fix(isolation): enforce required profile_id across all DB services; r…
d823d22 
…oute-level 404 for foreign IDs

- SessionImporter(profile_id) — required constructor arg; no per-method
  optional profile; contention re-raised past batch catch to reach run_txn
- ImportService.import_sources() — profile_id required (no default);
  internal actor resolution removed; backup root derived from profile_id
- AnalysisService — raises ValueError when db_session given without
  profile_id; compute-only mode (db_session=None) still allowed
- WaveformService.get_waveform_data() — does NOT close caller-owned
  session; regression guard test added
- Route DELETE /sessions/ and DELETE /analysis — get_owned_ids /
  get_owned_session_ids validate ownership before mutation; any foreign
  ID in the set returns 404 without revealing which failed
- Route-level two-profile isolation tests: TestDeleteSessionsCrossProfile
  Isolation and TestDeleteAnalysisCrossProfileIsolation prove foreign rows
  survive and the response is 404, not 200
- Production chunk contention test exercises real _import_batch_with_session
  via run_txn; proves rollback+replay yields exactly one persisted row
- Lifespan lease test replaced: exercises app.router.lifespan_context()
  with DeletionSaga.recover() faulted; asserts refcount==1 during serving,
  0 after exit

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 marked this pull request as draft August 3, 2026 03:58
@wpfleger96
wpfleger96 marked this pull request as ready for review August 3, 2026 04:01
@wpfleger96
wpfleger96 marked this pull request as draft August 3, 2026 04:15
npub17xpz0p704l6vlapga6nahzevr9h0kd9ggfzw640d9yevhmcgst2ql280uq @wpfleger96
fix(analysis): enforce profile ownership in store_result; remove Repo…
ce0a6b8 
…rtService session closes

Fix 1: AnalysisService.store_result() now validates the target session
belongs to self.profile_id before inserting. Selects Session JOIN Device
WHERE Device.profile_id == self.profile_id in the same transaction;
raises NotFoundError when absent. Prevents cross-profile analysis writes
from direct service callers.

Fix 2: Remove both await self._db.close() calls from ReportService
generate_summary_report() and generate_comparison_report(). Injected
sessions are caller-owned; only get_db()/db_session()/session_scope()
may close them. The closes caused silent rollback of local user/profile
provisioning on fresh DBs and InvalidRequestError on subsequent queries.

Fix 3: Tree-wide .close() sweep — all remaining hits are legitimate
session-owner closes (deps.py get_db, session.py session_scope/cursor,
cli/groups/analysis.py deliberate I/O-compute split, import_jobs.py
SSE channel close, parsers/formats/edf.py file handle closes).

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 marked this pull request as ready for review August 3, 2026 04:25
@wpfleger96
wpfleger96 merged commit 22a5b13 into main Aug 3, 2026
9 checks passed
@wpfleger96
wpfleger96 deleted the will/multiuser-phase1 branch August 3, 2026 04:31
@wpfleger-release-bot wpfleger-release-bot Bot mentioned this pull request Aug 2, 2026
Open
@wpfleger96 wpfleger96 mentioned this pull request Aug 3, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@wpfleger96