Skip to content

Review and clean up server handlers - #163

Merged
martsokha merged 8 commits into
mainfrom
refactor/handler-cleanups
Jul 29, 2026
Merged

Review and clean up server handlers#163
martsokha merged 8 commits into
mainfrom
refactor/handler-cleanups

Conversation

@martsokha

@martsokha martsokha commented Jul 28, 2026

Copy link
Copy Markdown
Member

Summary

A pass across all server handlers: extract shared logic, drop redundant queries, and fix stale docs and log levels surfaced by the review. No behavior changes to the API surface (responses, status codes, and routes are unchanged); the one route touch consolidates duplicate registrations onto single api_route calls.

Changes

Shared helpers (new handler/utility/accounts.rs)

  • resolve_creator_username / resolve_trigger_username — deduped from four near-identical per-handler copies.
  • build_password_user_inputs — deduped from accounts and authentication.
  • An IdempotencyKey request extractor replaces the ad-hoc header-parsing helper in the runs handler.

Fewer queries — drop redundant create-path re-reads in pipelines, connections, webhooks, policies, and workspaces: the creator is the authenticated caller, so resolve the handle directly instead of re-reading the just-created row. For connections, also skip the sync-time query (a fresh connection has no runs).

Docs corrected to match behavior

  • pipelines: real permission names, and "by slug" (not "by ID").
  • files: drop the false virus-scanning / validation / queued-for-processing claims.
  • accounts: drop the false "audit logging" claim.
  • connections: "credentials + context" → "credentials plus sync state".
  • runs: fix a stale doc describing a deleted function, and a wrong serde error message.

Log levels — lower routine operations that were logged at warn! (file delete, member remove/leave, token revoke) or info! (list-count logs, per-request health responses) to match the rest of the handlers.

Object storage — make the object key an associated type of ObjectBucket (ObjectStore<B> keyed by B::Key), so a store can no longer be opened with the wrong key type. Fixes a mispaired intermediates_store and removes the dead ContextFilesBucket / ContextKey.

Misc — consolidate invites' duplicate same-path route registrations; simplify the find_pipeline_run tuple order; tidy a stray trailing space in a log message.

Verification

  • cargo check --all-features --workspace ✅
  • cargo clippy --all-targets --all-features --workspace -- -D warnings ✅
  • cargo +nightly fmt --all -- --check ✅
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features --workspace ✅
  • cargo test --all-features --workspace ✅ (all suites pass)

🤖 Generated with Claude Code

martsokha and others added 7 commits July 28, 2026 20:02
@martsokha @claude
Extract Idempotency-Key header into a custom extractor
af4322e 
Replace the ad-hoc `idempotency_key(&HeaderMap)` helper in the runs
handler with an `IdempotencyKey` request extractor, so the detect
handler receives an already-validated `Option<String>` and the header
parsing/validation (ASCII, 1-255 chars) lives with the other
extractors. A malformed header now rejects with 400 before the handler
body runs, matching the previous behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martsokha @claude
Clean up runs handler: stale doc, wrong error message, tuple order
c15cf5b 
Three small correctness/clarity fixes in the runs handler:

- Replace the stale doc comment on `resolve_trigger_username`, which
  described a deleted "(pipeline slug, run number)" lookup (run_number
  was dropped) and the wrong function entirely.
- Map analyzed-document (de)serialization failures through a dedicated
  `analysis_serde_error` instead of `serialize_error`, whose message
  ("Failed to process pipeline definition") was wrong for the analysis
  store/load paths.
- Return the query's natural `(run, pipeline, trigger_username)` order
  from `find_pipeline_run` instead of repacking it, and update the
  three call sites.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martsokha @claude
Make the object key an associated type of ObjectBucket
fe73619 
Each bucket is addressed by exactly one key type, so bind the key to the
bucket via `ObjectBucket::Key` instead of a free `ObjectStore<B, K>`
parameter. `ObjectStore<B>` and the `object_store::<B>()` /
`*_store()` helpers now derive the key from the bucket, so a store can
no longer be opened with a key that does not belong to it.

This also fixes `intermediates_store`, which was bound to `FileKey`
while the intermediates bucket is keyed by `IntermediateKey`, and drops
the now-dead `ContextFilesBucket` / `ContextKey` left over after the
contexts resource was removed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martsokha @claude
Drop private rustdoc link from IdempotencyKey docs
9306595 
The struct doc linked the private `MAX_KEY_LENGTH` const, which fails
`cargo doc -D warnings` (public item linking a private one). State the
255-character limit inline instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martsokha @claude
Clean up pipelines handler: stale docs and a redundant re-read
96a1e2b 
- Correct the permission names in the create/list/update/delete doc
  comments, which named file permissions (UploadFiles, ViewFiles,
  UpdateFiles, DeleteFiles) and a non-existent owner check instead of
  the pipeline permissions actually enforced.
- Fix "by ID" / "unique identifier" wording on the get endpoint;
  pipelines are addressed by slug.
- Resolve the creator's handle directly from the authenticated account
  on create instead of re-reading the just-created pipeline by slug.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martsokha @claude
Clean up connections handler: redundant re-read on create, stale doc
1b208e2 
- On create, resolve the creator's handle directly from the
  authenticated account and pass `None` for last-synced (a fresh
  connection has no sync runs), dropping the re-read of the
  just-created connection and its grouped sync-time query.
- Reword the module doc's "credentials + context" to "credentials plus
  sync state" so "context" no longer reads as the removed contexts
  resource.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martsokha @claude
Review and clean up server handlers
db72ebe 
Sweep across all handlers, consolidating shared logic and fixing stale
docs, wrong log levels, and redundant queries surfaced by the review.

Shared helpers (new handler/utility/accounts.rs):
- resolve_creator_username / resolve_trigger_username — deduped from
  four near-identical per-handler copies.
- build_password_user_inputs — deduped from accounts and authentication.

Drop redundant create-path re-reads in pipelines, connections, webhooks,
policies, and workspaces: the creator is the authenticated caller, so
resolve the handle directly instead of re-reading the just-created row
(and, for connections, skip the sync-time query since a fresh connection
has no runs).

Docs corrected to match behavior:
- pipelines: real permission names and "by slug" (not "by ID").
- files: drop the false virus-scanning / validation / queueing claims.
- accounts: drop the false "audit logging" claim.
- connections: "credentials + context" -> "credentials plus sync state".

Lower log levels for routine operations that were logged at warn!
(files delete, member remove/leave, token revoke) or info! (list-count
and per-request health logs) to match the rest of the handlers.

Consolidate invites' duplicate same-path route registrations onto single
api_route calls, and tidy a stray trailing space in a log message.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martsokha martsokha added refactor code restructuring without behavior change server API handlers, middleware, auth nats messaging, job queues, object storage labels Jul 28, 2026
@martsokha martsokha self-assigned this Jul 28, 2026
@martsokha @claude
Flag the current session token in the API token responses
646c6e0 
The JWT carries the session's token id internally (jti) but the token
list exposes no way for a client to tell which listed token is the one
the current request authenticated with. Add a `current` boolean to the
ApiToken response, set by comparing each token's id to the request's
token id, so the frontend can single out the active session. The list,
read, and update handlers set it; create does not, since a freshly
minted token is never the current request's token.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martsokha
martsokha merged commit 1e317cc into main Jul 29, 2026
7 checks passed
@martsokha
martsokha deleted the refactor/handler-cleanups branch July 29, 2026 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@martsokha martsokha

Labels

nats messaging, job queues, object storage refactor code restructuring without behavior change server API handlers, middleware, auth

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@martsokha