Commit graph

8 commits

Author SHA1 Message Date
Snider
b7bc526d50 test(agent/brain): regression coverage for filter field bounds (closes #1000)
#1000 was stale-fixed: BrainService::recall() validates filter input
via the shared validator at line 489, which already bounds org,
project, type, agent_id. forget() bounds id at line 499.

These tests pin the safety claim explicitly:
- project=129 chars rejected
- agent_id=65 chars rejected
- project="core" accepted (sanity)
- project=128 chars accepted (boundary)

Note: BrainList.php (separate MCP list path) still lacks explicit
max lengths for project + agent_id — file outside this lane's allow-
list. File a follow-up if that surface needs the same bounds.

Co-authored-by: Codex <noreply@openai.com>
Closes tasks.lthn.sh/view.php?id=1000
2026-04-25 19:23:41 +01:00
Snider
385b89b3eb fix(agent/brain): cap remember()/recall()/forget() input field sizes
Bound input field sizes against memory/DB/Qdrant bloat (DoS-by-self):
- content: 65536 bytes via mb_strlen
- tags: max 100 entries; each tag max 128 chars
- agent_id, type: 64 chars each
- project, org: 128 chars each
- supersedes_id: ULID-shape only

validateRememberInput() throws InvalidArgumentException at every entry
point (remember, recall, forget) before any DB or upstream call. Field-
specific error messages so callers know which field violated.

Pest covers good-path, content-too-long, tags-array-too-large, tag-
length, exact-boundary cases.

Co-authored-by: Codex <noreply@openai.com>
Closes tasks.lthn.sh/view.php?id=1001
2026-04-25 18:58:41 +01:00
Snider
dea64f4099 fix(agent/brain): walk supersede chain to current head + cycle guard
remember() now resolves a stale supersedes_id to the current live head
before writing — when X has been superseded by Y, a retried call with
supersedes_id=X automatically links the new memory to Y instead of
silently dropping the supersede.

- Walk the chain from supplied supersedes_id to find the active head
- Cap the walk at depth 100 (cycle/runaway protection)
- Throw RuntimeException("Detected cycle while resolving supersede chain")
  on detected cycle, BEFORE any DB write
- Throw InvalidArgumentException("Superseded memory not found") when
  the original supersedes_id never existed
- deleteSupersededMemory no longer silently no-ops once the resolved
  head is expected to exist

Pest coverage extended:
- Direct chain link (X exists, succeeds with X→linked)
- Retry path (X→Y, then retry on X produces Z→Y, walks chain)
- Never-existed target (graceful error)
- Synthetic X↔Y cycle (caps walk + throws, no writes leak)

Co-authored-by: Codex <noreply@openai.com>
Closes tasks.lthn.sh/view.php?id=316
2026-04-25 18:42:52 +01:00
Snider
167ce9783e fix(agent/brain): authorise org against MCP context at every entry point
remember(), recall(), forget(), and elasticSearch() now resolve the
allowed-orgs set from the authenticated request context (mcp_workspace_context),
preferring explicit authorised_orgs/authorized_orgs, falling back to the
authenticated workspace's org/slug. A mismatched org throws
AuthorizationException BEFORE any Qdrant/Elasticsearch call or destructive
DB action — closes the horizontal-priv-escalation vector where an MCP
client could recall/remember/forget memories scoped to ANY org by
setting org="other-org" in the request body.

Pest coverage in OrgScopingTest covers good path, unauthorised recall
(asserts no HTTP), cross-org forget (asserts no DB delete), unauthorised
remember (asserts no embed/index jobs).

Note: BrainList free-form org filter is a separate ticket — outside this
lane's allowlist.

Co-authored-by: Codex <noreply@openai.com>
Closes tasks.lthn.sh/view.php?id=312
2026-04-25 18:32:19 +01:00
Snider
a1a0981b06 fix(agent/brain): retryableHttp narrows retryable set + 6-attempt budget
retryableHttp() now retries only 408 (Request Timeout), 429 (Too Many
Requests), and 503 (Service Unavailable). 500-and-other-5xx fail
immediately so the circuit-breaker registers them as a single
failure rather than smearing across retry attempts. Retry-After
honoured (numeric + HTTP-date), capped reasonably.

Attempt budget bumped to 6 so a burst of 5 transient 503s can recover
within ONE circuit-permitted call — the original concern from #311.

Note: CircuitBreaker is already applied OUTSIDE the logical Brain
operation by the MCP tool layer, not around each HTTP retry. The
nesting report was stale at this code shape; the real drift was the
retryableHttp() retry set + budget.

Pest coverage in CircuitBreakerTest:
- Recovered 503 burst → circuit stays closed, no failure registered
- Exhausted 503 burst → ONE breaker failure (not five)
- 429 + Retry-After 1 → sleeps 1s, no breaker failure
- 500 → immediate breaker failure, no retry

Co-authored-by: Codex <noreply@openai.com>
Closes tasks.lthn.sh/view.php?id=311
2026-04-25 18:14:40 +01:00
Snider
b6565263f3 fix(agent/brain): lock forget+supersede paths against late index writes
Cache::lock keyed by memory id wraps the delete path in BrainService::
forget(); supersede cleanup in remember() lifted to the same idiom.
forget() now ALWAYS queues DeleteFromIndex on a successful delete
(was previously skipped when indexed_at was null — left late writes
from stale preloaded models a window to land entries after the
underlying memory was gone).

Index write paths (qdrantUpsert / elasticIndex) re-check that the
memory row still exists before writing — defence-in-depth against any
future caller that holds a stale model reference past a forget.

Pest coverage extended in SupersedeForgetIndexCleanupTest:
- never-indexed forget queues cleanup
- late stale-model index writes are skipped after forget
- never-indexed supersede cleanup queues deletion
- late stale-model index writes are skipped after supersede

Co-authored-by: Codex <noreply@openai.com>
Closes tasks.lthn.sh/view.php?id=999
2026-04-25 18:04:55 +01:00
Snider
6832d40587 fix(agent/brain): batch — org maxLength + retry semantics + forget index cleanup
Codex 5.5 batch lane processed 8 brain Mantis tickets. 4 implemented,
1 stale, 3 deferred.

Tickets implemented:
- #313 — MCP schemas (BrainRemember/Recall/List): org field maxLength=128 with runtime validation; recall filter.org also bounded; pest test coverage added
- #314 — BrainList: removed withCircuitBreaker('brain') from DB-only handler; CircuitBreakerTest updated to assert no breaker call
- #315 — BrainService.retryableHttp(): now retries 408 (request-timeout), 429 (rate-limit), and 5xx; honours Retry-After header; focused retry tests added
- #326 — BrainService.forget(): dispatches DeleteFromIndex only when row has indexed_at (was unconditional); SupersedeForgetIndexCleanupTest covers never-indexed case

Tickets stale-fixed: #316 (RememberKnowledge already rejects missing/deleted supersedes target before dangling retry)
Tickets deferred: #121 (cross-surface audit), #311 (retry-inside-breaker architectural redesign), #312 (no authoritative org claim in MCP request context yet)

Co-authored-by: Codex <noreply@openai.com>
Closes tasks.lthn.sh/view.php?id=313
Closes tasks.lthn.sh/view.php?id=314
Closes tasks.lthn.sh/view.php?id=315
Closes tasks.lthn.sh/view.php?id=326
2026-04-25 14:55:40 +01:00
Snider
c616ff1e32 fix(brain): close openbrain audit gaps — org scoping + index cleanup + reindex flags + MCP schemas + circuit layer
Closes the 5 PARTIAL items flagged in docs/AUDIT-openbrain-20260424.md.

- Gap A (org scoping persisted on writes): new migration adds `org`
  nullable+indexed column to brain_memories; BrainMemory fillable;
  RememberKnowledge action forwards org; BrainService::remember
  persists it.

- Gap B (supersede/forget Elastic cleanup): BrainService::forget
  dispatches DeleteFromIndex (handles both Qdrant + Elastic); supersede
  path dispatches cleanup for the old memory id before replacing it.
  DeleteFromIndex itself untouched — already handled both indexes.

- Gap C (brain:reindex flags): --org, --project, --stale (null OR
  >14d old), --dry-run (count+stop), --elastic-only added to the
  artisan command.

- Gap D (MCP schemas expose org): brain_remember, brain_recall,
  brain_list now accept `org` in input schema + forward into
  action/service.

- Gap E (resilience uneven): brain_list now wrapped in
  withCircuitBreaker('brain', ...) matching the pattern used by
  BrainRemember/Recall/Forget. BrainService gains retryableHttp()
  helper — 100/300/900ms exponential backoff, retries only on 5xx +
  connection errors, not on 4xx. Qdrant calls route through it;
  Ollama left alone (EmbedMemory job has its own retry).

Tests (Good/Bad/Ugly per gap):
- Feature/Brain/OrgScopingTest.php
- Feature/Brain/SupersedeForgetIndexCleanupTest.php
- Feature/Brain/ReindexFlagsTest.php
- Feature/Mcp/BrainSchemaOrgTest.php
- Feature/Brain/CircuitBreakerTest.php

php -l clean on all 13 files. Pest binary not in this checkout —
CI path validates the full suite.

Closes tasks.lthn.sh/view.php?id=107

Co-authored-by: Codex <noreply@openai.com>
Co-Authored-By: Virgil <virgil@lethean.io>
2026-04-24 08:14:06 +01:00