Skip to content

feat: add core and space migration packages#71

Merged
cevian merged 164 commits into
mainfrom
multiplayer
Jun 16, 2026
Merged

feat: add core and space migration packages#71
cevian merged 164 commits into
mainfrom
multiplayer

Conversation

@jgpruitt

Copy link
Copy Markdown
Collaborator

No description provided.

cevian
cevian previously requested changes May 29, 2026

@cevian cevian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great progress. comments inline

Comment thread packages/core/migrate/incremental/001_shard.sql Outdated
Comment thread packages/core/migrate/provision.sql Outdated
, slug text not null unique check (slug ~ '^[a-z0-9]{12}$')
, name citext not null
, shard_id int not null references core.shard (id)
, language text not null default 'english' check (language ~ '^[a-z_]+$')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not put language support in now

@jgpruitt jgpruitt May 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one way or the other you have to configure pg_textsearch indexes with a language. The database migrations for the spaces are already wired to configure pg_textsearch languages.

Comment thread packages/core/migrate/incremental/004_group_member.sql Outdated
Comment thread packages/core/migrate/incremental/005_tree_access.sql Outdated
Comment thread packages/core/migrate/migrate.ts Outdated
Comment thread packages/core/migrate/migrate.ts Outdated
return;
}

const sorted1 = [...incrementals].sort((a, b) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we error if they werent already sorted. They should be, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they should be sorted. I'm indifferent on whether we error

*/
alter table {{schema}}.memory add constraint temporal_bounds_convention check
(
temporal is null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably dont want empty one either (force null instead)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't follow. the current logic is intended to allow 3 options

  1. null
  2. a point in time: start = end inclusive-inclusive
  3. a range: start < end inclusive-exclusive

I don't think an empty tstzrange is possible with this constraint unless i'm missing something.

Comment thread packages/space/migrate/bootstrap.ts Outdated
@@ -0,0 +1,163 @@
import { info, reportError, span } from "@pydantic/logfire-node";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to improve function reuse from a bunch of these migration files. Right now we have a lot of code duplication. Maybe a common migration utility lib?

@jgpruitt jgpruitt May 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. I care far more about code locality than DRY. If there is actual, felt pain, I guess we could put identical pieces into a common utility lib, but I wouldn't try to make a common "database migrator" process. As soon as they start to differ, you have to decide whether to parameterize the common doohickey to support both ways of working (which makes it harder to reason about and change), or pull it back apart.

If it's just little utility functions, then I guess I don't have a problem with it.

Comment thread packages/space/migrate/migrate.ts Outdated
@@ -0,0 +1,505 @@
import { createHash } from "node:crypto";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots of duplication here too

Comment on lines +145 to +161
-- principal_space: fire only when an admin row is removed or demoted (NEW can't
-- be referenced in a DELETE trigger's WHEN, hence two). group_member: fire on any
-- removal; the fn early-outs unless the group is an admin group.
create or replace trigger principal_space_keep_admin_del
after delete on {{schema}}.principal_space
for each row when (old.admin)
execute function {{schema}}.enforce_last_admin();

create or replace trigger principal_space_keep_admin_upd
after update on {{schema}}.principal_space
for each row when (old.admin and not new.admin)
execute function {{schema}}.enforce_last_admin();

create or replace trigger group_member_keep_admin_del
after delete on {{schema}}.group_member
for each row
execute function {{schema}}.enforce_last_admin();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I LOVE this! But I think these might be better as CONSTRAINT TRIGGERS that are DEFERRABLE INITIALLY DEFERRED, that way they can run at the end of the transaction. WDYT?

When the CONSTRAINT option is specified, this command creates a constraint trigger. This is the same as a regular trigger except that the timing of the trigger firing can be adjusted using SET CONSTRAINTS. Constraint triggers must be AFTER ROW triggers on plain tables (not foreign tables). They can be fired either at the end of the statement causing the triggering event, or at the end of the containing transaction; in the latter case they are said to be deferred. A pending deferred-trigger firing can also be forced to happen immediately by using SET CONSTRAINTS. Constraint triggers are expected to raise an exception when the constraints they implement are violated.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a99722f — all three are now DEFERRABLE INITIALLY DEFERRED constraint triggers, so the invariant is judged once at commit and an in-transaction admin swap (demote incumbent, promote replacement) no longer trips on the transient zero-admin state.

One wrinkle: Postgres supports neither CREATE OR REPLACE nor IF NOT EXISTS for constraint triggers, and a bare drop+create would take an ACCESS EXCLUSIVE lock on every idempotent run. So each is wrapped in a guard that drops+recreates only when the live trigger isn't already a deferred constraint trigger — it upgrades the old plain trigger on first run, then is a lock-free no-op afterward. Added regression tests for the swap, the still-rejected drop-to-zero (ME001 at commit, rolled back), and the guard's upgrade/skip behavior.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!
It's always a bummer when I find DDL that doesn't support OR REPLACE or IF NOT EXISTS. :(

)
as $func$
-- Group membership is space-scoped by group_member.space_id and confers
-- space access transitively — a member need NOT have a direct principal_space

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we sure we don't want ALL members of a space to be represented in principal_space?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user_tree_access() and agent_tree_access() both enforce that the principal must be represented in principal_space. I think we'd want groups to work the same way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We settled on the opposite of Model 2: yes — every space member must have a principal_space row, and group membership alone no longer confers space membership. Done in bd65f43. member_tree_access now gates group grants on is_principal_in_space (matching what user_tree_access/agent_tree_access already required for direct grants), so build_tree_access — the auth gate — is empty for someone who is only in a group. Rationale: we did not want a second path to space membership; everyone is invited and joins the space explicitly (which also keeps the door open to opt-in invitations later). An admin can still pre-stage someone into a group before they join — the group's grants just stay dormant until they do. is_principal_space_admin, enforce_last_admin, list_space_principals, and list_spaces_for_member were all updated to match.

Comment on lines +15 to +18
insert into {{schema}}.principal_space (space_id, principal_id, admin)
values (_space_id, _principal_id, _admin)
on conflict (principal_id, space_id) do update set
admin = excluded.admin; -- updated_at maintained by the before-update trigger

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we may need to ensure that a group isn't added to a space other than the space listed on its principal row

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — added that guard in 081ce79. Since add_principal_to_space is the single insert path into principal_space for every kind (provisioning, invite redemption, direct add), I enforced it there: it now rejects rostering a group whose principal.space_id differs from the target space, raising SQLSTATE 23514 (check_violation) so mapCoreError surfaces it as a VALIDATION_ERROR. Users/agents are global (space_id null) so it constrains groups only. Added a migration test for the foreign-space rejection plus the still-valid same-space add.

and principal_id = _principal_id
returning 1
)
select exists (select 1 from del_membership)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function seems to assume that every principal belonging to a space is represented in principal_space, but other functions assume that being in a group in a space implicitly puts you in the space.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved by the same model change (bd65f43): being in a group no longer implicitly puts you in the space, so principal_space is now the single source of truth for membership and the assumption is consistent everywhere. remove_principal_from_space's return value (keyed on the deleted principal_space row) is therefore correct by construction — a group-only principal was never a member.

Comment on lines +152 to +154
select gm.member_id as id, false as direct, false as admin
from {{schema}}.group_member gm
where gm.space_id = _space_id

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we treat principals that are added to admin groups as if they are not admins?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — that was inconsistent with is_principal_space_admin. Fixed in 8c2d51d: list_space_principals now computes admin via is_principal_space_admin(p.id, _space_id) instead of hardcoding false for the group branch, so a user who is a space admin only through an admin group is now reported admin=true. Reusing the function means the listing shares the one canonical definition (direct admin row OR membership of an admin group, never an agent) and can't drift from it — and it also tidies up a latent inconsistency for agents. Extended the admin-group engine test to assert the member shows direct=false, admin=true.

Comment on lines +15 to +18
insert into {{schema}}.tree_access (space_id, principal_id, tree_path, access)
values (_space_id, _principal_id, _tree_path, _access)
on conflict (space_id, principal_id, tree_path) do update set
access = excluded.access -- updated_at maintained by the before-update trigger

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we enforce a check that the principal belongs to the space?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With explicit membership (bd65f43) we deliberately do NOT add this check: a grant to a non-member is harmless because build_tree_access gates effectiveness on a principal_space row, so the grant is simply dormant until the principal joins. That's the same pre-staging we allow for group membership (an admin can add you to a group before you join). So grant_tree_access stays permissive on purpose rather than enforcing space membership.

as $func$
-- The out columns (id, inserted) shadow table columns inside the body; the
-- body never reads them as variables, so resolve ambiguity to the columns.
#variable_conflict use_column

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never seen this before. Neat!

-- re-enqueue the embedding only when content actually changed, so a
-- meta-only replace does not re-embed.
-------------------------------------------------------------------------------
create or replace function {{schema}}.batch_create_memory

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice!

Comment on lines +233 to +235
-- Without resetting vt the row would sit out the full claim lock (~minutes)
-- before retrying; the worker's own rate-limit backoff (honoring Retry-After)
-- paces the actual retry. No-op once the row is terminal.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works if there is a single worker, though, right? But I guess if one worker is hit with a rate limit, the other(s) will too, unless they are using different API keys for the embedding provider?

cevian added a commit that referenced this pull request Jun 16, 2026
…riggers

The last-admin guard fired as plain per-statement AFTER triggers, so a
single transaction that swapped admins (demote the incumbent, then promote
a replacement) was rejected mid-flight on the transient zero-admin state.

Convert the three keep-admin triggers to DEFERRABLE INITIALLY DEFERRED
constraint triggers so the invariant is judged once at commit against the
transaction's final state. Postgres supports neither CREATE OR REPLACE nor
IF NOT EXISTS for constraint triggers, and a bare drop+create would take an
ACCESS EXCLUSIVE lock on every idempotent migration run, so each is wrapped
in a guard that drops+recreates only when the live trigger isn't already a
deferred constraint trigger: it upgrades a pre-existing plain trigger on the
first run, then is a lock-free no-op on every run after.

Tests: an intra-txn admin swap now commits; dropping to zero admins still
rejects with ME001 at commit and rolls back; and the guard upgrades a legacy
plain trigger then skips without churning the trigger oid.

Addresses jgpruitt review comment on PR #71.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cevian added a commit that referenced this pull request Jun 16, 2026
list_space_principals hardcoded admin=false for principals reached through a
group, so a user who is a space admin only via an admin group was listed as a
non-admin — inconsistent with is_principal_space_admin, which treats admin as
transitive through an admin group.

Compute the admin column via is_principal_space_admin(p.id, _space_id) so the
listing shares the one canonical definition (direct admin row OR membership of
an admin group, never an agent) and can't drift from it. This also fixes a
latent inconsistency for agents (never space admins) and simplifies the CTEs.

Update the SpacePrincipal / spacePrincipalResponse docs to "effective
space-admin status" and extend the admin-group engine test to assert the
member now shows direct=false, admin=true in listSpacePrincipals.

Addresses jgpruitt review comment on PR #71.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cevian added a commit that referenced this pull request Jun 16, 2026
A group's principal row fixes it to one space (principal.space_id, non-null
for groups), but add_principal_to_space did a blind upsert into principal_space
with no check — so a group could be rostered into a foreign space.

add_principal_to_space is the single insert path into principal_space for every
kind (provisioning, invite redemption, direct add), so guard it there: convert
to plpgsql and reject adding a group whose principal.space_id differs from the
target space, raising SQLSTATE 23514 (check_violation) so mapCoreError surfaces
it as a VALIDATION_ERROR. Users/agents are global, so this constrains groups
only. Add a migration test covering the foreign-space rejection and the
still-valid same-space add.

Addresses jgpruitt review comment on PR #71.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines +63 to +77
create or replace function {{schema}}.add_group_member
( _space_id uuid
, _group_id uuid
, _member_id uuid
, _admin bool default false
)
returns void
as $func$
insert into {{schema}}.group_member (space_id, group_id, member_id, admin)
values (_space_id, _group_id, _member_id, _admin)
on conflict (space_id, member_id, group_id) do update set
admin = excluded.admin -- updated_at maintained by the before-update trigger
$func$ language sql volatile security invoker
set search_path to pg_catalog, {{schema}}, public, pg_temp
;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should double-check that _space_id and _group_id match the principal table.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or use a constraint trigger to enforce it

@jgpruitt jgpruitt marked this pull request as ready for review June 16, 2026 19:00
jgpruitt and others added 21 commits June 16, 2026 21:03
…ng, postgres.js pilot)

