Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
# Conflicts: # ee/apps/den-api/src/routes/auth/index.ts
benjaminshafii
added a commit
to joi-fairshare/openwork
that referenced
this pull request
Jul 3, 2026
Default token scopes are read-only since different-ai#2280, so the save step failed with insufficient_mcp_scope. Mirrors the desktop client's mint (den.ts).
benjaminshafii
added a commit
that referenced
this pull request
Jul 3, 2026
) * feat(den-db): memory bank schema, typeids, idempotent FULLTEXT index Stage 1 of Memory Bank v0 (TASK-1). - Register `memory`/`memctx` typeid prefixes in packages/utils [B5] - Add `memory` + `memory_context` tables. No DB-level FK (den-db avoids FK constraints per PlanetScale/Vitess convention); the memory->context cascade is app-enforced in the Stage 2 delete handler, backed by an index on memory_id. - Migration 0028 (tables + indexes) with a hand-appended CREATE FULLTEXT INDEX for the migrate path (Drizzle mysql-core has no FULLTEXT DSL, drizzle-orm#1495). - ensureMemoryFulltextIndex / assertMemoryFulltextIndexExists + a bootstrap hook, so the FULLTEXT index is created idempotently on BOTH the fresh-install (drizzle-kit push + baseline, which drops migration-only indexes) and the migrate paths [B2]. - verify-memory-schema.ts: proof harness (schema, FULLTEXT on both paths, NL MATCH, typeid round-trip, explicit cascade, idempotency). - docs/memory-bank-architecture.md (authoritative architecture). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(den-db): address Stage 1 review findings (mig-0, arch-0/1/2, ts-0/1) Zero-findings pass after a 5-lens review of the Stage 1 schema commit. - mig-0: run the idempotent FULLTEXT ensure on ALL apply paths, not just bootstrap. `db:migrate` and `db:push` now chain `ensure-fulltext-indexes.ts`, so a `db:migrate`-only run on an already-baselined DB (the prod path in den-db-migrate.yml, and the local db:push dev path) still creates the index. - arch-0: expose the proof harness as `db:verify-memory` (was only runnable via a raw node invocation, weakening its CI-gate claim). - arch-1: single `ensureFulltextIndexes()` seam in fulltext.ts called by both apply paths, so they cannot drift; bootstrap log is now accurate. - arch-2: extract createExecutor/Executor into scripts/db-executor.ts, shared by bootstrap.ts and the ensure runner (removes memory-scoped triplication). - ts-0/int-0: roll back before best-effort cleanup in verify-memory-schema.ts so a mid-transaction failure cannot leak marker rows. - ts-1: make isRecord file-private (dead export surface). Re-proven against MySQL 8.4: bootstrap, db:push, and the db:migrate chain each create the FULLTEXT index; second ensure is a no-op; db:verify-memory passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(den-api): user-scoped memory routes + Memory MCP tag (TASK-2, TASK-3) Stage 2 of Memory Bank v0. Routes (routes/memory): postMemory, getMemorySearch, getMemory (list), deleteMemoryById — all owner-scoped on user_id as the PRIMARY predicate (NOT the org-scoped worker pattern) [B4]: - getMemoryByIdForUser + 404 (non-leaking) for ids the caller does not own. - Flattened, mostly-optional save payload; shape encoded in the OpenAPI summary; server sets source and forces scope='user' (ignores any client scope) [B3]. - One-transaction save (memory + N context rows); input bounds on content/tags/ contexts/query/limit. - Bound MATCH(content) AGAINST(:q IN NATURAL LANGUAGE MODE) combined with user_id in a single and(); never sql.raw. Empty result set -> 200 (not an error). - Explicit memory->context cascade in the delete handler (den-db has no DB FKs). - Structured save/search/delete events (JSON log lines) for measurement/debugging. Guarded with authenticatedRoute(); org id read from the session (works for the MCP internal-principal path without re-resolving org membership). MCP (TASK-3): "Memory" added to SAFE_INCLUDED_TAGS; routes tagged ["Memory"], so the four capabilities surface via search_capabilities / execute_capability. The operationIds are exactly postMemory/getMemorySearch/getMemory/deleteMemoryById (never memory_save/memory_search). Tests (bun): test/memory-routes.test.ts (operationIds, MCP allow+scopes, payload bounds, scope/source not client-settable) and test/memory-ownership.test.ts — the cross-user IDOR regression merge gate [B4] proving save -> Bob 404 -> Alice recall -> cascade delete against the real app + MySQL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(den-api): address Stage 2 review findings (int-1/3, arch-1/2/conv-1, ts-1/2/3) Zero-findings pass after a 5-lens review (security · perf · TS · integrity · arch). - int-1/arch-2: save now sets created_at/updated_at explicitly to a single `now` and returns that same row via toMemoryResponse, so the 201 body is DB-authoritative (matches a later getMemory/getMemorySearch) — MySQL has no INSERT ... RETURNING. - conv-1: one `const row = {...} satisfies typeof MemoryTable.$inferInsert` is the single source for both the insert and the response — they cannot drift. - int-3: context rows share the same `now`, so a memory and its provenance carry one timestamp; memory.created_at is the app-clock ordering authority (documented). - ts-1: POST response routed through toMemoryResponse (was an inline literal). - ts-2: DELETE 204 uses emptyResponse(); ts-3: dropped dead schema exports. - sec-1: comment — reads are user-scoped, intentionally org-spanning (§8). - arch-1: comment — org id from the resolved session; desktop hydrates via /me/orgs. - int-2: architecture §9 records account/org-deletion memory reaping as deferred pre-GA. Accepted-in-writing (no code change): sec-2 (session ids already normalized → can't throw), perf-1 (inherent MySQL fulltext post-filter), perf-2 (composite index deferred, small v0 corpus), ts-4/arch-3 (intentional, required by the mapper). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(server): static search-first ## Memory Bank prompt (TASK-4) Append a static, distinct `## Memory Bank` section to OPENWORK_AGENT_PROMPT (separate from the credential-hygiene `## Memory` section), injected unconditionally. Search-first [B1]: reach memory only by discovering a capability via search_capabilities then running it via execute_capability — never a bespoke memory_save/memory_search tool; it is NOT a local file. Covers draft → human-confirm/edit → execute save, explicit lexical recall (no auto-recall), a list/delete manage note, and no-secrets guidance (the only v0 plaintext-at-rest mitigation) extended to cited snippets. Also corrects the capabilities-knowledge plugin, which previously told the agent there was "no separate long-term memory store" — it now points at the Memory Bank capability. Test: openwork-runtime-config.test.ts asserts the section is present, distinct from `## Memory`, names search_capabilities/execute_capability, contains neither memory_save nor memory_search, and includes no-secrets guidance (4 pass). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(evals): memory-save-recall flow — discover → save → recall (TASK-9) Mirror mcp-search-capabilities.flow.mjs. Proves, on the live meta-MCP + a signed-in desktop app, that the agent discovers the memory-save capability search-first (no memory_save tool), execute_capability persists a scope='user' memory, a FRESH mcp call recalls it by natural-language query (cross-session lexical recall), and the desktop Memory panel shows it. Fails loudly on B1 (search-first) and B3 (flattened save body) regressions. Proven end-to-end against the running stack: postMemory discovered search-first (hasBody:true) → saved mem_… (scope forced user) → recalled via "Acme renewal deal" (FULLTEXT score 0.18) → deleted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(den-api): execute_capability accepts stringified path/query args MCP clients (LLMs) frequently pass `path`/`query` as a JSON-encoded string rather than an object, which failed execute_capability input validation ("expected record, received string") — the same hazard normalizeToolBody already guarded for the body. Loosen the path/query schema to accept a string and normalize it via a new normalizeToolRecord (mirrors normalizeToolBody), so stringified args like `"{\"q\":\"acme\"}"` work. Unblocks getMemorySearch recall via the agent. General improvement — applies to every capability, not just memory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(den-api): save memory only to an org the caller is a member of (orgMemberRoute) POST /v1/memory previously took org_id from session.activeOrganizationId with no membership check, so a stale/forged active org (or MCP principal) could write a memory tagged to an org the user isn't in. Switch the save guard from authenticatedRoute() to orgMemberRoute(): resolveOrganizationContextMiddleware resolves the active org AND verifies membership via getOrganizationContextForUser (404 otherwise) on both the cookie/bearer and MCP-principal paths. org_id now comes from the verified organizationContext.organization.id. - 400 organization_unavailable → the middleware returns 404 organization_not_found for a non-member / no-active-org caller (before the handler runs). - Reads (search/list) and delete stay authenticatedRoute() — user_id-scoped and cross-org by design (v0 recalls across all of the caller's orgs; org_id is only recorded for the future org-shared bank, §8). - memory-ownership.test.ts seeds real org + membership rows (required by the gate) and adds a non-member save → 404 assertion. Proven live via the meta-MCP (member save→recall still works) and by the ownership integration test (5 pass: member 201, non-member 404, IDOR 404, cascade delete). den-api tsc: 0 errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(evals): mint the memory eval MCP token with mcp:write scope Default token scopes are read-only since #2280, so the save step failed with insufficient_mcp_scope. Mirrors the desktop client's mint (den.ts). --------- Co-authored-by: jthorare <60298547+jthorare@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Benjamin Shafii <benjamin.shafii@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
pnpm --filter @openwork-ee/den-api exec bun test test/mcp-scopes.test.ts test/mcp-auth.test.ts test/mcp-invoke-body.test.ts test/mcp-oauth-client-policy.test.ts test/mcp-token-lifetime.test.ts test/mcp-membership-revocation.test.ts→ 23 pass, 0 failpnpm --filter @openwork-ee/den-api build→ passedpnpm --filter @openwork/app typecheck→ passedpnpm dev:web-locallive validation after restoring local MySQL health:mcp:readandmcp:writeopenid profile email mcp:readmcp:readremains read-only; explicitmcp:writeremains write-only["mcp:read"]["mcp:read", "mcp:write"]insufficient_mcp_scopeNote
Live third-party OAuth code exchange exposed a separate existing Better Auth MySQL adapter issue where the authorization code row is consumed but
/oauth2/tokenreturnsinvalid_grant. The scope behavior up through registration, consent/code issuance, and first-party MCP enforcement is validated in this PR; that token-exchange bug is separate from this least-privilege change.