Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/synthorg/api/auth/controller.py`:
- Around line 784-795: Silent success occurs when _extract_jti(request) returns
falsy; add a DEBUG-level log to record this path for troubleshooting. In the
block where you currently return if not jti, call logger.debug with a clear
message including relevant identifiers (e.g., auth_user.user_id and any
request/session metadata available) so missing jti events are recorded; leave
the revoke and logger.info(API_SESSION_FORCE_LOGOUT, ...) behavior unchanged
when jti exists. Ensure the new log uses the same logger instance and does not
change control flow.
- Around line 286-295: The except Exception handler currently logs a failure
using the API_SESSION_CREATED event constant which is misleading; update the
logger call in the exception block (the except Exception clause in the session
creation flow) to use a failure-specific event constant (e.g.,
API_SESSION_CREATION_FAILED) or at minimum change the event param to a clear
failure string and adjust the log level if appropriate, keeping the rest of the
context (session_id, user_id, exc_info=True) intact so log consumers can
distinguish success vs failure.
- Around line 304-318: Change the broad exception caught when decoding tokens to
the specific jwt.InvalidTokenError: in the block that calls
request.app.state["app_state"] and app_state.auth_service.decode_token(token)
replace the except jwt.PyJWTError: handler with except jwt.InvalidTokenError: so
the helper mirrors the middleware’s expected exception type and only handles
token decode failures explicitly; keep the generic Exception handler and logging
for other failures unchanged.
- Around line 691-707: Validate the incoming scope parameter in the sessions
listing handler: ensure scope is one of the allowed values ("own" or "all") and
return a 400/BadRequest with a clear message if not; then keep the existing
logic that calls store.list_all() when scope == "all" and auth_user.role ==
HumanRole.CEO, otherwise call store.list_by_user(auth_user.user_id). Add the
check near the start of the function (before using scope) and consider replacing
the raw string with an enum or constant for allowed scopes to make comparisons
clearer; reference the AuthenticatedUser type, HumanRole.CEO, and the
store.list_all / store.list_by_user calls when locating where to add the
validation.

In `@src/synthorg/api/auth/presence.py`:
- Around line 8-10: The file currently creates a logger (logger =
get_logger(__name__)) that’s unused; either remove the import/instantiation or
add DEBUG logs inside the presence state handlers — update the logger usage in
the connect and disconnect functions (or similarly named methods handling
presence) to call logger.debug with contextual messages (e.g., "presence.connect
user_id=..." and "presence.disconnect user_id=...") so the logger variable is
used, otherwise delete the get_logger import and the logger =
get_logger(__name__) line to avoid an unused import.

In `@src/synthorg/api/auth/session_store.py`:
- Around line 256-271: The update_last_active method currently performs an
UPDATE that quietly does nothing if the session_id is missing; modify
update_last_active to inspect the statement result (e.g., use the returned
cursor/statement object from await self._db.execute in update_last_active) and,
if no rows were affected (rowcount == 0), emit a DEBUG log via
self._logger.debug with context including the session_id and timestamp;
otherwise proceed to commit as before. Ensure you reference update_last_active
and self._db.execute and that self._logger exists (create or use an existing
logger on the class if necessary).

In `@src/synthorg/api/concurrency.py`:
- Around line 19-37: compute_etag currently returns a weak ETag (prefixing with
W/) but check_if_match uses it with If-Match which per RFC7232 requires strong
validators; change compute_etag to emit a strong ETag (remove the W/ prefix,
keep the quoted hex string) so If-Match uses a strong validator, and verify
check_if_match still expects the quoted digest from compute_etag; alternatively,
if weak semantics were intended, update check_if_match to use If-None-Match
logic—choose the strong-ETag route and remove the W/ in compute_etag to fix the
RFC violation.

In `@src/synthorg/api/controllers/settings.py`:
- Around line 162-184: The current ETag check in _check_setting_etag is
TOCTOU-prone—make settings_service.set accept an optional expected_etag or
expected_updated_at and propagate that to repository.set so the DB performs an
atomic check-and-update (UPDATE ... WHERE namespace=? AND key=? AND updated_at=?
or equivalent) and returns a success flag; update settings_service.set and
repository.set to raise/return a ConflictException when the expected_updated_at
doesn't match (or return False and have controller translate to 409), and keep
_check_setting_etag only as an early advisory validation (or remove its
enforcement) so the real concurrency guarantee is enforced in
settings_service.set/repository.set.

In `@src/synthorg/api/errors.py`:
- Line 43: SESSION_REVOKED (1002) is defined but never returned to clients;
update the authentication failure path to emit this specific error code when a
token is revoked. Locate the token verification / auth middleware (e.g.,
functions named verify_token, authenticate_request, or the auth exception
handler) and add a branch that detects revoked tokens and raises/returns an API
error that includes SESSION_REVOKED (1002) as the error code instead of the
generic unauthorized response; ensure the emitted error uses the same error
response schema used elsewhere so clients can distinguish revoked sessions.

In `@src/synthorg/api/lifecycle.py`:
- Line 216: The code currently reaches into repository internals by calling
persistence.users._db; instead add a public accessor on the persistence layer
(e.g., persistence.get_db() or persistence.users.get_db()) that returns the
DB/connection and use that accessor in lifecycle.py instead of _db. Implement
the new accessor in the persistence module (exposing the concrete DB object from
the users repository), update the call site where db = persistence.users._db to
db = persistence.get_db() or db = persistence.users.get_db(), and ensure type
hints and any existing callers are updated to use the public method rather than
the _db attribute.
- Around line 224-229: The current except Exception block around session store
init (logged with API_APP_STARTUP) treats failures as non-fatal, which permits
startup without revocation support; change it to treat initialization failure as
fatal: log the error (include exc_info/exception details) at error level and
either re-raise the exception or terminate startup (e.g., sys.exit(1)) so the
app does not start without a functioning session store used by JWT jti
revocation checks; update the except block that wraps the session store init and
replace logger.warning with logger.error and a fatal fail/raise to prevent
fail-open behavior.

In `@src/synthorg/api/state.py`:
- Around line 451-458: The setter set_session_store currently overwrites
self._session_store silently; change it to behave like
set_auth_service/set_task_engine by guarding against reassignment (only allow
setting when self._session_store is None and raise/return if already set) and
add an INFO-level state-transition log when the store is assigned; reference the
set_session_store method and the _session_store attribute and mirror the
guard+processLogger.info call pattern used in set_auth_service and
set_task_engine.

---

Outside diff comments:
In `@src/synthorg/settings/definitions/api.py`:
- Around line 1-7: The module docstring in
src/synthorg/settings/definitions/api.py incorrectly states "11 settings";
update that top-of-file docstring to reflect the current number of settings (15)
and any updated grouping if present so it matches the actual registrations in
this module (e.g., the server, CORS, rate limiting, authentication, setup
entries plus the four new TLS/proxy settings); edit the triple-quoted string at
the top of the file to replace "11" with "15" and adjust any related phrasing
(such as counts of runtime-editable vs bootstrap-only settings) to match the
actual definitions registered by this module.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/synthorg/api/auth/presence.py`:
- Around line 27-32: Update the public identifier type hints to use NotBlankStr
from core.types: change the connect(self, user_id: str), disconnect(self,
user_id: str), get_active_connections(self, user_id: str) and is_online(self,
user_id: str) signatures to use user_id: NotBlankStr, add/import NotBlankStr in
this module, and adjust any related docstrings/type comments to reflect
NotBlankStr for the user identifier parameters so the boundary enforces
non-empty/non-whitespace IDs.
- Around line 34-60: Update the presence logging to use the domain event
constants from synthorg.observability.events and log at INFO level: in the
connect and disconnect methods of the presence class (the methods named connect
and disconnect in src/synthorg/api/auth/presence.py) replace logger.debug calls
with logger.info and pass the appropriate event constants (e.g.,
synthorg.observability.events.USER_PRESENCE_CONNECT and
synthorg.observability.events.USER_PRESENCE_DISCONNECT) while retaining the
structured fields (user_id and count); add the necessary import for those
constants at the top of the module.