Add integration test suites for the new core/space migration system, run against a real ghost (TigerData) Postgres via TEST_DATABASE_URL. Tests isolate per-schema (core_test_<rand>, me_<slug>) so they run concurrently and parallel-safe across files; bun run test:db runs core + space.

Template the core migration SQL with {{schema}} so tests provision throwaway, isolated cores and never touch a real control plane; production still defaults to 'core' (exposed as CORE_SCHEMA). migrateCore now takes an optional schema.

Pilot the Bun.SQL -> postgres.js driver swap on the migrate path. Bun.SQL fails to return a pooled connection after a query/transaction error (oven-sh/bun#22395, present in 1.3.13 and 1.3.14), which hangs the suites and is a latent production hazard for the long-lived engine/accounts pools. Both postgres.js and pg fix it; postgres.js is a near-drop-in. Converted core/space migrate + bootstrap + scripts/migrate-db.ts + test-utils; verified on local and ghost.

Docs: CLAUDE.md gains a db-integration-test section and the Bun.SQL -> postgres.js migration recipe; TODO.md tracks test-utils/migration-runner consolidation and the core/space packaging question.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Real-DB regression for the max(access) + group by tree_path at the end of
agent_tree_access. An agent with a broad `foo` grant plus a redundant
`foo.bar` grant, clamped against an owner that grants only `foo.bar`, makes
both arms of the inner union emit `foo.bar` at different access levels; the
max collapses them to the single effective row. Without it the function
returns `foo.bar` twice (verified the assertion fails in that case).

Also document the --timeout 30000 needed to run a single integration file
directly against ghost (bun's 5s default overruns the migrating beforeAll
and surfaces as a misleading hook-timeout).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hard-interrupted integration runs (SIGKILL, OOM, a timed-out beforeAll) can
leave throwaway schemas behind. Add scripts/clean-test-schemas.ts and run it
as a pre-step in `test:db`.

Safety is by construction — the sweeper only matches names impossible in
production: `core_test_*` (prod control plane is the bare `core`) and
`metest_*`. To make the latter safe, test spaces now provision under a
`metest_<slug>` prefix instead of the production `me_<slug>`, via a new
optional `schema` override on migrateSpace (mirrors migrateCore; defaults to
slugToSchema(slug), so production is unchanged). `metest_` also avoids the
`me_` engine-schema prefix. Pointed at a real database the sweeper is a no-op.

It is age-gated (drops only schemas older than 60 min) so a concurrent
`test:db` sharing the database is safe; `test:db:clean:all` forces a full
reset. Verified on ghost: drops stale core_test_/metest_, skips fresh ones,
and never touches a production-shaped me_<slug> even with --all.

Update the two space tests that re-invoke migrateSpace on an existing space to
pass the schema override (as the core tests already do), so they target the
test schema rather than provisioning a stray me_<slug>.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cevian and others added 25 commits June 16, 2026 21:03
A --project filter only applied per-session AFTER parsing: discovery
still walked and parsed every transcript under ~/.claude/projects, so a
scoped `me claude init` showed (and paid for) a machine-wide scan —
105k files on a busy machine for a 30-file project.

Claude Code names each per-project directory after the session cwd with
every non-alphanumeric character replaced by `-`, so discovery now skips
directories that can't match the filter (exact encoded name or
`<encoded>-` prefix for descendant cwds). The encoding is lossy (`-` and
`/` collide), so this is a prune only — kept files still pass the exact
per-session cwd filter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
After a successful `me login` with an active space, print a "Next step"
note telling the user to run `me claude init` at the root of a software
development project. Skipped when no space is active — the existing
create/select-a-space hints are the right next step there — and absent
from --json/--yaml output.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Init wrote a CLAUDE.md pointing agents at the me_memory_search MCP tool
and backfilled history — but without `me claude install` no MCP server or
capture hooks exist, so init alone produced a half-working setup. A new
first step runs the same install as `me claude install` (full plugin,
user scope, login-session auth).

Steps gain an optional `available` gate: a step resolving false is
omitted entirely — no multiselect row, not in the non-interactive
baseline. The install step hides itself when the `claude` binary is
absent or `claude plugin list --json` already shows
memory-engine@memory-engine (unparseable output counts as not installed
— a wrong guess costs an idempotent re-install offer, never a missed
one). The probe is skipped for steps opted out via their --skip flag in
non-interactive runs, so `init --skip-plugin-install` never spawns
`claude`.

Also fixes the init e2e fixtures to mirror Claude Code's real on-disk
layout: transcripts now live in encoded-cwd directory names
(encodeProjectDir), which the project-scoped import has pruned by since
4dae311 — the literal "init-proj"/"skip-proj" fixture dirs were never
scanned, a break that commit missed by not re-running e2e. The e2e init
invocations pass --skip-plugin-install so a dev machine's claude never
attempts a real marketplace install against the test HOME.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Clone → ./bun install + install:local → login against the dev server →
`me claude install --dev`. Login precedes the plugin install (it needs
the session and stored server URL), --dev runs from inside the repo, and
the note explains that a dev-installed plugin keeps `me claude init`
from offering the published one over it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI runs every integration file in ONE bun process
(`find … | xargs bun test`), so start.integration.test.ts's module-scope
`process.env.SPACE_SCHEMA_PREFIX = "metest_"` leaked into every suite
that ran after it: their provisionUser created metest_<slug> schemas
while the tests query the hardcoded me_<slug> — 20 handler tests failing
with `schema "me_…" does not exist`. Local runs were green because
`./bun run check` passes --parallel, which isolates files per process.

