Skip to content

Commit 2a5fc43

Browse files
TheGreatAxiosTheGreatAxios
committed
docs: trim CONTRIBUTING, add AGENTS.md, drop SECURITY.md
1 parent 84b19fd commit 2a5fc43

5 files changed

Lines changed: 339 additions & 499 deletions

File tree

‎.github/SECURITY.md‎

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎AGENTS.md‎

Lines changed: 321 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,321 @@
1+
# AGENTS.md
2+
3+
## Purpose
4+
5+
`@corbits/artifacts` stores artifacts, their versions and uploads for an
6+
Interchange host. It owns the three tables and their migrations, the HTTP
7+
surface with its validation and status codes, the version and archive
8+
semantics, the upload gate (`createFileArtifact` refuses anything outside its
9+
required `policy` before the `ContentStore` is touched) and the download path
10+
with its `nosniff`/`attachment` behaviour. The host supplies the
11+
`Hono<TenantEnv>` app and database handle, the authenticated tenant and
12+
principal, `RequireGrant`, display-only provenance decoration and a
13+
`ContentStore`.
14+
15+
## Layout
16+
17+
- `src/mount.ts` — `createArtifactRoutes`: parsing, validation, status codes; wires the host's `requireGrant`.
18+
- `src/workflow-mount.ts` — `createWorkflowArtifactRoutes`, the run-scoped routes for callers with no browser session.
19+
- `src/artifacts.ts` — the core domain: create, revise, find-or-version, list, get, archive, serialize.
20+
- `src/uploads.ts` — `createFileArtifact`, the MIME policies and the size caps.
21+
- `src/download.ts` — one download path over the three storage conventions.
22+
- `src/preview.ts`, `src/counts.ts` — previews and segment counts over the list query.
23+
- `src/content-store.ts` — the two shipped `ContentStore` implementations.
24+
- `src/tools.ts` — agent-facing tool definitions and windowed reads (caller tenant only).
25+
- `src/sidecar-bundle.ts` — the `./sidecar-bundle` entry the tool-package loader invokes.
26+
- `src/ports.ts` — the `ContentStore` type and the shared `ResolvedPrincipal` shape.
27+
- `src/schema.ts`, `src/migrations.ts` — the three tables and `runArtifactMigrations`.
28+
- `src/db.ts`, `src/arktype.ts` — the db handle type and arktype config.
29+
- `src/index.ts` — the package entry.
30+
- `e2e/` — real-Postgres suites; `e2e/reference-host.test.ts` runs the acceptance scenarios against `examples/reference-host`.
31+
32+
## Rules
33+
34+
- Nothing is mocked at the database boundary. Destructive test paths refuse to run unless `ALLOW_DESTRUCTIVE_ARTIFACT_TESTS=1` and the database is allowlisted (`artifact_core`, or a name ending in `_test`).
35+
- The coverage floor is 80% of lines and functions per file (`bunfig.toml`); `bun run test:coverage` enforces it.
36+
- A change to the route factory, a port or host wiring is shown working in `examples/reference-host`.
37+
- No `@workbench/*` imports: it is an unpublished scope.
38+
- A new `ContentStore` passes the same suite the shipped ones do, with no special cases.
39+
- `schema.ts` and `migrations/` change together; every migration statement is idempotent and every backfill is cheap once it has run.
40+
- One 404 covers never minted, malformed and another tenant's id; never add an existence oracle.
41+
- Every `any` or cast carries a comment saying why the type system leaves no alternative.
42+
43+
## Internals
44+
45+
### Where the routes are served
46+
47+
The core registers root-relative paths (`/artifacts*`) and takes no base path, so the _mount point_
48+
is the host's decision. The convention every `@corbits/*-core` package
49+
documents, and every example here demonstrates, is **`/api`** — the same prefix
50+
Interchange serves its own routes under (`app.route("/api/me", …)`,
51+
`app.route("/api/tenants", …)`). No `/v1` segment, no vendor prefix.
52+
53+
```ts
54+
// Host middleware has already placed `tenant` and `principal` on the context.
55+
app.route("/api", createArtifactRoutes({ db, contentStore, requireGrant }));
56+
```
57+
58+
which serves `/api/artifacts`, `/api/artifacts/:id`,
59+
`/api/artifacts/:id/versions`, and `/api/artifacts/:id/download`. Returning a sub-app rather than
60+
taking a base path keeps the factory free of a configurable base path.
61+
62+
Everything else it needs arrives through `deps` or the host's request context.
63+
Nothing is reached for.
64+
65+
### The route factory
66+
67+
`createArtifactRoutes(deps): Hono<TenantEnv>` returns a sub-app typed with
68+
Interchange's `TenantEnv`, built the way hub-api's `createGrantRoutes` is, so it
69+
composes beneath Interchange auth + tenant middleware. The host places full
70+
`tenant` and `principal` rows on the context; this package reads them natively
71+
and never invents a second principal resolution path.
72+
73+
Three options have no sensible default — `db`, `contentStore`, `requireGrant` —
74+
and the rest degrade a _feature_, never safety, when omitted. The README's option
75+
tables are the reference; what matters architecturally is
76+
that optional seams **fail closed**: no `decorate` means no decoration.
77+
78+
What the package does **not** require of a host: no session library, no UI, no
79+
directory, no owner/admin policy callback. What it DOES require: Interchange's
80+
control plane — `public.tenant` and `public.principal` must exist before the
81+
migrations run, because the tables carry hard foreign keys into them — and a
82+
host that puts the authenticated principal on `TenantEnv` and hands in its
83+
`RequireGrant`.
84+
85+
The principal's tenant is authoritative — there is no caller-supplied tenant
86+
override anywhere in the route or tool surface. Tool reads always stay inside
87+
`scope.tenantId`.
88+
89+
**This is a decision, not an oversight.** The prior `Identity` port let
90+
`readArtifact` / `readArtifactChunk` take a `tenantId` argument and cross into
91+
it when `identity.ownerIsMemberOfTenant(scope, tenantId)` said the caller's
92+
owner belonged there — a membership check this package invented and owned.
93+
That is exactly the kind of policy this PR removes. It is not replaced by a
94+
grant check, and won't be by a later one either: Interchange's `GrantStore`
95+
resolves a principal's grants **within one tenant**
96+
(`collectGrants(principalId, tenantId)`; `@intx/db`'s implementation filters
97+
`grant` rows by `tenant_id`, and a principal is itself a row scoped to one
98+
tenant). There is no platform primitive for "principal P, home tenant A, holds
99+
a grant readable from tenant B" to check — inventing one here would mean this
100+
package building a second, bespoke cross-tenant authorization concept on top
101+
of the platform's, which is the precise failure mode "authorization is the
102+
host's job" is meant to prevent. If a real product need for cross-tenant
103+
artifact reads shows up, it belongs in Interchange's grant model, not
104+
re-derived per package.
105+
106+
### Three custom seams
107+
108+
Beyond the host's native context and grants, this package exposes **three**
109+
extension seams: the substrate (`ContentStore`), a display-only decorator
110+
(`decorate` / provenance), and a grant-provisioning hook (`onArtifactCreated`).
111+
Authorization is not a custom seam — it is the host's Interchange `RequireGrant`;
112+
`onArtifactCreated` is not authorization either, it is the write side of the
113+
same idea — the host deciding what makes its grant model true, this package
114+
only handing it the row and the scope that made it.
115+
116+
### Row decoration
117+
118+
`decorate`'s display-only status is a contract, not a convention: it may add
119+
fields to rows on their way out and must never affect _what_ is returned or
120+
_who_ may see it. Joining a host's workflow tables inside this package would
121+
couple it to a schema it must not know, so the host supplies the decorator.
122+
Clients that need an owner display name resolve `ownerPrincipalId` themselves;
123+
this package never ships directory names on the wire.
124+
125+
### Grant provisioning (`onArtifactCreated`)
126+
127+
Checking a grant (`requireGrant`) and minting one (`onArtifactCreated`) are the
128+
same host responsibility looked at from both ends: this package neither
129+
invents authorization policy nor decides who a newly created row belongs to
130+
for grant purposes — it hands the host the row, inside the transaction that
131+
made it durable, and the host decides.
132+
133+
Creating needs its own grant, `create` on `artifact:*`, checked before the
134+
body is read; the reference host seeds it for every principal in its tenant.
135+
`examples/reference-host` then provisions a real `creator`-origin grant on create —
136+
`write` and `archive` on `artifact:<id>` for the creating principal, inserted
137+
into Interchange's own `grant` table via `@intx/db`'s schema, in the same
138+
transaction as the artifact row. Its `buildApp`'s default `requireGrant` is the
139+
platform's real `createRequireGrant` over that same table (via
140+
`createGrantStore`), not a stub — a principal with no matching row is refused,
141+
exactly as in production. See `grantOwnership` in
142+
`examples/reference-host/src/index.ts` and the "ownership-derived grants"
143+
scenarios in its acceptance suite for the end-to-end proof: the creator
144+
succeeds, a co-tenant with no grant does not.
145+
146+
### ContentStore
147+
148+
`ContentStore` is the substrate seam:
149+
150+
- `put(tx, scope, blob)` persists bytes **inside the caller's transaction** and
151+
returns the two artifact-row fields a stored file determines (`content` and
152+
`source`). Taking the transaction rather than the handle is what makes "bytes,
153+
artifact row, and version 1" a single atomic write — a failure anywhere leaves
154+
no orphan bytes and no orphan artifact.
155+
- `get(db, artifact)` resolves **out-of-band** bytes, or `null` when the store
156+
keeps content in the row itself. It is tenant-scoped: a reference resolving to
157+
another tenant's bytes must return `null`.
158+
159+
Two implementations ship and both pass the same suite: `InlineContentStore`
160+
(bytes in a tenant-owned `upload` row, referenced by `source.upload.id`) and
161+
`DataUrlContentStore` (bytes inline in `content` as a base64 `data:` URL, no
162+
side-table row, so its `get` returns `null`). A third backend — object storage,
163+
say — is a third implementation, not a rewrite. The single download path
164+
resolves the storage conventions in precedence order rather than branching on
165+
which store is installed.
166+
167+
### Data model
168+
169+
Three physical tables — `artifact`, `artifact_version`, `upload`.
170+
171+
**Hard control-plane foreign keys, by design.** `tenant_id` is `NOT NULL` and
172+
references the host's `tenant(id)` (`ON DELETE CASCADE` — a deleted tenant takes its
173+
artifacts with it) and `principal_id` / `owner_principal_id` reference
174+
the host's `principal(id)` (`ON DELETE SET NULL` — a removed principal detaches its
175+
artifacts rather than destroying them). This package is coupled to Interchange:
176+
it mounts on Interchange-shaped hosts only, and the host's own migrations must
177+
have run before `runArtifactMigrations`. The internal key —
178+
`artifact_version.artifact_id` — cascades with its artifact.
179+
180+
**Cheap row-local CHECKs.** `artifact.version` and `artifact_version.version`
181+
must be ≥ 1; `upload.size` must be ≥ 0. These are
182+
single-column constraints — free at write time.
183+
184+
**Principal↔tenant alignment is host-owned.** The package FKs each column into
185+
the control plane independently; it does **not** enforce that `principal_id` (or
186+
`owner_principal_id`) belongs to the same tenant as `tenant_id`. A multi-table
187+
trigger or composite FK into `public.principal` would couple every write to a
188+
control-plane lookup and is deliberately out of scope. The host's middleware and
189+
context are the authority: routes stamp the `(tenantId, principalId)` pair from
190+
the Interchange `principal` already on `TenantEnv`, so a correctly mounted host
191+
never plants a cross-tenant principal. Operators cleaning legacy rows before the
192+
`tenant_id NOT NULL` migration must assign a valid tenant or delete orphans —
193+
the migration fails with an explicit message if null `tenant_id` rows remain.
194+
195+
**`kind` is free-form text, not a pg enum,** validated at the application edge.
196+
New kinds cost no migration. What is _not_ free-form is the import allowlist:
197+
`POST /api/artifacts` may only mint `link` or `document`, so an untrusted caller
198+
cannot stamp a file-shaped, downloadable kind onto a row whose content is a URL
199+
or a pasted body.
200+
201+
**History is append-only.** Every create and every revision writes an
202+
`artifact_version` row — including version 1, written eagerly with the artifact —
203+
so a version-pinned read always resolves. The version bump has a double guard:
204+
`SELECT ... FOR UPDATE` serializes writers, and the `(artifact_id, version)`
205+
unique constraint makes a racing writer that somehow computed the same next
206+
version fail loudly rather than corrupt history.
207+
208+
**Archival is a soft hide.** `archived_at` null means visible; a timestamp means
209+
hidden from discovery. Deep links to archived artifacts still load.
210+
211+
**Find-or-version is a package primitive, not a constraint.** The only
212+
uniqueness the schema enforces is `(artifact_id, version)` — there is no
213+
constraint on `(tenant_id, title, kind)`, so "find by title, create if
214+
absent, add a version if present" is not naturally atomic: a plain
215+
read-then-write of that pattern races, and two concurrent callers can both
216+
observe NOT FOUND and both create, leaving two artifacts with the same
217+
title.
218+
219+
A uniqueness constraint on `(tenant_id, title, kind)` was considered and
220+
rejected — not just for now, but structurally. `createArtifact` is a
221+
public, unconditional insert with no title lookup of its own, called
222+
directly by the import route, the upload path, and `artifact_link_file`.
223+
Two independent creates sharing a title is normal, intended behavior on
224+
every one of those paths — a coworker uploading `report.pdf` twice, or two
225+
agents each linking a file named `notes.md`, are not bugs. A hard
226+
`UNIQUE(tenant_id, title, kind)` constraint would reject those ordinary
227+
inserts outright, not just gate on a one-time backfill of legacy
228+
duplicates. Uniqueness on that triple is a property of the _find-or-version
229+
pattern specifically_, not an invariant of the table, so it does not belong
230+
in the schema — it belongs exactly where it now lives, inside the one code
231+
path that promises it.
232+
233+
Separately, this package also has no way to confirm existing tenants are
234+
already free of duplicate `(title, kind)` rows, which would make even a
235+
scoped constraint risky to backfill. That is not the main reason for
236+
rejecting the constraint, and it is not by itself decisive.
237+
238+
Instead, `findOrVersionArtifact(db, args)` (in `artifacts.ts`) closes the
239+
race with a transaction-scoped advisory lock keyed by
240+
`hashtext(tenantId, kind, title)`, in its own lock-space namespace (the
241+
two-`int4`-argument form of `pg_advisory_xact_lock`, disjoint from the
242+
single-`bigint` form `runArtifactMigrations` uses). Collision semantics:
243+
whichever concurrent caller acquires the lock first creates the artifact;
244+
every other caller for the identical `(tenantId, kind, title)` blocks, then
245+
finds the row the winner just committed and revises it. Two overlapping
246+
callers always converge on ONE artifact with two versions, never two rows —
247+
callers for a different tenant, kind, or title never contend with each
248+
other. This guarantee holds only for callers that go through
249+
`findOrVersionArtifact`; a caller that instead calls `createArtifact`
250+
directly is unconstrained by design, as above, and a caller that hand-rolls
251+
its own find-then-create against a _different_ lock is not serialized
252+
against this one — the primitive closes the race for its own call path, not
253+
for every possible way to write an artifact.
254+
255+
**`upload` is never a standalone resource.** There is no `POST /uploads`; every
256+
upload eagerly mints its artifact, and the row is reachable only through
257+
`source.upload.id`.
258+
259+
The list index is `(tenant_id, updated_at, id)`. The `id` is the list's
260+
tie-break and must be _in_ the index, or the keyset cursor's row-value
261+
comparison falls out of the index condition into a filter and drags a sort
262+
behind it.
263+
264+
### Migration runner
265+
266+
`runArtifactMigrations(config, { schema })` takes the same arguments as
267+
Interchange's `runMigrations`, and a host calls it right after that, with the
268+
same values. `schema` is where the host's `tenant` and `principal` tables live;
269+
the runner rewrites the `"public".` foreign-key references in the SQL files to
270+
it. It is idempotent and safe to call on every boot of every replica.
271+
272+
- The whole run is one transaction whose first statement takes a
273+
**transaction-scoped** advisory lock, so the lock releases on commit or
274+
rollback and there is no unlock call to lose on an error path.
275+
`CREATE TABLE IF NOT EXISTS` is not itself race-safe, so the lock — not the
276+
`IF NOT EXISTS` — is what makes concurrent cold starts safe.
277+
- The runner opens its own single-connection client and discards NOTICEs, so a
278+
re-run, where Postgres answers every `IF NOT EXISTS` with a NOTICE, prints
279+
nothing.
280+
- Event timestamps (`created_at`, `updated_at`, `archived_at`) are
281+
**`timestamptz`**. List keyset cursors project through `AT TIME ZONE 'UTC'`
282+
and compare with `::timestamptz`, so paging and date filters stay on the
283+
absolute instant under any session `TimeZone`.
284+
- Releases up to 0.1.0 kept a checksum ledger in `artifacts.migrations`.
285+
`0002_drop_migration_ledger.sql` removes it; a database migrated by 0.1.0
286+
already has the shape `0001_artifacts.sql` creates, so its statements no-op.
287+
288+
**The package owns its own Postgres schema.** Every table and index lives in
289+
`artifacts`, created by the runner and qualified in every DDL statement and
290+
every query — nothing resolves through `search_path`, so the package shares a
291+
database with the host's control plane without ever being able to collide with
292+
(or silently adopt) a host table of the same name. The coupling to the host is
293+
explicit instead: `tenant_id` and the principal columns are hard FKs into the
294+
host schema's `tenant` / `principal` (see the data model).
295+
296+
### Known limits
297+
298+
- **No file parsing, ever.** No PDF parser, no spreadsheet parser, no text
299+
extractor, and none is planned. An extractor is a heavyweight, fast-moving
300+
native dependency, and what the extracted text is _for_ is the host's product.
301+
The contract the package offers a parsing host instead is **parse before you
302+
store**: `createFileArtifact` is the only way a file becomes an artifact, so a
303+
host that parses first and fails leaves nothing orphaned.
304+
- **Two of the three MIME allowlists are host-owned surfaces.** This package
305+
ships all three constants but only owns the gallery import route; spreadsheet
306+
ingest and attachment divert are the host's routes calling this package's gate.
307+
- **Upload caps are fixed constants,** not configuration: 10 MB per file, 50
308+
files and 100 MB per request.
309+
- **`InlineContentStore` keeps bytes in Postgres.** That is a deliberate
310+
zero-dependency default, not a recommendation at scale; a large corpus wants a
311+
`ContentStore` over object storage.
312+
- **List paging caps at 100** rows (default 20).
313+
- **One 404 covers three causes** for a resolved caller — never minted,
314+
malformed, or another tenant's. Distinguishing them would be
315+
an existence oracle. Expect no more detail than that from the API.
316+
317+
## Local development
318+
319+
```sh
320+
bun install && bun run check
321+
```

0 commit comments

Comments
 (0)