In `@src/synthorg/api/auth/session.py`:
- Around line 45-50: The model validator _validate_temporal_ordering on the
Session model currently only checks created_at <= expires_at; extend it to also
validate that last_active_at (if present) falls within the session lifetime by
ensuring created_at <= last_active_at <= expires_at, and raise a ValueError with
a clear message (e.g., "last_active_at must be between created_at and
expires_at") when the check fails; update the validator logic in
_validate_temporal_ordering to perform these comparisons on the Session fields
created_at, last_active_at, and expires_at.
- Around line 24-25: The docstring states user_agent is capped at 512 characters
but nothing enforces it; add validation at the session boundary by checking the
user_agent parameter in the session creation path (e.g., Session.__init__ or the
create_session/login_user function) and either truncate it to 512 characters or
raise a ValueError when it exceeds 512; ensure the validation happens before the
value is stored on the session object and include a short warning log if you
truncate.

In `@src/synthorg/api/channels.py`:
- Around line 50-59: The user_channel helper currently allows empty user IDs
(producing "user:")—update its signature to accept core.types.NotBlankStr
(import NotBlankStr) and enforce/reject empty suffixes at the helper boundary
(raise ValueError if the provided value is blank) so malformed channels cannot
be created; likewise update the complementary extractor function extract_user_id
and any other channel helpers in the same file (the ones around lines 74-85) to
accept/return NotBlankStr (or NotBlankStr | None for optional cases), validate
inputs, and consistently raise on invalid/blank identifiers rather than
returning empty strings.

In `@src/synthorg/api/concurrency.py`:
- Around line 40-68: The check_if_match function currently only does string
equality and logs conflicts at INFO; update it to handle full If-Match syntax:
treat a request_etag of "*" as a match, and parse comma-separated entity-tag
lists (strip whitespace and optional surrounding quotes) to check if
current_etag is present; if no match, call logger.warning with
API_CONCURRENCY_CONFLICT and the same context (resource, request_etag,
current_etag) and then raise VersionConflictError (include a descriptive message
mentioning resource/current_etag). Ensure you update check_if_match to skip when
request_etag is empty as before, but otherwise perform the wildcard/list parsing
and use warning-level logging before raising the error.

In `@src/synthorg/api/config.py`:
- Around line 208-231: The code currently treats any non-empty trusted_proxies
as safe and misses IPv6 wildcard binds, so update the validation in the
trusted_proxies loop (and the subsequent host check using self.host and
API_NETWORK_EXPOSURE_WARNING) to detect and reject overly-broad entries like
0.0.0.0/0 or ::/0: parse each entry with ipaddress.ip_network(entry,
strict=False) and if network.prefixlen == 0 (or the network covers the entire
address family) raise ValueError (or at minimum mark as unsafe) so the
network-exposure warning is not suppressed; also extend the host wildcard check
to include '::' (IPv6) in addition to '0.0.0.0' so binding to all interfaces
triggers the warning unless a legitimately narrow trusted_proxies list is
present.

In `@src/synthorg/api/controllers/ws.py`:
- Around line 644-657: The warning currently logs the raw probed channel string
(variable c) in the denial path which may contain another user's identifier;
update the denial logging in the block that checks is_user_channel(c) so it does
not persist the full user:<id> value — either redact the identifier portion
(e.g. replace everything after the prefix returned by user_channel(...) with a
fixed token) or log a deterministic hash of c instead; modify the logger.warning
call (API_WS_USER_CHANNEL_DENIED) to include conn_user.user_id and the
redacted/hashed form of c (but never the raw c), leaving the rest of the control
flow (valid list, subscribed.update) unchanged.
- Around line 497-502: Move presence disconnect into an unconditional teardown
so it runs even if unsubscribe fails: call app_state =
socket.app.state["app_state"] and
app_state.user_presence.disconnect(user.user_id) from a finally block that
always executes, and wrap await channels_plugin.unsubscribe(subscriber) in its
own try/except to catch and log unsubscribe errors (use logger.error or similar)
rather than letting that exception prevent the disconnect; keep the existing
logger.info(API_WS_DISCONNECTED, client=str(socket.client)) after successful
disconnect or in the outer finally as appropriate.

In `@src/synthorg/api/state.py`:
- Around line 456-472: Update set_session_store to log the duplicate-assignment
error before raising and to use the repository event constants instead of a raw
string: import the relevant constants (e.g. EVENT_SESSION_STORE_CONFIGURED and
EVENT_SESSION_STORE_ALREADY_CONFIGURED) from synthorg.observability.events, call
logger.warning(...) with EVENT_SESSION_STORE_ALREADY_CONFIGURED and a
descriptive note immediately before raising RuntimeError when
self._session_store is not None, and replace the raw
"app.state.session_store_set" in the success path with
logger.info(EVENT_SESSION_STORE_CONFIGURED, note="Session store configured").

In `@src/synthorg/persistence/protocol.py`:
- Line 7: Change the get_db() signature in the Protocol to return a concrete
typed contract instead of Any: define or reference a DB connection Protocol/ABC
(e.g., DBConnectionProtocol) and use that as the return type for get_db();
update the Protocol import and annotations accordingly (replace Any with
DBConnectionProtocol or at minimum object) and add a docstring on get_db() that
callers must expect PersistenceConnectionError for connection failures and
NotImplementedError for unsupported backends; apply the same stricter typing to
the related public functions mentioned (lines ~101-110) so mypy strict mode
passes.

In `@src/synthorg/persistence/sqlite/schema.sql`:
- Around line 216-218: Add a new composite index on sessions(revoked,
expires_at) because the load_revoked() and the global active-sessions query in
session_store.py filter by revoked and expires_at without user_id; create an
index like idx_sessions_revoked_expires on (revoked, expires_at) in schema.sql
so those global WHERE revoked = ? AND expires_at > ? queries can use an index
and avoid full table scans.

In `@tests/unit/api/fakes.py`:
- Around line 604-606: Update the return type of FakePersistenceBackend.get_db
to match the protocol by changing its annotation from object to Any: modify the
signature of get_db in the FakePersistenceBackend class to return typing.Any
(import Any from typing if not already imported) and keep the same
NotImplementedError body and message so the fake still raises as before.

---

Outside diff comments:
In `@src/synthorg/settings/service.py`:
- Around line 466-544: The concurrent-modification branch in the async method
set (when written is False after calling self._repository.set) raises
VersionConflictError without logging; add a log call (e.g., logger.warning or
logger.error) immediately before raising that includes the namespace, key,
expected_updated_at and updated_at/context and a short message like "Concurrent
modification on {namespace}/{key}" to match the other error paths (see existing
use of logger.warning with SETTINGS_NOT_FOUND and SETTINGS_VALIDATION_FAILED and
logger.error with SETTINGS_ENCRYPTION_ERROR) so the error is recorded before
raising VersionConflictError.

In `@tests/unit/api/auth/test_middleware.py`:
- Around line 57-68: The tests currently never exercise the session revocation
branch because _FakeState sets has_session_store = False; add a new unit test
that constructs an app like the existing one but sets
_FakeState.has_session_store = True, injects a mock session_store on the fake
state, configures the mock's is_revoked to return True for the test jti, invokes
the protected_route through middleware_cls (using the same auth_service/session
context), and assert the response status is 401 to verify the revocation guard
path is enforced.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/synthorg/api/auth/controller.py`:
- Around line 734-746: The current handler silently falls back to list_by_user
when a non-CEO requests scope="all"; update the branch after the _VALID_SCOPES
check to make the behavior explicit: either (A) enforce denial by raising a 403
(e.g., raise ApiForbiddenError or similar) when scope == "all" and
auth_user.role != HumanRole.CEO so only CEOs can use store.list_all, or (B) keep
the safe fallback but set an explicit response header (e.g.,
response.headers["X-Scope-Applied"] = "own") before calling store.list_by_user
to indicate the reduced scope; implement the chosen change in the controller
where auth_user, _VALID_SCOPES, HumanRole.CEO, store.list_all and
store.list_by_user are referenced (i.e., immediately before calling
store.list_by_user).

In `@src/synthorg/api/auth/session_store.py`:
- Around line 98-102: The session lifecycle logs are using logger.debug for
state transitions; change the calls that log API_SESSION_CREATED (in function
handling session creation where logger.debug(API_SESSION_CREATED,
session_id=session.session_id, user_id=session.user_id)) and the corresponding
session cleanup log (the logger.debug that emits the cleanup constant, e.g.,
API_SESSION_CLEANED_UP) to logger.info while preserving the same message key and
structured fields (session_id, user_id) so these transitions emit at INFO level
to comply with the guidelines.
- Around line 54-56: The public methods on the session store accept plain str
for identifier parameters; update all public API parameter types to use
NotBlankStr (e.g., change session_id: str and user_id: str to session_id:
NotBlankStr and user_id: NotBlankStr) in the SessionStore class methods
referenced (including create, get/fetch, revoke/is_revoked and any methods
accepting tuples/optionals of IDs at the noted locations), and adjust any
tuple/optional type annotations to the corresponding NotBlankStr variants; keep
using the existing imported NotBlankStr and ensure internal uses still treat
values as strings where needed.
- Around line 198-214: The UPDATE in revoke_all_for_user currently sets
revoked=1 for all non-revoked rows regardless of expiry; change it to only
target active sessions by adding the same expires_at > ? predicate used later
(use now = datetime.now(UTC).isoformat()), bind (user_id, now) to the UPDATE
executed via self._db so the returned cursor.rowcount reflects only
newly-revoked active sessions, and keep updating self._revoked from the SELECT
that already uses expires_at > ?.

In `@src/synthorg/api/config.py`:
- Around line 182-189: The before-validator _normalize_empty_tls currently
mutates the incoming data mapping in place; instead make and operate on a copy
and return that to avoid side effects. Update _normalize_empty_tls to deep-copy
the incoming dict (use copy.deepcopy) into a new variable, perform the
empty-string-to-None normalization on that copy for keys ("ssl_certfile",
"ssl_keyfile", "ssl_ca_certs"), and return the copy; also ensure callers
ultimately receive an immutable mapping by wrapping the final constructed
mapping with MappingProxyType as per guidelines.

In `@src/synthorg/api/controllers/ws.py`:
- Around line 651-664: _handle_subscribe currently treats any channel in
_ALL_CHANNELS_SET as acceptable which causes non-privileged users to be
acknowledged for budget channels they will never receive; update the loop that
builds valid (iterating over channels and comparing to _ALL_CHANNELS_SET and
own_user_ch) to call _channel_allowed(conn_user, c) (or equivalent signature)
and only append c to valid if _channel_allowed returns true (preserve the
existing special-case check for own_user_ch and is_user_channel logging
behavior), so that subscribed and filters only include channels that _on_event
can actually deliver.

In `@src/synthorg/settings/service.py`:
- Around line 538-551: The code logs a SETTINGS_VALIDATION_FAILED event when a
write fails due to a version mismatch (variable written is False) but sets
reason="concurrent_modification", which dilutes observability; change the log
call in the block that raises VersionConflictError so it emits a dedicated event
constant (e.g., SETTINGS_VERSION_CONFLICT) instead of SETTINGS_VALIDATION_FAILED
and keep the metadata (namespace, key, expected_updated_at,
reason="concurrent_modification") intact; update any import or event registry
usage if needed and ensure the log location remains in the same branch where
written is falsy and VersionConflictError is raised.

In `@tests/unit/api/auth/test_session_store.py`:
- Around line 65-66: The db fixture's return annotation uses AsyncGenerator with
a single type parameter; update the type hint on the fixture function db to use
the full form AsyncGenerator[aiosqlite.Connection, None] (i.e., include the
second type parameter None) so static type checkers like mypy accept it; change
the annotation on the async def db(...) to AsyncGenerator[aiosqlite.Connection,
None] and adjust imports if necessary.

In `@tests/unit/api/test_channels.py`:
- Around line 55-73: The test currently accepts an empty user suffix for
extract_user_id (test_extract_user_id_empty_suffix), but after adding validation
in channels.py extract_user_id should reject "user:"; update the test to assert
the new behavior—either expect a ValueError to be raised when calling
extract_user_id("user:") or assert it returns None if you choose to signal
invalid channels that way—adjust test_extract_user_id_empty_suffix accordingly
and keep other tests (user_channel, is_user_channel,
test_extract_user_id_valid/non_user_channel) unchanged; reference
extract_user_id, user_channel, and is_user_channel when making the change.

---

Duplicate comments:
In `@src/synthorg/api/auth/presence.py`:
- Around line 34-38: The presence logging should only emit structured domain
events for real state transitions: update the connect(self, user_id) and
disconnect(self, user_id) methods to use the domain event constants (e.g.,
events.presence.PRESENCE_CONNECTED and events.presence.PRESENCE_DISCONNECTED)
instead of inline string messages, change their log level to INFO, and call
logger.info(EVENT_CONST, user_id=user_id, count=self._counts[user_id]) when an
actual transition occurs; additionally, make disconnect return early without
logging if the user was never connected (no-op path) to avoid spurious
disconnect events.
- Around line 25-28: UserPresence currently uses plain str for user IDs in its
storage and public method signatures; update it to use NotBlankStr from
core.types everywhere: import NotBlankStr, change the _counts typing to
dict[NotBlankStr, int], update method signatures for connect(user_id:
NotBlankStr), disconnect(user_id: NotBlankStr), any other methods that
accept/return user IDs (e.g., get_counts / all public APIs) to use NotBlankStr
in parameters and return types, and adjust any internal annotations/usages
accordingly so the presence API consistently enforces NotBlankStr.

In `@src/synthorg/api/auth/session.py`:
- Around line 23-25: The Session model's user_agent field is not enforcing the
documented 512-char cap; update the code that builds or defines Session (e.g.,
the Session dataclass/ORM model or the constructor used in
create_session/Session.create) to enforce a maximum length of 512 characters by
either adding a field constraint/validator or truncating the incoming user_agent
value before constructing the Session instance (ensure the symbol names
user_agent and Session are targeted so all session creation paths abide by the
cap).

In `@src/synthorg/api/state.py`:
- Around line 467-475: Replace the hard-coded "app.state.session_store_set"
string with dedicated event constants: import the appropriate event names (e.g.,
SESSION_STORE_ALREADY_CONFIGURED and SESSION_STORE_CONFIGURED) from the
synthorg.observability.events module used by the repo and use them in
logger.warning and logger.info respectively inside set_session_store();
specifically, update the logger.warning call that signals the
failure/already-configured path to use the SESSION_STORE_ALREADY_CONFIGURED
constant and update the logger.info success call to use
SESSION_STORE_CONFIGURED, and add the new imports at the top of the file.

In `@src/synthorg/persistence/sqlite/settings_repo.py`:
- Around line 124-133: The optimistic-CAS branch that checks expected_updated_at
only issues an UPDATE, so when expected_updated_at is the empty-string sentinel
(meaning “no DB row yet”) the first edit fails; modify the method handling
expected_updated_at to detect the "no-row" sentinel and perform an
insert-if-absent path: after the UPDATE returns zero rows, if
expected_updated_at == "" then attempt to INSERT the (namespace, key, value,
updated_at) row (or use a single SQL upsert/INSERT OR IGNORE then UPDATE) and
treat a successful insert as CAS success; reference the symbols
expected_updated_at, updated_at, namespace, key, value, cursor and self._db in
settings_repo.py to locate where to add the insert/UPSERT branch.