The prefix is now set in beforeAll and restored in afterAll (the
slug.test.ts save/restore pattern); the e2e suite keeps its module-scope
assignment since it runs in its own invocation. Verified with the exact
CI command shape: all 17 integration files in one process, 225 pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI's bare `find … | xargs bun test` ran every test file in one process
while local runs use --parallel (per-file isolation) — the divergence
that let a module-scope env leak pass locally and break CI (cd05343).
CI now calls package.json scripts so the command shape has one source of
truth: the integration job reuses the existing test:db (whose schema
cleaner is a no-op against the fresh CI container), and a new test:unit
mirrors it for non-integration files. Both carry --parallel, matching
the local process model.

Deliberately NOT `./bun run check`: check is the dev loop — its
`lint --write` would auto-fix violations in the runner instead of
failing, and it merges everything into one job, putting the Docker
Postgres build on the critical path of every lint error.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Surfaced by the previous commit: running the integration suites with
--parallel against the FRESH CI container failed with a unique_violation
from `create extension`. Extensions are database-global, but each
migrator (auth, core, the space bootstrap) serializes only against its
own advisory key — two different migrators racing on a fresh database
both pass ensureExtension's existence check and the loser dies on the
pg_extension catalog insert. Never seen against ghost or an established
deployment because the extensions already exist there; two server
replicas booting against a fresh database could hit the same race.

ensureExtension now takes a single database-wide advisory xact lock
("memory:extensions") before the existence check. The lock is the only
one shared across migrators and is always acquired last, so no ordering
cycle; it holds until the winner's commit makes the extension visible,
at which point the loser's check sees it and no-ops. Re-acquisition
within one migrator's transaction is immediate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI now sets TEST_CI=1 in both jobs: conditional describe.skipIf gates
include !process.env.TEST_CI, so in CI every gated suite RUNS and missing
prerequisites fail loudly as test errors instead of skipping silently
(the e2e suite had been "passing" CI for its entire life without
executing). The integration job gains a test:e2e step (real CLI + real
in-process server + real OpenAI embeddings against the CI Postgres
container) and both jobs receive the OPENAI_API_KEY secret — the unit job
runs the live OpenAI embedding suite. Requires the OPENAI_API_KEY repo
secret; until it exists, CI fails on those suites (by design: silence is
an error). Fork PRs receive no secrets and fail the same way.

The Ollama embedding suite is deleted rather than exempted: it needs a
live local Ollama (dev-only provider, never deployed) and would be the
lone skip CI tolerates. The shared embedding code keeps live coverage
through the OpenAI suite; test the Ollama provider manually against a
local Ollama if touching it.

Locally the rigor inverts: `check` is now the fast inner loop (typecheck
+ lint + unit tests, no database, ~13s) and `check:full` carries the old
everything-chain — including actually running e2e, which the old check
silently skipped without TEST_DATABASE_URL (it now defaults to ghost).
CLAUDE.md documents the convention: new skipIf gates must include
!process.env.TEST_CI.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Test verification in CLAUDE.md now points TEST_DATABASE_URL at the local
Docker Postgres (same image CI builds): the full suite drops ~4min→10s
and e2e ~65s→18s — WAN round trips to ghost dominate these
statement-chatty suites. The ghost testing_me instructions stay, scoped
to explicit ghost testing only; the scripts' ghost fallback (when
TEST_DATABASE_URL is unset) is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The `test`/`check:full` scripts no longer fall back to the remote ghost
instance when TEST_DATABASE_URL is unset — a silent fallback to a shared
remote database contradicts the local-first verification policy (and for
anyone without the ghost CLI it degraded into an empty-string URL and a
confusing connection error). They now default to the local me-postgres
container, so the bare commands are the fast path; the localhost default
also keeps `check:full`'s e2e leg running (its skip gate keys off the
env var being set). Ghost remains available by passing
TEST_DATABASE_URL="$(ghost connect testing_me)" explicitly; CI overrides
with its own container env as before.

Bare `./bun run check:full` verified: ~30s, full suite + e2e green
against the local container.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s to Usage

The quick start now tells the golden-path story — init at a project root
does everything (plugin install, session + git history backfill, CLAUDE.md
pointer). The create/search/install commands move to a Usage section,
which also surfaces the `me import` group. Two fixes while here: the
create example writes to share.* (an arbitrary top-level tree needs a
grant most members don't hold), and "Row-Level Security" is replaced with
the actual model (tree-scoped grants in SQL; RLS was removed as
unperformant).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tory

New `me import git-hook [repo]` installs a marker-delimited managed block
into the repo's effective post-commit hook (worktree-aware via
`git rev-parse --git-path hooks`): a backgrounded, silenced
`me import git` with an absolute invocation, so commits from GUI clients
work and the commit never blocks or fails. Because the import is
high-water incremental, any fire catches up the entire backlog (pulls,
merges, rebases included) — post-commit alone suffices. Re-install
replaces the block in place; a foreign hook is preserved and appended to;
`--remove` deletes the block (and the file when only the shebang
remains). Repos routing hooks through core.hooksPath (husky etc.) are
refused with instructions instead of writing into committed files.

`me claude init` gains a git-hook step (after git import, `--skip-git-hook`),
hidden when not applicable or already installed.

Unit tests cover the block upsert/remove helpers; a new e2e test proves
the full loop — install, commit, memory appears, --remove.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The init availability gate becomes a tri-state: "available" (offer the
step), "hidden" (not applicable — no claude binary, not a git repo, a
core.hooksPath manager), or "done" (already set up). Done steps print a
green ✓ line above the multiselect — "Claude Code plugin already
installed" / "Git post-commit hook already installed" — instead of
disappearing silently, so users know why the rows are missing.

isGitHookInstallable becomes gitHookStatus
(installable | not-applicable | installed) to carry the distinction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Print the already-done lines as one block: clack's default spacing of 1
puts a bare guide line above each log message, so the second and later
✓ lines now pass spacing 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
After the init steps complete, print a note telling the user what the
wiring buys them: Claude now draws on the project's memories (past
sessions, git history) automatically for history/architecture questions
and while exploring for new features — plus example prompts for
invoking memories explicitly. Text mode only; skipped when nothing ran.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Generic "memory" in the example prompts didn't tell Claude which tool
to reach for — name the product so the prompts reliably route to the
me MCP tools.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…riggers

The last-admin guard fired as plain per-statement AFTER triggers, so a
single transaction that swapped admins (demote the incumbent, then promote
a replacement) was rejected mid-flight on the transient zero-admin state.

Convert the three keep-admin triggers to DEFERRABLE INITIALLY DEFERRED
constraint triggers so the invariant is judged once at commit against the
transaction's final state. Postgres supports neither CREATE OR REPLACE nor
IF NOT EXISTS for constraint triggers, and a bare drop+create would take an
ACCESS EXCLUSIVE lock on every idempotent migration run, so each is wrapped
in a guard that drops+recreates only when the live trigger isn't already a
deferred constraint trigger: it upgrades a pre-existing plain trigger on the
first run, then is a lock-free no-op on every run after.

Tests: an intra-txn admin swap now commits; dropping to zero admins still
rejects with ME001 at commit and rolls back; and the guard upgrades a legacy
plain trigger then skips without churning the trigger oid.

Addresses jgpruitt review comment on PR #71.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Restructure `me claude init` so steps are grouped by source — Claude Code
sessions and git history each pair a one-time backfill of existing data with
ongoing capture going forward, plus a project-config group for the CLAUDE.md
pointer. The interactive picker uses a grouped multiselect so each backfill
sits next to its ongoing-capture counterpart.

Already-done steps are now offered in the picker as unchecked "re-run" rows
(reinstall plugin / hook, rewrite pointer) instead of being dropped; non-
interactive runs still report them as ✓ lines. The CLAUDE.md step gains an
up-to-date check (projectMemoryPointerUpToDate) so a present-and-current
block reports done while a stale one stays offered for refresh.

init closes with a recap (initOutroLead) summarizing what is now covered —
historical data imported and/or hooks keeping it current — derived from each
step's new `kind` (backfill | ongoing | config). Docs and tests updated.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
list_space_principals hardcoded admin=false for principals reached through a
group, so a user who is a space admin only via an admin group was listed as a
non-admin — inconsistent with is_principal_space_admin, which treats admin as
transitive through an admin group.

Compute the admin column via is_principal_space_admin(p.id, _space_id) so the
listing shares the one canonical definition (direct admin row OR membership of
an admin group, never an agent) and can't drift from it. This also fixes a
latent inconsistency for agents (never space admins) and simplifies the CTEs.

Update the SpacePrincipal / spacePrincipalResponse docs to "effective
space-admin status" and extend the admin-group engine test to assert the
member now shows direct=false, admin=true in listSpacePrincipals.

Addresses jgpruitt review comment on PR #71.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A group's principal row fixes it to one space (principal.space_id, non-null
for groups), but add_principal_to_space did a blind upsert into principal_space
with no check — so a group could be rostered into a foreign space.

add_principal_to_space is the single insert path into principal_space for every
kind (provisioning, invite redemption, direct add), so guard it there: convert
to plpgsql and reject adding a group whose principal.space_id differs from the
target space, raising SQLSTATE 23514 (check_violation) so mapCoreError surfaces
it as a VALIDATION_ERROR. Users/agents are global, so this constrains groups
only. Add a migration test covering the foreign-space rejection and the
still-valid same-space add.

Addresses jgpruitt review comment on PR #71.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An agent's `~` now resolves to home.<ownerId>.<agentId> instead of
home.<agentId>, and add_principal_to_space grants a joining agent
owner@home.<ownerId>.<agentId>. Because the owner holds owner@home.<ownerId>,
the nested grant is covered and agent_tree_access keeps it effective (a bare
home.<agentId> grant was clamped to nothing). Agents are now usable on join
instead of getting "No access", and an owner can see what its agent stores
under ~.

- 006_membership.sql: kind-aware home grant (user vs nested agent)
- space/path.ts: homePrefix(id, ownerId?) + homeOwner option on TreePathOptions
- validate_api_key returns owner_id; plumbed through ValidatedApiKey,
  SpaceAuthContext/SpaceRpcContext (ownerId), and support.ts homeOpts()
- docs + CLAUDE.md updated; the two implemented DECISIONS_FOR_REVIEW entries
  removed and the open "agent share-on-join" entry refreshed

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r it

Space membership is now a principal_space row, full stop. A group's grants
(and admin, if it's an admin group) are effective only for a member who ALSO
holds a principal_space row — group membership alone never confers space
access. Joining (invite redemption / direct add) is the single membership path;
an admin can still pre-stage a member into a group before they join (the grant
stays dormant until they do). This avoids a second invitation path and keeps
the door open to opt-in invitations later.

Core SQL:
- member_tree_access gates group grants on is_principal_in_space, so
  build_tree_access (the server auth gate) is empty for a non-member.
- is_principal_space_admin + enforce_last_admin: admin via an admin group now
  requires direct membership (function-only; triggers unchanged).
- list_space_principals lists only direct members and drops the now-vestigial
  `direct` column (guarded drop: only a stale signature is dropped, no churn).
- list_spaces_for_member (space.list) lists only direct memberships.

TS/protocol: drop `direct` from SpacePrincipal / spacePrincipalResponse and
their mappers.

Resolves jgpruitt's #2 (explicit membership); with it #3
(remove_principal_from_space's return value is now correct by construction) and
#6 (grants to non-members are harmless/dormant) fall out. Tests + docs updated.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cevian cevian merged commit 609cc55 into main Jun 16, 2026
3 checks passed
@cevian cevian deleted the multiplayer branch June 16, 2026 19:12
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.

3 participants