Fix agent delivery, audit integrity, and governed synthetic cleanup - #41
Closed
jusso-dev wants to merge 11 commits into
Closed
Fix agent delivery, audit integrity, and governed synthetic cleanup#41jusso-dev wants to merge 11 commits into
jusso-dev wants to merge 11 commits into
Conversation
jusso-dev
marked this pull request as ready for review
July 27, 2026 05:16
📝 WalkthroughWalkthroughAdds gateway-authenticated, organisation-scoped agent direct-message invocation with terminal reply projection, a governed synthetic-cleanup maintenance workflow (manifest capture/approve/apply/retry with object-storage deletion), archived-record filtering, structured audit-chain integrity verification, an evidence object-storage adapter, centralized demo/seed identifiers, and homelab configuration/docs/scripts. ChangesAgent Direct-Message Invocation and Reply Flow
Estimated code review effort: 5 (Critical) | ~120 minutes Governed Synthetic Cleanup Maintenance Workflow
Estimated code review effort: 5 (Critical) | ~120 minutes Archived-record Visibility Filtering
Estimated code review effort: 2 (Simple) | ~15 minutes Audit-Chain Integrity Verification
Estimated code review effort: 4 (Complex) | ~60 minutes Demo and Seed Data ID Centralization
Estimated code review effort: 3 (Moderate) | ~25 minutes Homelab Configuration, Docs, and Migration Snapshots
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Human
participant RoomMessagesRoute
participant AgentDirectMessageDomainService
participant DurableAgentRuntime
participant RoomReply
Human->>RoomMessagesRoute: POST direct-room message
RoomMessagesRoute->>AgentDirectMessageDomainService: maybeQueue(message)
AgentDirectMessageDomainService->>AgentDirectMessageDomainService: check capability, kill switch, allowed room
AgentDirectMessageDomainService->>DurableAgentRuntime: enqueue agentRun (idempotent)
DurableAgentRuntime->>DurableAgentRuntime: claim + execute
DurableAgentRuntime->>RoomReply: projectDirectMessageTerminalReply
RoomReply-->>Human: agent-status reply message
sequenceDiagram
participant Operator
participant SyntheticCleanupDomainService
participant SyntheticCleanupDB
participant Worker
participant ObjectStorage
Operator->>SyntheticCleanupDomainService: execute(capture)
SyntheticCleanupDomainService->>SyntheticCleanupDB: captureSyntheticCleanupManifest
Operator->>SyntheticCleanupDomainService: execute(apply, approved manifest)
SyntheticCleanupDomainService->>SyntheticCleanupDB: applySyntheticCleanup
SyntheticCleanupDB-->>Worker: enqueue object_delete job
Worker->>ObjectStorage: verify + deleteObject
Worker->>SyntheticCleanupDB: recordSyntheticCleanupObjectDeletionAttempt
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (16)
apps/web/lib/archive-visibility.test.ts (1)
26-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMake this a behavior-level regression test.
The substring check does not prove the normal collection query applies the archive predicate. Add focused route/domain tests with active and archived fixtures, asserting only active records are returned.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/archive-visibility.test.ts` around lines 26 - 28, Replace the source-string assertion in the collectionQueries test with focused route/domain behavior tests using both active and archived fixtures. Execute each normal collection query through its public path and assert that only active records are returned, covering the archive predicate rather than merely checking for isNull syntax.apps/web/lib/demo-data.ts (1)
1-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the UUID namespaces in one shared module.
These independently maintained maps must remain identical for web demo references to resolve to seeded database records. A later change in only one copy can produce unseeded or mismatched room, actor, or message IDs.
apps/web/lib/demo-data.ts#L1-L62: import shared ID constants instead of declaring local copies.packages/database/src/seed-data.ts#L53-L116: move/re-export the canonical maps from a dependency-neutral shared module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/demo-data.ts` around lines 1 - 62, Centralize the canonical starterIds and demoIds maps in a dependency-neutral shared module, then update packages/database/src/seed-data.ts lines 53-116 to move or re-export those maps and apps/web/lib/demo-data.ts lines 1-62 to import them instead of declaring local copies. Preserve every existing UUID and map shape so web references continue resolving to the seeded database records..env.example (1)
33-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the dotenv-linter ordering warnings in both environment examples.
.env.example#L33-L35: moveMUSTER_AGENT_GATEWAY_TOKENbeforeMUSTER_BASE_URLandMUSTER_RESEARCH_ALLOWED_FEED_ORIGINSbeforeNODE_ENV.deploy/docker/.env.homelab.example#L14-L19: reorder the token entries and placeMUSTER_RESEARCH_ALLOWED_FEED_ORIGINSbeforeMUSTER_VERSIONas reported by dotenv-linter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.example around lines 33 - 35, Reorder the environment variable entries to satisfy dotenv-linter: in .env.example, place MUSTER_AGENT_GATEWAY_TOKEN before MUSTER_BASE_URL and MUSTER_RESEARCH_ALLOWED_FEED_ORIGINS before NODE_ENV; in deploy/docker/.env.homelab.example, reorder the token entries as reported and place MUSTER_RESEARCH_ALLOWED_FEED_ORIGINS before MUSTER_VERSION, without changing values.Source: Linters/SAST tools
apps/web/lib/agent-direct-message-domain.ts (1)
240-245: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRedact
traceIdbefore persisting it inrequest.
traceIdis caller-supplied (request header) and is redacted for the audit and outbox writes at Lines 307 and 317, but stored raw here.agent-learning-domain.tslater reads this value back out and redacts it, confirming it is not trusted.♻️ Proposed change
request: { kind: "direct_message", sourceMessageId: input.messageId, humanRequest: source.plainText, - traceId, + traceId: redactObservationText(traceId), },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/agent-direct-message-domain.ts` around lines 240 - 245, Update the request persistence payload in the direct-message flow around the request object to store a redacted traceId, matching the redaction already used for the audit and outbox writes. Ensure agent-learning-domain receives only the redacted value while preserving the existing request fields and behavior.apps/web/lib/agent-direct-message-domain.integration.test.ts (2)
192-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the mutated agent definition in a
finally.If either assertion fails,
allowedRoomsstays[](orkillSwitchstaystrue) for the remainder of the suite, cascading unrelated failures. Wrap the mutations so the restore at Lines 219-222 always runs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/agent-direct-message-domain.integration.test.ts` around lines 192 - 223, Update the “honours the kill switch and allowed-room boundary” test to wrap its agent-definition mutations and assertions in a try/finally block. Keep the final restoration of killSwitch and allowedRooms in the finally block so it runs even when either assertion or setup step fails, preserving the original agent definition for the rest of the suite.
100-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFixtures are never cleaned up.
afterAll(closeDatabase)leaves the synthetic actor, room, memberships, agent definition, and every source/reply message behind in the integration database, unlikeapps/worker/src/research-status.test.tswhich deletes its fixtures. Repeated runs accumulate rows that then have to be handled by the synthetic-cleanup workflow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/agent-direct-message-domain.integration.test.ts` around lines 100 - 102, Update the integration test teardown around afterAll(closeDatabase) to explicitly delete all fixtures created by the test, including the synthetic actor, room, memberships, agent definition, and source/reply messages, before closing the database. Follow the cleanup pattern used by the research-status test while preserving database closure after fixture removal.apps/worker/src/research-status.test.ts (1)
26-27: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
database()runs during collection even when the suite is skipped.The
describecallback body executes at collection time, sodatabase()is invoked on non-integration runs whereafterAll/closeDatabasenever fires. Move it intobeforeAll(as the other suites do) to avoid a stray pool/open handle.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/worker/src/research-status.test.ts` around lines 26 - 27, Move the database() invocation out of the describeIntegration callback’s collection-time setup and initialize it within beforeAll instead. Update the research terminal status persistence suite to retain the database reference for its tests, while ensuring closeDatabase in afterAll receives the initialized instance during integration runs only.apps/web/lib/agent-learning-domain.ts (2)
822-864: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffSequential per-run lookups extend the kill-switch transaction.
Each cancelled run issues its own three-table
selectinside the transaction that already holds row locks onagent_definitionsand every cancelledagent_runsrow. A kill switch flipped on an agent with many queued/running runs serialises that many round trips before commit. Consider resolving the eligible source messages in one batched query keyed by(roomId, sourceMessageId)before the insert loop.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/agent-learning-domain.ts` around lines 822 - 864, Batch-resolve eligible source messages for all parsed cancelled runs with one query keyed by each run’s roomId and sourceMessageId, preserving the existing organisation, direct-room, active-membership, non-deleted, and source-message constraints. Replace the per-run three-table select inside the cancelledRuns loop with lookups into the batched results, while keeping runs without a valid source message skipped before insertion.
823-829: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
z.uuid()herez.string().uuid()is deprecated in Zod 4; switch this UUID field to the top-level schema.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/agent-learning-domain.ts` around lines 823 - 829, Update the direct_message request schema’s sourceMessageId field in the safeParse flow to use Zod 4’s top-level z.uuid() validator instead of the deprecated z.string().uuid() chain, leaving the other request fields unchanged.apps/agent-gateway/src/service-auth.ts (1)
16-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider the stricter top-level
z.uuid()for consistency.This uses the deprecated chained
z.string().uuid()form, whileruntime.tsuses the newer top-levelz.uuid()for equivalent id fields (huntId,sourceMessageId). Per Zod 4's migration guide,z.uuid()enforces RFC 9562/4122 version/variant bits more strictly thanz.string().uuid(). Worth aligning on one validator across the gateway for consistent UUID acceptance semantics.♻️ Suggested alignment
- const parsed = z.string().uuid().safeParse(value); + const parsed = z.uuid().safeParse(value);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/agent-gateway/src/service-auth.ts` around lines 16 - 21, Update parseGatewayOrganisationId to use the top-level z.uuid() validator instead of the deprecated z.string().uuid() chain, preserving the existing safeParse handling and null result for invalid values.packages/database/migrations/0019_fuzzy_zemo.sql (1)
35-35: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winProvenance lookups can't use this index effectively.
loadCandidateRowsqueriessynthetic_artifact_provenanceby(organisation_id, artifact_id IN (...))withoutartifact_table, so only theorganisation_idprefix ofsynthetic_artifact_provenance_artifact_uniqueis usable and the rest degrades to a filter/scan underFOR UPDATE. Consider adding a supporting index on("organisation_id","artifact_id").⚡ Proposed index
CREATE UNIQUE INDEX "synthetic_artifact_provenance_artifact_unique" ON "synthetic_artifact_provenance" USING btree ("organisation_id","artifact_table","artifact_id");--> statement-breakpoint +CREATE INDEX "synthetic_artifact_provenance_org_artifact_idx" ON "synthetic_artifact_provenance" USING btree ("organisation_id","artifact_id");--> statement-breakpoint🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/database/migrations/0019_fuzzy_zemo.sql` at line 35, Update the migration’s synthetic_artifact_provenance indexes to add a supporting non-unique index on organisation_id and artifact_id, matching the lookup shape used by loadCandidateRows while retaining the existing uniqueness constraint.packages/database/src/synthetic-cleanup.integration.test.ts (1)
803-813: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded manifest id couples this test to the previous one.
"019fa301-0000-7000-8000-000000000003"silently duplicatesbasePlan("3"). Derive it from the same helper (or hoist the captured manifest) so a change to the suffix scheme fails loudly rather than yielding an empty receipt and a confusingparseSyntheticCleanupManifest(undefined)error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/database/src/synthetic-cleanup.integration.test.ts` around lines 803 - 813, Update the test around “requires a fresh independent approval to reconcile a pending object” to derive the manifestId from the existing basePlan("3") helper or reuse the previously captured manifest instead of hardcoding the UUID. Preserve the receipt lookup and parsing flow while ensuring changes to the suffix scheme cannot silently produce an empty receipt.apps/web/lib/synthetic-cleanup-domain.ts (1)
82-97: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winManifest is canonicalised and re-hashed on every hop.
parseSyntheticCleanupManifestruns here, then again insidefindSyntheticCleanupReceipt,applySyntheticCleanup, andlistSyntheticCleanupObjectDeletionAttempts. WithselectionEvidenceallowed up to 110k entries andobjectStorageObjectsup to 10k, that is four full Zod parses plus four SHA-256 canonicalisations of the same payload on the request thread. Consider letting the already-parsed manifest short-circuit the re-parse (e.g. an internal accepting the validated type) for the apply path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/synthetic-cleanup-domain.ts` around lines 82 - 97, Avoid repeatedly parsing and hashing the same manifest in the apply flow by passing the already-validated manifest from this handler into findSyntheticCleanupReceipt, applySyntheticCleanup, and listSyntheticCleanupObjectDeletionAttempts through internal APIs that accept the validated type. Preserve boundary validation for untrusted entry points and keep verify/request_approval behavior unchanged.packages/database/src/synthetic-cleanup.ts (1)
97-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
z.iso.datetime({ offset: true }).z.string().datetime()is deprecated on Zod 4, so switching keeps this schema on the current API.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/database/src/synthetic-cleanup.ts` at line 97, Update the generatedAt schema field to use Zod 4’s z.iso.datetime({ offset: true }) API instead of the deprecated z.string().datetime({ offset: true }) call, preserving the existing offset validation behavior.packages/evidence/src/object-storage.test.ts (1)
1-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering headObject/deleteObject edge cases too.
Only
getObjectVersion's happy path is tested here. GivenheadObject's null-on-404 branch anddeleteObject's 404-tolerance both gate the cleanup deletion safety logic inapps/worker/src/synthetic-cleanup-object.ts, a couple more cases (404 → null, non-404 error → throw) would meaningfully increase confidence in this critical path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evidence/src/object-storage.test.ts` around lines 1 - 36, Extend the “versioned object storage” tests for defaultObjectStorage.headObject and deleteObject: verify headObject returns null on a 404 and rethrows non-404 failures, and verify deleteObject tolerates 404 responses while propagating other errors. Reuse the existing fetch stubbing and cleanup pattern without changing production behavior.packages/evidence/src/object-storage.ts (1)
32-110: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftConsider a maintained S3 client instead of hand-rolled SigV4.
This is a from-scratch AWS SigV4 signer (canonical request, scope derivation, HMAC chain). AWS's own docs explicitly recommend always using an SDK "unless you have a good reason not to," since subtle canonicalization/encoding mistakes silently produce signature-mismatch failures or, worse, requests that verify incorrectly. Given this now guards a destructive delete path, a well-tested client (e.g.
@aws-sdk/client-s3or a minimal S3-compatible library) would reduce correctness/security risk versus maintaining custom signing code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evidence/src/object-storage.ts` around lines 32 - 110, Replace the hand-rolled SigV4 helpers sha256, hmac, objectUrl, and signingHeaders with a maintained S3-compatible client, preferably `@aws-sdk/client-s3`, and route the destructive object-storage operations through its authenticated request methods. Preserve the existing endpoint, bucket, key, credentials, region, and HTTP operation behavior, including S3-compatible custom endpoints.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/api/v1/maintenance/synthetic-cleanup/route.ts`:
- Line 8: In the synthetic-cleanup route handler, add a Content-Length
validation before the request.json() call, rejecting requests that exceed the
endpoint’s defined maximum body size with the existing client-error response
pattern. Keep JSON parsing unchanged for requests within the cap, and account
for missing or invalid Content-Length headers without bypassing the limit
policy.
In `@apps/web/app/api/v1/rooms/`[id]/messages/route.ts:
- Around line 60-72: Refactor postMessage so
AgentDirectMessageDomainService.maybeQueue is not invoked inline after the
message commit. Instead, include the agent-invocation request in the message
outbox event and have the outbox consumer drive maybeQueue
transactionally/reliably, ensuring persisted messages are eventually queued
without relying on client retries.
In `@apps/web/lib/agent-direct-message-domain.ts`:
- Around line 131-173: Move requireCapability(subject, "agents.invoke") to
before the active-agent count and availability checks in the direct-message
eligibility flow. Ensure unauthorized subjects are rejected before any
agent_count or agent_unavailable outcome is returned, while preserving the
existing eligibility logic for authorized subjects.
In `@apps/web/lib/connector-domain.ts`:
- Around line 46-49: Update the existing-record reactivation flow in configure()
to explicitly clear archivedAt when reusing a matching archived integration
record, so the reconfigured connector is returned by list(); alternatively,
reject archived records and create a new record if that matches the retention
policy.
In `@apps/worker/src/index.ts`:
- Around line 162-167: Update the worker dispatch request in the visible fetch
call to include the required x-muster-organisation-id header alongside
authorization. Populate it from the existing organisation identifier used by
this dispatch flow, ensuring all non-report jobs, including direct-message runs,
send the organisation header to the gateway.
In `@apps/worker/src/research-config.test.ts`:
- Around line 12-18: Update the test case around the compose configuration
assertion to inspect the web and worker service blocks specifically, verifying
each contains MUSTER_RESEARCH_ALLOWED_FEED_ORIGINS and that unrelated services
do not. Replace the global toHaveLength(2) text-count assertion with
service-scoped checks while preserving the expected environment mapping.
In `@apps/worker/src/research-status.ts`:
- Around line 26-56: Update the query constructing run in the research-status
write path to require an unarchived room and verify that run.agentId is still an
authorized member of that room before posting. Reuse the existing room archival
and agent-membership checks from agent-direct-message-domain.ts, preserving the
current organization, run, and expectedStatus filters.
In `@docker-compose.yml`:
- Line 32: Update the MUSTER_AGENT_GATEWAY_TOKEN configuration in
docker-compose.yml to remove the repository-known fallback value. Require an
explicitly provided token or generate a secure token before startup, ensuring
the gateway never starts with a predictable credential when the variable is
unset.
In `@packages/audit/src/index.ts`:
- Around line 34-38: Update the legacyApprovalIdActions allow-list to include
integration.action.failed so failed events with metadata.approvalId map to
legacy-compatible-not-strict. Add a regression test covering the failed-action
event and its expected classification.
In `@packages/database/src/verify-audit-integrity.ts`:
- Around line 5-9: Validate the format of MUSTER_AUDIT_ORGANISATION_ID
immediately after the existing presence check and before any audit query,
rejecting malformed identifiers with a clear error. Reuse the project’s
established organisation-ID validation mechanism if available, while preserving
the existing required-environment-variable error.
In `@packages/evidence/src/object-storage.ts`:
- Around line 112-120: Update storageConfiguration() to remove the hardcoded
defaults for accessKey and secretKey, requiring OBJECT_STORAGE_ACCESS_KEY and
OBJECT_STORAGE_SECRET_KEY to be explicitly configured and failing fast when
either is missing. Preserve the existing defaults for endpoint, bucket, and
region.
- Around line 122-267: Update every fetch call in
defaultObjectStorage—putObject, getObject, headObject, getObjectVersion, and
deleteObject—to include the established AbortSignal.timeout(10_000) signal
option, matching the existing external-call timeout pattern. Preserve each
method’s current request and response handling.
---
Nitpick comments:
In @.env.example:
- Around line 33-35: Reorder the environment variable entries to satisfy
dotenv-linter: in .env.example, place MUSTER_AGENT_GATEWAY_TOKEN before
MUSTER_BASE_URL and MUSTER_RESEARCH_ALLOWED_FEED_ORIGINS before NODE_ENV; in
deploy/docker/.env.homelab.example, reorder the token entries as reported and
place MUSTER_RESEARCH_ALLOWED_FEED_ORIGINS before MUSTER_VERSION, without
changing values.
In `@apps/agent-gateway/src/service-auth.ts`:
- Around line 16-21: Update parseGatewayOrganisationId to use the top-level
z.uuid() validator instead of the deprecated z.string().uuid() chain, preserving
the existing safeParse handling and null result for invalid values.
In `@apps/web/lib/agent-direct-message-domain.integration.test.ts`:
- Around line 192-223: Update the “honours the kill switch and allowed-room
boundary” test to wrap its agent-definition mutations and assertions in a
try/finally block. Keep the final restoration of killSwitch and allowedRooms in
the finally block so it runs even when either assertion or setup step fails,
preserving the original agent definition for the rest of the suite.
- Around line 100-102: Update the integration test teardown around
afterAll(closeDatabase) to explicitly delete all fixtures created by the test,
including the synthetic actor, room, memberships, agent definition, and
source/reply messages, before closing the database. Follow the cleanup pattern
used by the research-status test while preserving database closure after fixture
removal.
In `@apps/web/lib/agent-direct-message-domain.ts`:
- Around line 240-245: Update the request persistence payload in the
direct-message flow around the request object to store a redacted traceId,
matching the redaction already used for the audit and outbox writes. Ensure
agent-learning-domain receives only the redacted value while preserving the
existing request fields and behavior.
In `@apps/web/lib/agent-learning-domain.ts`:
- Around line 822-864: Batch-resolve eligible source messages for all parsed
cancelled runs with one query keyed by each run’s roomId and sourceMessageId,
preserving the existing organisation, direct-room, active-membership,
non-deleted, and source-message constraints. Replace the per-run three-table
select inside the cancelledRuns loop with lookups into the batched results,
while keeping runs without a valid source message skipped before insertion.
- Around line 823-829: Update the direct_message request schema’s
sourceMessageId field in the safeParse flow to use Zod 4’s top-level z.uuid()
validator instead of the deprecated z.string().uuid() chain, leaving the other
request fields unchanged.
In `@apps/web/lib/archive-visibility.test.ts`:
- Around line 26-28: Replace the source-string assertion in the
collectionQueries test with focused route/domain behavior tests using both
active and archived fixtures. Execute each normal collection query through its
public path and assert that only active records are returned, covering the
archive predicate rather than merely checking for isNull syntax.
In `@apps/web/lib/demo-data.ts`:
- Around line 1-62: Centralize the canonical starterIds and demoIds maps in a
dependency-neutral shared module, then update packages/database/src/seed-data.ts
lines 53-116 to move or re-export those maps and apps/web/lib/demo-data.ts lines
1-62 to import them instead of declaring local copies. Preserve every existing
UUID and map shape so web references continue resolving to the seeded database
records.
In `@apps/web/lib/synthetic-cleanup-domain.ts`:
- Around line 82-97: Avoid repeatedly parsing and hashing the same manifest in
the apply flow by passing the already-validated manifest from this handler into
findSyntheticCleanupReceipt, applySyntheticCleanup, and
listSyntheticCleanupObjectDeletionAttempts through internal APIs that accept the
validated type. Preserve boundary validation for untrusted entry points and keep
verify/request_approval behavior unchanged.
In `@apps/worker/src/research-status.test.ts`:
- Around line 26-27: Move the database() invocation out of the
describeIntegration callback’s collection-time setup and initialize it within
beforeAll instead. Update the research terminal status persistence suite to
retain the database reference for its tests, while ensuring closeDatabase in
afterAll receives the initialized instance during integration runs only.
In `@packages/database/migrations/0019_fuzzy_zemo.sql`:
- Line 35: Update the migration’s synthetic_artifact_provenance indexes to add a
supporting non-unique index on organisation_id and artifact_id, matching the
lookup shape used by loadCandidateRows while retaining the existing uniqueness
constraint.
In `@packages/database/src/synthetic-cleanup.integration.test.ts`:
- Around line 803-813: Update the test around “requires a fresh independent
approval to reconcile a pending object” to derive the manifestId from the
existing basePlan("3") helper or reuse the previously captured manifest instead
of hardcoding the UUID. Preserve the receipt lookup and parsing flow while
ensuring changes to the suffix scheme cannot silently produce an empty receipt.
In `@packages/database/src/synthetic-cleanup.ts`:
- Line 97: Update the generatedAt schema field to use Zod 4’s z.iso.datetime({
offset: true }) API instead of the deprecated z.string().datetime({ offset: true
}) call, preserving the existing offset validation behavior.
In `@packages/evidence/src/object-storage.test.ts`:
- Around line 1-36: Extend the “versioned object storage” tests for
defaultObjectStorage.headObject and deleteObject: verify headObject returns null
on a 404 and rethrows non-404 failures, and verify deleteObject tolerates 404
responses while propagating other errors. Reuse the existing fetch stubbing and
cleanup pattern without changing production behavior.
In `@packages/evidence/src/object-storage.ts`:
- Around line 32-110: Replace the hand-rolled SigV4 helpers sha256, hmac,
objectUrl, and signingHeaders with a maintained S3-compatible client, preferably
`@aws-sdk/client-s3`, and route the destructive object-storage operations through
its authenticated request methods. Preserve the existing endpoint, bucket, key,
credentials, region, and HTTP operation behavior, including S3-compatible custom
endpoints.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: da8e70e6-c4f2-49b6-b826-29a714a1267d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (72)
.env.exampleREADME.mdapps/agent-gateway/src/index.tsapps/agent-gateway/src/runtime.integration.test.tsapps/agent-gateway/src/runtime.tsapps/agent-gateway/src/service-auth.test.tsapps/agent-gateway/src/service-auth.tsapps/web/app/api/v1/agent-runs/[id]/route.tsapps/web/app/api/v1/agents/[id]/learning/route.tsapps/web/app/api/v1/hunts/route.tsapps/web/app/api/v1/maintenance/synthetic-cleanup/route.tsapps/web/app/api/v1/reports/route.tsapps/web/app/api/v1/reports/schedules/route.tsapps/web/app/api/v1/rooms/[id]/messages/route.tsapps/web/app/api/v1/tasks/[id]/cancel/route.tsapps/web/app/api/v1/tasks/[id]/events/route.tsapps/web/app/api/v1/tasks/route.tsapps/web/lib/agent-direct-message-domain.integration.test.tsapps/web/lib/agent-direct-message-domain.tsapps/web/lib/agent-gateway.tsapps/web/lib/agent-learning-domain.integration.test.tsapps/web/lib/agent-learning-domain.tsapps/web/lib/alfie-research-domain.tsapps/web/lib/archive-visibility.test.tsapps/web/lib/connector-domain.tsapps/web/lib/demo-data.tsapps/web/lib/object-storage.tsapps/web/lib/synthetic-cleanup-domain.test.tsapps/web/lib/synthetic-cleanup-domain.tsapps/worker/package.jsonapps/worker/src/index.tsapps/worker/src/research-config.test.tsapps/worker/src/research-status.test.tsapps/worker/src/research-status.tsapps/worker/src/synthetic-cleanup-object.test.tsapps/worker/src/synthetic-cleanup-object.tsdeploy/docker/.env.homelab.exampledeploy/docker/README.mddeploy/docker/docker-compose.homelab.ymldocker-compose.ymldocs/operations/alfie-research.mddocs/operations/backup-restore.mddocs/operations/incident-recovery.mddocs/synthetic-cleanup.mdpackage.jsonpackages/audit/src/audit.test.tspackages/audit/src/index.tspackages/authz/src/index.tspackages/database/migrations/0017_daily_dexter_bennett.sqlpackages/database/migrations/0018_watery_morg.sqlpackages/database/migrations/0019_fuzzy_zemo.sqlpackages/database/migrations/meta/0017_snapshot.jsonpackages/database/migrations/meta/0018_snapshot.jsonpackages/database/migrations/meta/0019_snapshot.jsonpackages/database/migrations/meta/_journal.jsonpackages/database/package.jsonpackages/database/src/domain-transaction.tspackages/database/src/index.tspackages/database/src/schema.tspackages/database/src/seed-data.test.tspackages/database/src/seed-data.tspackages/database/src/seed.tspackages/database/src/synthetic-cleanup.integration.test.tspackages/database/src/synthetic-cleanup.test.tspackages/database/src/synthetic-cleanup.tspackages/database/src/verify-audit-integrity.tspackages/evidence/src/index.tspackages/evidence/src/object-storage.test.tspackages/evidence/src/object-storage.tsplaywright.config.tsscripts/install-homelab.shscripts/test-install-homelab.sh
| const traceId = requestTraceId(request); | ||
| try { | ||
| const subject = await apiSubject(request); | ||
| const body = await request.json(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how problemResponse classifies non-domain errors (e.g. SyntaxError)
fd -t f 'api-context.ts' apps/web/lib --exec cat -n {}Repository: jusso-dev/Muster
Length of output: 3718
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the maintenance synthetic cleanup route and related request handling.
route_file="apps/web/app/api/v1/maintenance/synthetic-cleanup/route.ts"
echo "## $route_file"
cat -n "$route_file"
echo
echo "## search for problemResponse usage"
rg -n "problemResponse\\(" apps/web -g '!**/node_modules/**'
echo
echo "## search for request\\.json or content-length guards in api routes"
rg -n "content-length|request\\.json\\(|arrayBuffer\\(|text\\(" apps/web/app/api/v1 -g '!**/node_modules/**'Repository: jusso-dev/Muster
Length of output: 13875
🏁 Script executed:
#!/bin/bash
set -euo pipefail
route_file="apps/web/app/api/v1/maintenance/synthetic-cleanup/route.ts"
echo "## $route_file"
cat -n "$route_file"
echo
echo "## Any explicit request body limits in this route or nearby helpers"
rg -n "content-length|request\\.json\\(|problemResponse|synthetic-cleanup|manifest" apps/web/app/api/v1 apps/web/lib -g '!**/node_modules/**'Repository: jusso-dev/Muster
Length of output: 26887
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## locate synthetic cleanup manifest and parser"
rg -n "SyntheticCleanupManifest|parseSyntheticCleanupManifest|selection-evidence|objectStorageObjects|object records|evidence" apps/web/lib apps/web/app/api/v1/maintenance/synthetic-cleanup -g '!**/node_modules/**'
echo
echo "## inspect relevant synthetic cleanup domain sections"
file="apps/web/lib/synthetic-cleanup-domain.ts"
wc -l "$file"
sed -n '1,220p' "$file"
echo
echo "## inspect any manifest schema/test references"
rg -n "SyntheticCleanupManifest|selectionEvidence|objectStorageObjects" apps/web/lib -g '!**/node_modules/**'Repository: jusso-dev/Muster
Length of output: 23541
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## search for synthetic cleanup schema definitions"
rg -n "SyntheticCleanupPlanSchema|SyntheticCleanupManifestSchema|selectionEvidence|objectStorageObjects|parseSyntheticCleanupManifest|captureSyntheticCleanupManifest" . -g '!**/node_modules/**' -g '!**/.next/**'
echo
echo "## inspect any database schema file that defines synthetic cleanup"
fd -t f 'synthetic-cleanup' . -x sh -c 'echo "FILE: $1"; wc -l "$1"; sed -n "1,260p" "$1"' sh {}Repository: jusso-dev/Muster
Length of output: 50372
Add a request-body size cap before parsing. In apps/web/app/api/v1/maintenance/synthetic-cleanup/route.ts, request.json() buffers the full payload in memory, and this endpoint accepts large manifests, so a content-length ceiling would reduce the risk of oversized maintenance requests pinning the handler.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/api/v1/maintenance/synthetic-cleanup/route.ts` at line 8, In the
synthetic-cleanup route handler, add a Content-Length validation before the
request.json() call, rejecting requests that exceed the endpoint’s defined
maximum body size with the existing client-error response pattern. Keep JSON
parsing unchanged for requests within the cap, and account for missing or
invalid Content-Length headers without bypassing the limit policy.
Comment on lines
+60
to
+72
| try { | ||
| agentInvocation = await new AgentDirectMessageDomainService().maybeQueue( | ||
| subject, | ||
| { messageId: result.message.id, roomId: id }, | ||
| traceId, | ||
| ); | ||
| } catch (error) { | ||
| agentInvocationError = redactObservationText( | ||
| error instanceof Error | ||
| ? error.message | ||
| : "The direct-message agent could not be queued.", | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Invocation is not atomic with message creation.
postMessage commits first, then maybeQueue opens its own transaction. A crash in between leaves a persisted human message with no queued run and no outbox record; nothing re-drives it because recovery depends on the client re-posting the same idempotency key. Consider driving the invocation from the message outbox event instead of inline in the request path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/api/v1/rooms/`[id]/messages/route.ts around lines 60 - 72,
Refactor postMessage so AgentDirectMessageDomainService.maybeQueue is not
invoked inline after the message commit. Instead, include the agent-invocation
request in the message outbox event and have the outbox consumer drive
maybeQueue transactionally/reliably, ensuring persisted messages are eventually
queued without relying on client retries.
Comment on lines
+131
to
+173
| const active = members.filter( | ||
| (member) => | ||
| member.actorStatus === "active" && | ||
| member.definitionId !== null && | ||
| member.definitionStatus === "active" && | ||
| member.killSwitch === false, | ||
| ); | ||
| if (active.length !== 1) { | ||
| return { | ||
| handled: true, | ||
| queued: false, | ||
| duplicate: false, | ||
| reason: "agent_count", | ||
| }; | ||
| } | ||
| const agent = active[0]!; | ||
| if ( | ||
| !Array.isArray(agent.allowedRooms) || | ||
| !agent.allowedRooms.includes(input.roomId) || | ||
| !agent.definitionId || | ||
| !agent.runtime || | ||
| !agent.model || | ||
| !agent.promptVersion || | ||
| agent.maximumRuntimeSeconds === null || | ||
| agent.maximumTokenBudget === null || | ||
| agent.maximumCostCents === null | ||
| ) { | ||
| return { | ||
| handled: true, | ||
| queued: false, | ||
| duplicate: false, | ||
| reason: "agent_unavailable", | ||
| }; | ||
| } | ||
|
|
||
| const agentId = agent.definitionId; | ||
| const runtime = agent.runtime; | ||
| const model = agent.model; | ||
| const promptVersion = agent.promptVersion; | ||
| const maximumRuntimeSeconds = agent.maximumRuntimeSeconds; | ||
| const maximumTokenBudget = agent.maximumTokenBudget; | ||
| const maximumCostCents = agent.maximumCostCents; | ||
| requireCapability(subject, "agents.invoke"); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Enforce agents.invoke before returning eligibility outcomes.
requireCapability runs only after the agent-count/availability branches, so a subject without agents.invoke still receives agent_count / agent_unavailable, disclosing agent configuration state and altering the caller's Jessie path. Move the check ahead of the eligibility evaluation (it is cheap and has no dependency on the queried rows).
🔒 Proposed fix
if (!source) return null;
+ requireCapability(subject, "agents.invoke");
const members = await this.db const maximumCostCents = agent.maximumCostCents;
- requireCapability(subject, "agents.invoke");
const idempotencyKey = `agent-direct-message:${input.messageId}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const active = members.filter( | |
| (member) => | |
| member.actorStatus === "active" && | |
| member.definitionId !== null && | |
| member.definitionStatus === "active" && | |
| member.killSwitch === false, | |
| ); | |
| if (active.length !== 1) { | |
| return { | |
| handled: true, | |
| queued: false, | |
| duplicate: false, | |
| reason: "agent_count", | |
| }; | |
| } | |
| const agent = active[0]!; | |
| if ( | |
| !Array.isArray(agent.allowedRooms) || | |
| !agent.allowedRooms.includes(input.roomId) || | |
| !agent.definitionId || | |
| !agent.runtime || | |
| !agent.model || | |
| !agent.promptVersion || | |
| agent.maximumRuntimeSeconds === null || | |
| agent.maximumTokenBudget === null || | |
| agent.maximumCostCents === null | |
| ) { | |
| return { | |
| handled: true, | |
| queued: false, | |
| duplicate: false, | |
| reason: "agent_unavailable", | |
| }; | |
| } | |
| const agentId = agent.definitionId; | |
| const runtime = agent.runtime; | |
| const model = agent.model; | |
| const promptVersion = agent.promptVersion; | |
| const maximumRuntimeSeconds = agent.maximumRuntimeSeconds; | |
| const maximumTokenBudget = agent.maximumTokenBudget; | |
| const maximumCostCents = agent.maximumCostCents; | |
| requireCapability(subject, "agents.invoke"); | |
| requireCapability(subject, "agents.invoke"); | |
| const active = members.filter( | |
| (member) => | |
| member.actorStatus === "active" && | |
| member.definitionId !== null && | |
| member.definitionStatus === "active" && | |
| member.killSwitch === false, | |
| ); | |
| if (active.length !== 1) { | |
| return { | |
| handled: true, | |
| queued: false, | |
| duplicate: false, | |
| reason: "agent_count", | |
| }; | |
| } | |
| const agent = active[0]!; | |
| if ( | |
| !Array.isArray(agent.allowedRooms) || | |
| !agent.allowedRooms.includes(input.roomId) || | |
| !agent.definitionId || | |
| !agent.runtime || | |
| !agent.model || | |
| !agent.promptVersion || | |
| agent.maximumRuntimeSeconds === null || | |
| agent.maximumTokenBudget === null || | |
| agent.maximumCostCents === null | |
| ) { | |
| return { | |
| handled: true, | |
| queued: false, | |
| duplicate: false, | |
| reason: "agent_unavailable", | |
| }; | |
| } | |
| const agentId = agent.definitionId; | |
| const runtime = agent.runtime; | |
| const model = agent.model; | |
| const promptVersion = agent.promptVersion; | |
| const maximumRuntimeSeconds = agent.maximumRuntimeSeconds; | |
| const maximumTokenBudget = agent.maximumTokenBudget; | |
| const maximumCostCents = agent.maximumCostCents; | |
| const idempotencyKey = `agent-direct-message:${input.messageId}`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/agent-direct-message-domain.ts` around lines 131 - 173, Move
requireCapability(subject, "agents.invoke") to before the active-agent count and
availability checks in the direct-message eligibility flow. Ensure unauthorized
subjects are rejected before any agent_count or agent_unavailable outcome is
returned, while preserving the existing eligibility logic for authorized
subjects.
Comment on lines
+46
to
+49
| and( | ||
| eq(schema.integrationRecords.organisationId, subject.organisationId), | ||
| isNull(schema.integrationRecords.archivedAt), | ||
| ), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reconfiguring an archived connector leaves it permanently hidden.
configure() reuses a matching archived record but does not reset archivedAt; after this filter, a successful reconfiguration cannot appear in list(). Explicitly restore archivedAt when reactivating the existing record, or reject archived records and create a new one according to the intended retention policy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/connector-domain.ts` around lines 46 - 49, Update the
existing-record reactivation flow in configure() to explicitly clear archivedAt
when reusing a matching archived integration record, so the reconfigured
connector is returned by list(); alternatively, reject archived records and
create a new record if that matches the retention policy.
Comment on lines
+162
to
+167
| const gatewayToken = process.env.MUSTER_AGENT_GATEWAY_TOKEN?.trim(); | ||
| if (!gatewayToken) throw new Error("Agent gateway token is not configured"); | ||
| const response = await fetch( | ||
| `${process.env.AGENT_GATEWAY_URL ?? "http://agent-gateway:3002"}/v1/runs/dispatch`, | ||
| { | ||
| headers: { authorization: `Bearer ${gatewayToken}` }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Include x-muster-organisation-id in worker dispatches.
The gateway create/dispatch contract requires the organisation header, but this branch sends only Authorization. Non-report jobs on muster-agents, including direct-message runs, will be rejected before dispatch.
Proposed fix
- headers: { authorization: `Bearer ${gatewayToken}` },
+ headers: {
+ authorization: `Bearer ${gatewayToken}`,
+ "x-muster-organisation-id": job.data.organisationId,
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const gatewayToken = process.env.MUSTER_AGENT_GATEWAY_TOKEN?.trim(); | |
| if (!gatewayToken) throw new Error("Agent gateway token is not configured"); | |
| const response = await fetch( | |
| `${process.env.AGENT_GATEWAY_URL ?? "http://agent-gateway:3002"}/v1/runs/dispatch`, | |
| { | |
| headers: { authorization: `Bearer ${gatewayToken}` }, | |
| const gatewayToken = process.env.MUSTER_AGENT_GATEWAY_TOKEN?.trim(); | |
| if (!gatewayToken) throw new Error("Agent gateway token is not configured"); | |
| const response = await fetch( | |
| `${process.env.AGENT_GATEWAY_URL ?? "http://agent-gateway:3002"}/v1/runs/dispatch`, | |
| { | |
| headers: { | |
| authorization: `Bearer ${gatewayToken}`, | |
| "x-muster-organisation-id": job.data.organisationId, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/worker/src/index.ts` around lines 162 - 167, Update the worker dispatch
request in the visible fetch call to include the required
x-muster-organisation-id header alongside authorization. Populate it from the
existing organisation identifier used by this dispatch flow, ensuring all
non-report jobs, including direct-message runs, send the organisation header to
the gateway.
| MUSTER_MOCK_INTEGRATIONS: "true" | ||
| MUSTER_AGENT_RUNTIME: ${MUSTER_AGENT_RUNTIME:-codex} | ||
| AGENT_GATEWAY_URL: http://agent-gateway:3002 | ||
| MUSTER_AGENT_GATEWAY_TOKEN: ${MUSTER_AGENT_GATEWAY_TOKEN:-local-agent-gateway-token-change-me-32-characters} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove the repository-known bearer-token fallback.
Line 32 silently enables a publicly known gateway credential whenever the variable is unset, defeating the authenticated gateway boundary. Require an explicit token (or generate one before startup) instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.yml` at line 32, Update the MUSTER_AGENT_GATEWAY_TOKEN
configuration in docker-compose.yml to remove the repository-known fallback
value. Require an explicitly provided token or generate a secure token before
startup, ensuring the gateway never starts with a predictable credential when
the variable is unset.
Comment on lines
+34
to
+38
| const legacyApprovalIdActions = new Set([ | ||
| "integration.action.queued", | ||
| "integration.action.succeeded", | ||
| ]); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find every appendAuditEvent call whose metadata includes approvalId, to confirm
# whether other action types besides queued/succeeded/failed need the same allow-list entry.
rg -n -B4 -A1 'approvalId:\s*(metadata\.approvalId|undefined)' apps/worker/src apps/web/lib apps/agent-gateway/src packages/database/src --type ts -g '!**/*.test.ts'Repository: jusso-dev/Muster
Length of output: 1331
Include integration.action.failed in the legacy approvalId allow-list. Old failed integration events carry the same approvalId: metadata.approvalId shape as queued/succeeded, so they should also map to legacy-compatible-not-strict; otherwise they fall through to invalid. Add a regression test for the failed-action case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/audit/src/index.ts` around lines 34 - 38, Update the
legacyApprovalIdActions allow-list to include integration.action.failed so
failed events with metadata.approvalId map to legacy-compatible-not-strict. Add
a regression test covering the failed-action event and its expected
classification.
Comment on lines
+5
to
+9
| const organisationId = process.env.MUSTER_AUDIT_ORGANISATION_ID; | ||
|
|
||
| if (!organisationId) { | ||
| throw new Error("MUSTER_AUDIT_ORGANISATION_ID is required"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Consider validating MUSTER_AUDIT_ORGANISATION_ID format before querying.
A mistyped or non-existent organisation id currently returns zero events, which trivially reports outcome: "strict-valid" — a false assurance during an incident if the operator doesn't notice eventCount: 0 in the JSON output.
🛡️ Proposed guard
const organisationId = process.env.MUSTER_AUDIT_ORGANISATION_ID;
if (!organisationId) {
throw new Error("MUSTER_AUDIT_ORGANISATION_ID is required");
}
+if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(organisationId)) {
+ throw new Error("MUSTER_AUDIT_ORGANISATION_ID must be a UUID");
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const organisationId = process.env.MUSTER_AUDIT_ORGANISATION_ID; | |
| if (!organisationId) { | |
| throw new Error("MUSTER_AUDIT_ORGANISATION_ID is required"); | |
| } | |
| const organisationId = process.env.MUSTER_AUDIT_ORGANISATION_ID; | |
| if (!organisationId) { | |
| throw new Error("MUSTER_AUDIT_ORGANISATION_ID is required"); | |
| } | |
| if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(organisationId)) { | |
| throw new Error("MUSTER_AUDIT_ORGANISATION_ID must be a UUID"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/database/src/verify-audit-integrity.ts` around lines 5 - 9, Validate
the format of MUSTER_AUDIT_ORGANISATION_ID immediately after the existing
presence check and before any audit query, rejecting malformed identifiers with
a clear error. Reuse the project’s established organisation-ID validation
mechanism if available, while preserving the existing
required-environment-variable error.
Comment on lines
+112
to
+120
| function storageConfiguration() { | ||
| return { | ||
| endpoint: process.env.OBJECT_STORAGE_ENDPOINT ?? "http://127.0.0.1:9000", | ||
| bucket: process.env.OBJECT_STORAGE_BUCKET ?? "muster-evidence", | ||
| region: process.env.OBJECT_STORAGE_REGION ?? "us-east-1", | ||
| accessKey: process.env.OBJECT_STORAGE_ACCESS_KEY ?? "muster", | ||
| secretKey: process.env.OBJECT_STORAGE_SECRET_KEY ?? "local-minio-secret", | ||
| }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove hardcoded fallback credentials.
storageConfiguration() defaults accessKey/secretKey to well-known values ("muster" / "local-minio-secret") when the corresponding env vars aren't set, instead of failing fast. Since this configuration signs every put/get/head/delete request against the evidence bucket — including the destructive cleanup delete path — a deployment that forgets to set these vars silently authenticates with a guessable credential pair rather than erroring out.
🔒 Proposed fix: fail fast instead of defaulting secrets
function storageConfiguration() {
+ const accessKey = process.env.OBJECT_STORAGE_ACCESS_KEY;
+ const secretKey = process.env.OBJECT_STORAGE_SECRET_KEY;
+ if (!accessKey || !secretKey) {
+ throw new Error("Object storage credentials are not configured");
+ }
return {
endpoint: process.env.OBJECT_STORAGE_ENDPOINT ?? "http://127.0.0.1:9000",
bucket: process.env.OBJECT_STORAGE_BUCKET ?? "muster-evidence",
region: process.env.OBJECT_STORAGE_REGION ?? "us-east-1",
- accessKey: process.env.OBJECT_STORAGE_ACCESS_KEY ?? "muster",
- secretKey: process.env.OBJECT_STORAGE_SECRET_KEY ?? "local-minio-secret",
+ accessKey,
+ secretKey,
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function storageConfiguration() { | |
| return { | |
| endpoint: process.env.OBJECT_STORAGE_ENDPOINT ?? "http://127.0.0.1:9000", | |
| bucket: process.env.OBJECT_STORAGE_BUCKET ?? "muster-evidence", | |
| region: process.env.OBJECT_STORAGE_REGION ?? "us-east-1", | |
| accessKey: process.env.OBJECT_STORAGE_ACCESS_KEY ?? "muster", | |
| secretKey: process.env.OBJECT_STORAGE_SECRET_KEY ?? "local-minio-secret", | |
| }; | |
| } | |
| function storageConfiguration() { | |
| const accessKey = process.env.OBJECT_STORAGE_ACCESS_KEY; | |
| const secretKey = process.env.OBJECT_STORAGE_SECRET_KEY; | |
| if (!accessKey || !secretKey) { | |
| throw new Error("Object storage credentials are not configured"); | |
| } | |
| return { | |
| endpoint: process.env.OBJECT_STORAGE_ENDPOINT ?? "http://127.0.0.1:9000", | |
| bucket: process.env.OBJECT_STORAGE_BUCKET ?? "muster-evidence", | |
| region: process.env.OBJECT_STORAGE_REGION ?? "us-east-1", | |
| accessKey, | |
| secretKey, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/evidence/src/object-storage.ts` around lines 112 - 120, Update
storageConfiguration() to remove the hardcoded defaults for accessKey and
secretKey, requiring OBJECT_STORAGE_ACCESS_KEY and OBJECT_STORAGE_SECRET_KEY to
be explicitly configured and failing fast when either is missing. Preserve the
existing defaults for endpoint, bucket, and region.
Comment on lines
+122
to
+267
| export const defaultObjectStorage: CleanupObjectStorage = { | ||
| async putObject(object) { | ||
| const { endpoint, bucket, region, accessKey, secretKey } = | ||
| storageConfiguration(); | ||
| const url = objectUrl(endpoint, bucket, object.storageKey); | ||
| const response = await fetch(url, { | ||
| method: "PUT", | ||
| headers: signingHeaders( | ||
| "PUT", | ||
| url, | ||
| object.body, | ||
| region, | ||
| accessKey, | ||
| secretKey, | ||
| object.contentType, | ||
| ), | ||
| body: Buffer.from(object.body), | ||
| }); | ||
| if (!response.ok) { | ||
| throw new Error( | ||
| `Object storage rejected upload with status ${response.status}`, | ||
| ); | ||
| } | ||
| }, | ||
| async getObject(storageKey) { | ||
| const { endpoint, bucket, region, accessKey, secretKey } = | ||
| storageConfiguration(); | ||
| const url = objectUrl(endpoint, bucket, storageKey); | ||
| const emptyBody = new Uint8Array(); | ||
| const response = await fetch(url, { | ||
| method: "GET", | ||
| headers: signingHeaders( | ||
| "GET", | ||
| url, | ||
| emptyBody, | ||
| region, | ||
| accessKey, | ||
| secretKey, | ||
| ), | ||
| }); | ||
| if (!response.ok) { | ||
| throw new Error( | ||
| `Object storage rejected download with status ${response.status}`, | ||
| ); | ||
| } | ||
| return new Uint8Array(await response.arrayBuffer()); | ||
| }, | ||
| async headObject(storageKey, versionId) { | ||
| const { endpoint, bucket, region, accessKey, secretKey } = | ||
| storageConfiguration(); | ||
| const url = objectUrl(endpoint, bucket, storageKey); | ||
| if (versionId !== "unversioned") { | ||
| url.searchParams.set("versionId", versionId); | ||
| } | ||
| const emptyBody = new Uint8Array(); | ||
| const response = await fetch(url, { | ||
| method: "HEAD", | ||
| headers: signingHeaders( | ||
| "HEAD", | ||
| url, | ||
| emptyBody, | ||
| region, | ||
| accessKey, | ||
| secretKey, | ||
| ), | ||
| }); | ||
| if (response.status === 404) return null; | ||
| if (!response.ok) { | ||
| throw new Error( | ||
| `Object storage rejected metadata request with status ${response.status}`, | ||
| ); | ||
| } | ||
| const size = Number(response.headers.get("content-length")); | ||
| const etag = response.headers.get("etag")?.replace(/^"|"$/g, ""); | ||
| const actualVersionId = | ||
| response.headers.get("x-amz-version-id") ?? "unversioned"; | ||
| if (!Number.isSafeInteger(size) || size < 0 || !etag || !actualVersionId) { | ||
| throw new Error("Object storage returned incomplete version metadata"); | ||
| } | ||
| const objectLockMetadata = Object.fromEntries( | ||
| ["x-amz-object-lock-mode", "x-amz-object-lock-retain-until-date"] | ||
| .map((header) => [header, response.headers.get(header)] as const) | ||
| .filter((entry): entry is readonly [string, string] => | ||
| Boolean(entry[1]), | ||
| ), | ||
| ); | ||
| return { | ||
| size, | ||
| etag, | ||
| versionId: actualVersionId, | ||
| legalHold: | ||
| response.headers.get("x-amz-object-lock-legal-hold")?.toUpperCase() === | ||
| "ON", | ||
| objectLockMetadata, | ||
| }; | ||
| }, | ||
| async getObjectVersion(storageKey, versionId) { | ||
| const { endpoint, bucket, region, accessKey, secretKey } = | ||
| storageConfiguration(); | ||
| const url = objectUrl(endpoint, bucket, storageKey); | ||
| url.searchParams.set("versionId", versionId); | ||
| const emptyBody = new Uint8Array(); | ||
| const response = await fetch(url, { | ||
| method: "GET", | ||
| headers: signingHeaders( | ||
| "GET", | ||
| url, | ||
| emptyBody, | ||
| region, | ||
| accessKey, | ||
| secretKey, | ||
| ), | ||
| }); | ||
| if (!response.ok) { | ||
| throw new Error( | ||
| `Object storage rejected version download with status ${response.status}`, | ||
| ); | ||
| } | ||
| return new Uint8Array(await response.arrayBuffer()); | ||
| }, | ||
| async deleteObject(storageKey, versionId) { | ||
| const { endpoint, bucket, region, accessKey, secretKey } = | ||
| storageConfiguration(); | ||
| const url = objectUrl(endpoint, bucket, storageKey); | ||
| if (versionId !== "unversioned") { | ||
| url.searchParams.set("versionId", versionId); | ||
| } | ||
| const emptyBody = new Uint8Array(); | ||
| const response = await fetch(url, { | ||
| method: "DELETE", | ||
| headers: signingHeaders( | ||
| "DELETE", | ||
| url, | ||
| emptyBody, | ||
| region, | ||
| accessKey, | ||
| secretKey, | ||
| ), | ||
| }); | ||
| if (!response.ok && response.status !== 404) { | ||
| throw new Error( | ||
| `Object storage rejected version deletion with status ${response.status}`, | ||
| ); | ||
| } | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add request timeouts to all storage calls.
None of putObject/getObject/headObject/getObjectVersion/deleteObject bound their fetch calls with a timeout. If the S3-compatible endpoint hangs, the calling worker job (including the synthetic-cleanup deletion loop in apps/worker/src/synthetic-cleanup-object.ts) blocks indefinitely, contradicting the timeout pattern already used elsewhere in this PR for external calls (apps/worker/src/index.ts's gateway dispatch uses AbortSignal.timeout(10_000)).
⏱️ Proposed fix: add AbortSignal timeouts
const response = await fetch(url, {
method: "HEAD",
headers: signingHeaders(...),
+ signal: AbortSignal.timeout(10_000),
});Apply the same signal option to the other four fetch calls (putObject, getObject, getObjectVersion, deleteObject).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const defaultObjectStorage: CleanupObjectStorage = { | |
| async putObject(object) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, object.storageKey); | |
| const response = await fetch(url, { | |
| method: "PUT", | |
| headers: signingHeaders( | |
| "PUT", | |
| url, | |
| object.body, | |
| region, | |
| accessKey, | |
| secretKey, | |
| object.contentType, | |
| ), | |
| body: Buffer.from(object.body), | |
| }); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Object storage rejected upload with status ${response.status}`, | |
| ); | |
| } | |
| }, | |
| async getObject(storageKey) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, storageKey); | |
| const emptyBody = new Uint8Array(); | |
| const response = await fetch(url, { | |
| method: "GET", | |
| headers: signingHeaders( | |
| "GET", | |
| url, | |
| emptyBody, | |
| region, | |
| accessKey, | |
| secretKey, | |
| ), | |
| }); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Object storage rejected download with status ${response.status}`, | |
| ); | |
| } | |
| return new Uint8Array(await response.arrayBuffer()); | |
| }, | |
| async headObject(storageKey, versionId) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, storageKey); | |
| if (versionId !== "unversioned") { | |
| url.searchParams.set("versionId", versionId); | |
| } | |
| const emptyBody = new Uint8Array(); | |
| const response = await fetch(url, { | |
| method: "HEAD", | |
| headers: signingHeaders( | |
| "HEAD", | |
| url, | |
| emptyBody, | |
| region, | |
| accessKey, | |
| secretKey, | |
| ), | |
| }); | |
| if (response.status === 404) return null; | |
| if (!response.ok) { | |
| throw new Error( | |
| `Object storage rejected metadata request with status ${response.status}`, | |
| ); | |
| } | |
| const size = Number(response.headers.get("content-length")); | |
| const etag = response.headers.get("etag")?.replace(/^"|"$/g, ""); | |
| const actualVersionId = | |
| response.headers.get("x-amz-version-id") ?? "unversioned"; | |
| if (!Number.isSafeInteger(size) || size < 0 || !etag || !actualVersionId) { | |
| throw new Error("Object storage returned incomplete version metadata"); | |
| } | |
| const objectLockMetadata = Object.fromEntries( | |
| ["x-amz-object-lock-mode", "x-amz-object-lock-retain-until-date"] | |
| .map((header) => [header, response.headers.get(header)] as const) | |
| .filter((entry): entry is readonly [string, string] => | |
| Boolean(entry[1]), | |
| ), | |
| ); | |
| return { | |
| size, | |
| etag, | |
| versionId: actualVersionId, | |
| legalHold: | |
| response.headers.get("x-amz-object-lock-legal-hold")?.toUpperCase() === | |
| "ON", | |
| objectLockMetadata, | |
| }; | |
| }, | |
| async getObjectVersion(storageKey, versionId) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, storageKey); | |
| url.searchParams.set("versionId", versionId); | |
| const emptyBody = new Uint8Array(); | |
| const response = await fetch(url, { | |
| method: "GET", | |
| headers: signingHeaders( | |
| "GET", | |
| url, | |
| emptyBody, | |
| region, | |
| accessKey, | |
| secretKey, | |
| ), | |
| }); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Object storage rejected version download with status ${response.status}`, | |
| ); | |
| } | |
| return new Uint8Array(await response.arrayBuffer()); | |
| }, | |
| async deleteObject(storageKey, versionId) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, storageKey); | |
| if (versionId !== "unversioned") { | |
| url.searchParams.set("versionId", versionId); | |
| } | |
| const emptyBody = new Uint8Array(); | |
| const response = await fetch(url, { | |
| method: "DELETE", | |
| headers: signingHeaders( | |
| "DELETE", | |
| url, | |
| emptyBody, | |
| region, | |
| accessKey, | |
| secretKey, | |
| ), | |
| }); | |
| if (!response.ok && response.status !== 404) { | |
| throw new Error( | |
| `Object storage rejected version deletion with status ${response.status}`, | |
| ); | |
| } | |
| }, | |
| }; | |
| export const defaultObjectStorage: CleanupObjectStorage = { | |
| async putObject(object) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, object.storageKey); | |
| const response = await fetch(url, { | |
| method: "PUT", | |
| headers: signingHeaders( | |
| "PUT", | |
| url, | |
| object.body, | |
| region, | |
| accessKey, | |
| secretKey, | |
| object.contentType, | |
| ), | |
| signal: AbortSignal.timeout(10_000), | |
| body: Buffer.from(object.body), | |
| }); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Object storage rejected upload with status ${response.status}`, | |
| ); | |
| } | |
| }, | |
| async getObject(storageKey) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, storageKey); | |
| const emptyBody = new Uint8Array(); | |
| const response = await fetch(url, { | |
| method: "GET", | |
| headers: signingHeaders( | |
| "GET", | |
| url, | |
| emptyBody, | |
| region, | |
| accessKey, | |
| secretKey, | |
| ), | |
| signal: AbortSignal.timeout(10_000), | |
| }); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Object storage rejected download with status ${response.status}`, | |
| ); | |
| } | |
| return new Uint8Array(await response.arrayBuffer()); | |
| }, | |
| async headObject(storageKey, versionId) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, storageKey); | |
| if (versionId !== "unversioned") { | |
| url.searchParams.set("versionId", versionId); | |
| } | |
| const emptyBody = new Uint8Array(); | |
| const response = await fetch(url, { | |
| method: "HEAD", | |
| headers: signingHeaders( | |
| "HEAD", | |
| url, | |
| emptyBody, | |
| region, | |
| accessKey, | |
| secretKey, | |
| ), | |
| signal: AbortSignal.timeout(10_000), | |
| }); | |
| if (response.status === 404) return null; | |
| if (!response.ok) { | |
| throw new Error( | |
| `Object storage rejected metadata request with status ${response.status}`, | |
| ); | |
| } | |
| const size = Number(response.headers.get("content-length")); | |
| const etag = response.headers.get("etag")?.replace(/^"|"$/g, ""); | |
| const actualVersionId = | |
| response.headers.get("x-amz-version-id") ?? "unversioned"; | |
| if (!Number.isSafeInteger(size) || size < 0 || !etag || !actualVersionId) { | |
| throw new Error("Object storage returned incomplete version metadata"); | |
| } | |
| const objectLockMetadata = Object.fromEntries( | |
| ["x-amz-object-lock-mode", "x-amz-object-lock-retain-until-date"] | |
| .map((header) => [header, response.headers.get(header)] as const) | |
| .filter((entry): entry is readonly [string, string] => | |
| Boolean(entry[1]), | |
| ), | |
| ); | |
| return { | |
| size, | |
| etag, | |
| versionId: actualVersionId, | |
| legalHold: | |
| response.headers.get("x-amz-object-lock-legal-hold")?.toUpperCase() === | |
| "ON", | |
| objectLockMetadata, | |
| }; | |
| }, | |
| async getObjectVersion(storageKey, versionId) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, storageKey); | |
| url.searchParams.set("versionId", versionId); | |
| const emptyBody = new Uint8Array(); | |
| const response = await fetch(url, { | |
| method: "GET", | |
| headers: signingHeaders( | |
| "GET", | |
| url, | |
| emptyBody, | |
| region, | |
| accessKey, | |
| secretKey, | |
| ), | |
| signal: AbortSignal.timeout(10_000), | |
| }); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Object storage rejected version download with status ${response.status}`, | |
| ); | |
| } | |
| return new Uint8Array(await response.arrayBuffer()); | |
| }, | |
| async deleteObject(storageKey, versionId) { | |
| const { endpoint, bucket, region, accessKey, secretKey } = | |
| storageConfiguration(); | |
| const url = objectUrl(endpoint, bucket, storageKey); | |
| if (versionId !== "unversioned") { | |
| url.searchParams.set("versionId", versionId); | |
| } | |
| const emptyBody = new Uint8Array(); | |
| const response = await fetch(url, { | |
| method: "DELETE", | |
| headers: signingHeaders( | |
| "DELETE", | |
| url, | |
| emptyBody, | |
| region, | |
| accessKey, | |
| secretKey, | |
| ), | |
| signal: AbortSignal.timeout(10_000), | |
| }); | |
| if (!response.ok && response.status !== 404) { | |
| throw new Error( | |
| `Object storage rejected version deletion with status ${response.status}`, | |
| ); | |
| } | |
| }, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/evidence/src/object-storage.ts` around lines 122 - 267, Update every
fetch call in defaultObjectStorage—putObject, getObject, headObject,
getObjectVersion, and deleteObject—to include the established
AbortSignal.timeout(10_000) signal option, matching the existing external-call
timeout pattern. Preserve each method’s current request and response handling.
This was referenced Jul 27, 2026
Owner
Author
StatusClosing as superseded by #42 ( What we found
Unique deltaTwo-dot diff No further commits needed on this branch. Follow-up CodeRabbit nits that remain on |
Owner
Author
jusso-dev
added a commit
that referenced
this pull request
Jul 28, 2026
fix: address CodeRabbit findings from PR #41
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.
What changed
Why
Issue #40 exposed gaps in direct-message delivery, research completion visibility, audit verification, and safe removal of synthetic demonstration data. This change closes those gaps without treating Redis or object storage as authoritative.
Safety
Validation
Closes #40
Summary by CodeRabbit