Hardening pass + 4 capability additions (review-driven)#1
Merged
Conversation
The GET /api/agent-actions list endpoint loaded the entire action_requests table into memory with findAll(), then filtered, sorted, and paginated in the application layer. This is inconsistent with the audit and approval endpoints (which already use Specifications) and does not survive real data volume. Introduce ActionRequestSpecifications and have the repository extend JpaSpecificationExecutor so filtering, ordering, and pagination are all pushed to the database. Existing ActionIngestionIT coverage (adminCanListActions, filterByStatus) exercises the new path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The policy evaluator sits on the action-ingestion hot path yet did two wasteful things per request: it recompiled every resource-pattern and condition regex from scratch, and it fired one findByPolicyId query per enabled policy (an N+1). Cache compiled Patterns by their source string in both PolicyEvaluator and ConditionEvaluator, and batch-load all conditions for the enabled policy set with a single findByPolicyIdIn, grouping them in memory. Behaviour is unchanged; the existing 35 policy tests still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
markUsed() previously did a findById + setter + save on every action submission. Because Agent extends the @Version-locked BaseEntity, that read-modify-write put the hottest write in the system on the optimistic- locking path, so two actions from the same agent could collide on the version column. Replace it with a @Modifying UPDATE that sets last_used_at (and updated_at) directly by id, leaving the version column untouched. The query flushes and clears automatically so no stale managed instance lingers in the persistence context. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Kafka events and RabbitMQ tasks were published inside the ingestion and approval transactions. A rollback after a send left the broker holding an event describing state that never became durable — a classic dual-write hazard. Introduce internal Spring application events (ActionReceived/Decided, ApprovalRequested, ActionCompleted) that the services publish in-band. A new TransactionalMessagingForwarder relays them to the existing publishers under @TransactionalEventListener(AFTER_COMMIT), so nothing reaches a broker unless the transaction commits. publishReceived now stamps the RECEIVED status as a constant rather than reading the entity (whose status is mutated later in the same transaction), keeping the deferred payload correct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The EXPIRED approval status, the APPROVAL_EXPIRED audit event, and the dashboard's EXPIRED filter were all defined but unreachable: nothing ever transitioned a past-deadline PENDING request, so it stayed PENDING forever. Add ApprovalExpiryService with a @scheduled sweep (interval configurable via agentops.approvals.expiry.*) that moves due PENDING approvals to EXPIRED and fails the underlying action closed (DENIED with an explanatory reason), emits the APPROVAL_EXPIRED audit event, and fans out the completion to the brokers. Scheduling is enabled outside the test profile; the worker is directly callable so ApprovalExpiryServiceIT verifies it without waiting on the clock. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The action lifecycle stopped at ALLOWED/APPROVED — the firewall cleared an
action but never learned whether the agent actually ran it, leaving the
ACTION_COMPLETED audit event and completed Kafka topic underused for
directly-allowed actions.
Add POST /api/agent-actions/{id}/complete: the agent reports success or
failure and the action moves to the new COMPLETED / FAILED terminal state.
Authentication reuses X-Agent-Key, verified against the action's own agent
via a new authenticate(agentId, key) overload, so only the owning agent can
close an action. Only ALLOWED/APPROVED actions are completable (else 409);
unknown ids 404, wrong keys 401. The completion is audited and fanned out
to Kafka (approval-less, so no Rabbit notification).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Neither the login endpoint nor the agent-action ingestion path had any throttling, so credential brute-force and request floods were unbounded. Add a dependency-free fixed-window limiter (FixedWindowRateLimiter) and a RateLimitFilter that runs ahead of the security chain: POST /api/auth/login is limited per client IP, and agent submit/complete per hashed agent key (IP fallback). Over-limit requests get a uniform 429 ApiError with a Retry-After header before any auth or DB work. Limits are configurable under agentops.security.rate-limit.* and disabled in the test profile so the suite stays deterministic; a dedicated RateLimitFilterIT re-enables it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Kafka topics and RabbitMQ queues were write-only — nothing read them, so the "event stream + work queue" architecture had no consumer side. Add a Kafka @KafkaListener over the received/decided/completed topics and a RabbitMQ @RabbitListener over the approval-request/notification queues, both projecting events into a new in-memory LiveMetricsService (per-type counters, a bounded recent-events buffer, and a subscriber hook the SSE feed will use). Consumers are guarded by agentops.messaging.consumers.enabled (and, for Rabbit, the presence of a RabbitTemplate) and disabled in the test profile so no broker connection is attempted; the handlers are unit-tested directly. Note: the live broker round-trip requires the docker stack and is not exercised by the test suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The dashboard and approval inbox only refreshed on user action, and the newly-added broker consumers had no outward surface. Add a live feed so consumed events reach the UI without polling. Backend: GET /api/stream is an SSE endpoint that sends an initial snapshot (counters + recent buffer) then an "activity" event per consumed broker event, sourced from LiveMetricsService. Because EventSource cannot set an Authorization header, JwtAuthenticationFilter also accepts the JWT as an access_token query parameter, scoped to the stream path. Frontend: EventStreamService wraps EventSource (token via query param) and exposes events$/snapshot$/connected; the approval inbox connects on init, live-reloads when an approval task flows through the queue, and shows a Live/Offline badge. Verified: backend IT (auth + async-start), 5 new Karma specs, production build. Live end-to-end behaviour needs the docker brokers + both apps running and is not exercised by the test suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
POLICY_UPDATED audit entries only noted that a policy changed, not what
changed — an admin auditing a policy edit could not see the before/after.
Snapshot the policy's mutable fields before applying an update and record a
field-level diff ({field: {from, to}}) plus a conditionsReplaced flag in the
audit details, including only fields that actually changed. Unchanged fields
are omitted. Verified by a new PolicyControllerIT case.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reflect the hardening/feature work across the reference docs: - docs/api.md — action-completion callback, SSE stream endpoint, a Rate limiting section, the 429 status, approval-expiry behaviour, and an explicit note on the stateless-JWT / localStorage tradeoff. - docs/architecture.md — after-commit publishing, the Kafka/Rabbit consumers feeding LiveMetricsService, the SSE live stream, the approval expiry sweeper, and the completion lifecycle. - README.md — refreshed feature summary and test counts (147 backend, 57 Karma). - CHANGELOG.md — Unreleased section covering all ten changes. 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.
Implements every issue and feature surfaced by the deep code review, one focused commit each (11 total).
Fixes / hardening
GET /api/agent-actionsnow filters/sorts/paginates in the DB via a JPASpecification(was loading the whole table into memory).findByPolicyIdIn) — removes per-request recompilation and an N+1.lastUsedAtwith a targeted@Modifyingupdate, off the optimistic-lock path.@TransactionalEventListener(AFTER_COMMIT)— no more phantom events on rollback.Features
PENDINGpast deadline →EXPIRED, action failed closed toDENIED,APPROVAL_EXPIREDaudit event (wires up previously-dead status/audit/filter).POST /api/agent-actions/{id}/complete— agent reports outcome →COMPLETED/FAILED.429+Retry-After.LiveMetricsService(topics/queues finally have a reader).GET /api/stream; Angular approval inbox live-reloads with a Live/Offline badge.POLICY_UPDATEDaudit entries.Verification
mvnw verify: 147 tests green (51 unit + 96 integration, H2).Caveats (as flagged during planning): the live Kafka/RabbitMQ broker round-trip (commit 8) and live end-to-end SSE (commit 9) require the docker stack + both apps running and are not exercised by the automated suites — those verify compilation, conditional guards, unit/handler logic, and auth.
🤖 Generated with Claude Code