Skip to content

feat: make the context-memory ACL switchable from the operator UI - #903

Merged
Weegy merged 3 commits into
mainfrom
feat/context-memory-flag
Aug 27, 2026
Merged

feat: make the context-memory ACL switchable from the operator UI#903
Weegy merged 3 commits into
mainfrom
feat/context-memory-flag

Conversation

@Weegy

@Weegy Weegy commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Closes nothing — Refs #899, part of #860.

W5 (#881) shipped the chat-context memory ACL — the scoping, the /memories/contexts/ tree, the promote route and its audit log — behind agents.context_memory, a plain column with no UI and no API endpoint. The only supported way to enable it was a hand-written UPDATE, which left the whole wave inert. This adds the operator surface, and reads the one part of the wave that shipped with a stated residual risk.

No schema change. The column and its CHECK constraint already exist (migration 0050).

What the route can do

GET /api/v1/operator/agents/<slug>/context-memory   → { slug, mode, modes }
PUT /api/v1/operator/agents/<slug>/context-memory     { mode: 'off' | 'enforce' | 'enforce-strict' }  → { ok: true, mode }

On the existing operator-agents router ({ ok: true } envelope, requireAuth from the parent mount). The union is taken from the code, not from the docs: ContextMemoryMode in memoryBinder.ts and the CHECK constraint in 0050 both spell off | enforce | enforce-strict, and a test pins the route's list against the migration file so the two cannot drift.

Four deliberate choices:

  • PUT rejects an unknown mode with 400 invalid_body rather than coercing it to off. A security switch that reports success while writing nothing is worse than an error. An untouched agent is auditable; one silently reset to off while the UI shows enforce is not.
  • GET narrows deny-default. A value written by a newer middleware during a rolling deploy reads back as off — the same rule parseContextMemoryMode applies in the orchestrator. The UI must never show "enforcing" for a value the runtime routes as off.
  • A write triggers registry.reload(), so the next turn is already scoped. Without it the switch would only take effect on the next process restart — the shape of bug that reads as "it did nothing" in production. The mode change is logged under [security-audit].
  • The mode is NOT a field on PATCH /:slug. That handler is the dashboard's rename/enable form and sends whatever it holds; folding a memory-scope change into it would let an unrelated edit carry one along. A test pins that a rename leaves an enforcing agent enforcing.

ConfigStore.updateAgent grew a contextMemory field on its patch type; the UPDATE uses COALESCE, so a patch that does not mention the flag can never reset an enforcing agent.

What the UI can do

A Chat-context memory section on the agent detail page, next to the other per-agent settings. Radio group, save button, localized error banner.

Switching away from off surfaces the three semantics before the operator can commit, and requires an explicit acknowledgement:

  1. Team tier is read-write. A turn from a team context writes into its own tier and into the shared team tier — the tier teammates read each other in.
  2. Agent tier is read-only. The orchestrator's own memory stays readable but becomes read-only for context turns, so "remember this globally" can no longer carry knowledge from one team to another.
  3. API turns get the agent-private scope only. Turns from the API carry no chat context, so they only ever reach the private memory of the orchestrator itself — never a team or channel tree.

Switching back to off, and tightening enforceenforce-strict, need no acknowledgement: the safe direction must never be harder than the unsafe one. Changing the selection re-arms the gate, so an acknowledgement given for one mode cannot unlock a save of another.

The mode list is intersected with the server's modes array — the server can take a mode away, but it cannot add one, since a mode this bundle has no label for cannot be rendered. Everything from the wire is narrowed through parseContextMemoryMode before it reaches state; a component that renders a security state should not inherit its trust from a type annotation. (That last point was not planned — the test asserting it failed first, against a component that trusted the DTO.)

i18n: all strings through the catalogue, EN + DE complete, npm run i18n:check green.

orchestrator.ts finding — the TurnOrigin threading

The W5 integration named one residual risk: the binding is threaded through ~8 signatures in a ~7300-line file, covered by tests but never exercised by a real streaming turn. I read that path. Verdict: the threading itself is correct — and now proven, not asserted — but it had one latent defect, which is fixed here, and it has two boundaries outside the orchestrator where the flag buys nothing.

The threading is sound

The binding is created exactly twice, once per turn entry point, both after input is final: runTurnCore (orchestrator.ts:3457) and chatStreamInContext (:5106). It travels as a required parameter through executeDirectLine (:3583), chatInContext (:4124), chatInContextInner (:4234), chatStreamInner (:5297) and prepareStreamSlot (:6203) down to dispatchToolInner:6782:

const memoryHandler = turnMemory ? turnMemory.handler : this.memoryToolHandler;

Every one of the four dispatch call sites passes it — the non-streaming tool loop (:4664), the streaming slot (:6216), sub-agent delegation via Direct Line (:3739), and the deadline/retry wrappers (:6436, :6446, :6449). The system prompt gets the matching treatment on both paths (:4415, :5539), so the model is never told about /memories/~team/ on a turn where that path is not mapped. The _rules decorator sits inside the binder's own stack (memoryBinder.ts:227), not on a separate orchestrator-side path. I found no path on which a turn carrying an origin reaches the agent-private handler.

Fail-closed is real and layered: mode: 'off' discards the origin in one branch before axes are resolved (memoryBinder.ts:162-165); an absent origin, unscoped, system, a channelType outside {teams, telegram, http, api}, or unusable patterns all resolve context-free (turnOrigin.ts:233-248, scopedMemoryStore.ts:195-220); a binder that throws is caught and degrades to the agent-private handler with a [security-audit] log (orchestrator.ts:3277-3287); and the shared trees are granted as ro:core, not core, so /memories/core/notes.md is not a one-line bypass.

Finding → fix: the binding could be dropped without a compile error

dispatchTool (:6316), dispatchToolDeadlined (:6375) and dispatchToolInner (:6753) took the binding in an optional position (turnMemory?: TurnMemoryBinding), while all six other signatures on the path take it in a required one. A call site that simply forgot the argument therefore compiled cleanly and fell back to this.memoryToolHandler — the agent-global stack — at runtime. That is precisely the silent scope widening the wave exists to prevent, and the code comment two lines above calls it "a structural impossibility".

Verified rather than assumed: I deleted turnMemory from the two tool-loop call sites and tsc passed. All seven cases of the new integration test then turned red.

Fixed by making the three parameters required (TurnMemoryBinding | undefined). Zero runtime change — every existing call site already passed all four arguments, so the build was clean on the first try. The same mutation now fails npm run typecheck with exit 2. A future call site that forgets the binding is a compile error, not a leak.

New: the first integration coverage of the binding

middleware/test/orchestrator/contextMemoryTurnBinding.test.ts — 7 cases. Every W5 suite so far stopped at MemoryBinder's handler; none constructed an Orchestrator or ran a turn, and contextBound appeared nowhere in middleware/test/. These build a real orchestrator with a scripted provider that calls the memory tool, drive both runTurn and chatStream with a Teams TurnOrigin, and assert the physical path recorded at the undecorated root store:

  • a turn with an origin lands under /memories/contexts/<slug>/… and not in the agent-global tree — buffered and streaming;
  • two different team contexts never resolve to the same physical path;
  • a turn without an origin stays agent-private under both enforce and enforce-strict;
  • mode: 'off' ignores a present origin entirely (the byte-identical rollout default that makes this UI safe to ship);
  • an unusable channelType narrows instead of throwing.

The harness deliberately wires a build-time memoryToolHandler over the raw root, so a lost binding would still succeed and write to the wrong place — which is what makes the assertions load-bearing. All seven were confirmed red under mutation before being reported green.

The streaming case is the one that mattered: an async generator is resumed in the async context of whoever calls .next(), which is exactly how an earlier wave silently lost turnContext on every streaming turn. It passes.

Two boundaries where the flag buys nothing (pre-existing, not fixed here)

Both are older than W5 and outside this PR's scope, but an operator flipping the switch should know, so they are now in the doc's "known limits" list:

  1. claude-cli provider. buildOrchestrator.ts:523-545 returns a CliChatAgent, not the Orchestrator, so bindTurnMemory never runs and origin is never read. Tools reach memory through ToolDispatchService, which has no memory branch — the divergence is already flagged at toolDispatchService.ts:568-571 ("scoped-memory shadowing"). Setting enforce-strict on a CLI-provider agent changes nothing.
  2. A sub-agent granted the native memory tool. adaptNativeToolForSubAgent (src/agents/subAgentToolHydration.ts:189-208) returns handle: (input) => handler(input) over the process-wide raw handler. The parent's dispatch carries the binding correctly, but the sub-agent's own loop calls the adapted raw handler — writing into the undecorated store, with no agent namespace and no context tier. This is reachable through supported operator configuration (an Agent Builder tool grant) and defeats both the W5 context ACL and the pre-existing per-agent isolation. Worth its own issue; the minimal fix is to deny memory in resolveSubAgentTools until sub-agents can be handed a turn-bound handler.

One boundary worth stating rather than fixing: the knowledge graph is not partitioned by context. maybePromoteTurn stamps originAgent, not a context key, so a fact auto-promoted from a team-A turn is recallable in team B for the same agent even under enforce-strict. That may well be an accepted boundary like the session-transcript decision (A3a), but "full quarantine" is not literally true of KG-derived recall. Relatedly, enforce means context turns can no longer author durable _rules at all (ro:core covers top-level _*) — a real behaviour change, correctly implemented, and now worth an operator-facing line.

Docs

  • docs/teams-multi-agent-identities.md §8 — the "there is no UI and no API endpoint, set it in the database" note is replaced with where the switch is, what the routes do, and why the mode is not on the rename PATCH; the known-limits list swaps the "no switch" entry for the two boundaries above; the file map gains the route, the component and the new test.
  • docs/CHANGELOG.md — an Added entry for the switch and a Fixed entry for the threading defect.

Gates

Gate Result
middleware: npm run build pass
middleware: npm run typecheck pass (all workspaces)
middleware: npm run lint pass (0 errors)
middleware: npm run typecheck:test pass — ratchet at 370, no regressions
middleware: npm test pass — exit 0, zero not ok across 12 348 lines of TAP
web-ui: npm run lint pass (0 errors)
web-ui: npm run typecheck pass
web-ui: npm run test pass — 982/982, 109 files
web-ui: npm run i18n:check pass — 4073 keys, en + de

New tests: 10 router cases, 7 orchestrator integration cases, 11 UI cases. No test pollution encountered — the full runs were green on both sides.

origin/main moved while this was in flight (#902); merged in, with the expected docs/CHANGELOG.md [Unreleased] collision resolved by keeping both entries. All gates re-run after the merge.

W5 (#881) shipped the chat-context memory ACL behind `agents.context_memory`,
a column with no UI and no API. The only way to enable it was a hand-written
UPDATE, which left the whole wave inert in practice.

API: GET/PUT /api/v1/operator/agents/:slug/context-memory on the existing
operator-agents router. PUT validates against the same value list as the CHECK
constraint of migration 0050, rejects an unknown mode with 400 rather than
coercing it to `off`, logs the change under `[security-audit]`, and reloads the
registry so the next turn is already scoped. Deliberately NOT a field on the
rename/enable PATCH: a memory-scope change must not ride along with an
unrelated edit. No schema change.

UI: a control on the agent detail page. Switching away from `off` surfaces the
three semantics an operator needs first (team tier read-write, agent tier
read-only, API turns agent-private only) and requires an explicit
acknowledgement; switching back to `off` does not, because the safe direction
must never be harder than the unsafe one.

Also fixes the one place the threading could silently regress: `dispatchTool`,
`dispatchToolDeadlined` and `dispatchToolInner` took the `TurnMemoryBinding` in
an optional position, so a call site that forgot it compiled cleanly and fell
back to the agent-global stack at runtime. Verified by dropping the argument —
it passed `tsc`. Now required (`| undefined`), runtime unchanged. Adds the
first integration coverage of the binding: real `runTurn` and `chatStream`
turns with a Teams TurnOrigin, asserting the physical write path.

Refs #899, part of #860
@Weegy Weegy closed this Aug 27, 2026
@Weegy Weegy reopened this Aug 27, 2026
…ting

The `as TurnOrigin` casts hid a malformed scope: `{ kind: 'channel', id }` is
not a member of the `ScopeId` union — the real kind is `conversation`. The
cases still passed, but through the TEAM axis derived from the container,
never through the channel tier they claimed to exercise, and the
`typecheck:test` ratchet rejected the casts as new errors.

Fixtures are now built structurally with no cast, so a malformed origin is a
compile error rather than a silent degradation to context-free — which would
turn these into assertions about the fallback instead of about the threading.
Mutation check re-run against the corrected fixtures: all seven still turn red
when the binding is dropped at the tool-loop call sites.

Refs #899, part of #860
@Weegy
Weegy force-pushed the feat/context-memory-flag branch from 8fbc158 to 4a51d2e Compare August 27, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant