Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
216 changes: 216 additions & 0 deletions server/src/__integration__/workspace-deletion-orphans.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
/**
* Deleting a workspace has to take the agents' data with it.
*
* The purge sweeps a list of soft-scoped tables with
* `DELETE FROM <t> WHERE company_id = $1`. That is only as good as the
* company_id the writers put there, and two of them do not put one:
*
* agent_workspace the agent's own filesystem endpoint (fs-endpoints.ts)
* wrote (agent_id, path, body, meta) and no tenant, so the
* column stayed NULL — while every other writer of that
* table (skills.ts, cli.ts, router.ts) supplies it.
* agent_climate the column carries DEFAULT 'personal' and NEITHER of its
* two INSERT sites names it, so every climate row in every
* workspace is labelled 'personal'.
*
* Both survive a `company_id = $1` sweep. What is left behind is an agent's
* memory files and its written notes about the people it worked with, for a
* workspace the owner deleted.
*
* The existing purge test does not catch this because it seeds its rows with a
* correct company_id — the shape the broken writers never produce. These seed
* rows the way the real writers do.
*
* Run: INTEGRATION_DATABASE_URL=… npm run test:integration
*/
import assert from 'node:assert/strict'
import { createServer, type Server } from 'node:http'
import { after, before, beforeEach, test } from 'node:test'
import { signAgentToken } from '../agents/runtime/jwt.js'
import { pool } from '../db/pool.js'
import { drainWorkspaceCleanupJobs } from '../workspace-cleanup.js'
import { buildApiTestApp, ensureSchemaOnce, resetAllTables, teardownAll } from './_helpers.js'

const OWNER_ID = 'u-orphan-owner'
const COMPANY_ID = 'co-orphan'
const AGENT_ID = 'agent-orphan'
let ownerServer: Server
let ownerBase = ''
let runtimeServer: Server
let runtimeBase = ''

before(async () => {
await ensureSchemaOnce()
const app = await buildApiTestApp(OWNER_ID)
await new Promise<void>((resolve) => {
ownerServer = createServer(app).listen(0, () => {
const address = ownerServer.address()
assert.ok(address && typeof address === 'object')
ownerBase = `http://127.0.0.1:${address.port}`
resolve()
})
})

// The agent's own filesystem endpoint, mounted the way runtime-server.test.ts
// does — the real index.ts entrypoint would boot schedulers and Redis.
const expressMod = await import('express')
const { runtimeRouter } = await import('../agents/runtime/server.js')
const runtimeApp = expressMod.default()
runtimeApp.use('/runtime', runtimeRouter)
await new Promise<void>((resolve) => {
runtimeServer = createServer(runtimeApp).listen(0, () => {
const address = runtimeServer.address()
assert.ok(address && typeof address === 'object')
runtimeBase = `http://127.0.0.1:${address.port}`
resolve()
})
})
})

beforeEach(async () => { await resetAllTables() })
after(async () => {
if (runtimeServer?.listening) await new Promise<void>((r) => runtimeServer.close(() => r()))
await teardownAll(ownerServer)
})

/** A workspace the owner can actually delete: they must have another one, or
* the route refuses ("cannot delete your only workspace"). */
async function seedDeletableWorkspace(): Promise<void> {
await pool.query(
`INSERT INTO users (id, email, display_name, tier) VALUES ($1, $2, $1, 'pro')`,
[OWNER_ID, `${OWNER_ID}@test.local`],
)
for (const [id, slug] of [[COMPANY_ID, COMPANY_ID], ['co-orphan-other', 'co-orphan-other']]) {
await pool.query(
`INSERT INTO companies (id, name, slug, owner_user_id) VALUES ($1, $2, $3, $4)`,
[id, `Workspace ${id}`, slug, OWNER_ID],
)
await pool.query(
`INSERT INTO company_members (company_id, user_id, role) VALUES ($1, $2, 'owner')`,
[id, OWNER_ID],
)
}
await pool.query(
`INSERT INTO participants
(id, company_id, kind, name, role, initial, avatar_bg, status, system_prompt)
VALUES ($1, $2, 'agent', 'Orphan agent', 'ops', 'O', '#abcdef', 'avail', 'test')`,
[AGENT_ID, COMPANY_ID],
)
}

/** Exactly what fs-endpoints.ts wrote: no company_id column at all. */
async function writeFuseStyleMemory(): Promise<void> {
await pool.query(
`INSERT INTO agent_workspace (agent_id, path, body, meta, updated_at)
VALUES ($1, 'memory/MEMORY.md', 'what I learned about the team', '{}'::jsonb, NOW())`,
[AGENT_ID],
)
}

/** Exactly what climate.ts and cli.ts write: no company_id, so DEFAULT 'personal'. */
async function writeClimateNote(): Promise<void> {
await pool.query(
`INSERT INTO agent_climate (agent_id, about_id, affinity, trust, last_note, updated_at)
VALUES ($1, $2, 0.5, 0.5, 'finds review feedback blunt', NOW())`,
[AGENT_ID, OWNER_ID],
)
}

async function deleteWorkspace(): Promise<Response> {
const response = await fetch(`${ownerBase}/api/companies/${COMPANY_ID}`, {
method: 'DELETE',
headers: { 'content-type': 'application/json', 'x-company-id': COMPANY_ID },
body: JSON.stringify({ confirmation: `Workspace ${COMPANY_ID}` }),
})
await drainWorkspaceCleanupJobs()
return response
}

async function countFor(table: string): Promise<number> {
const { rows } = await pool.query<{ count: string }>(
`SELECT COUNT(*)::text AS count FROM ${table} WHERE agent_id = $1`, [AGENT_ID],
)
return Number(rows[0].count)
}

// ── the two writers whose rows the company_id sweep cannot see ──────────────

test('[integration] deletion takes memory written through the agent filesystem', async () => {
await seedDeletableWorkspace()
await writeFuseStyleMemory()
assert.equal(await countFor('agent_workspace'), 1, 'precondition: the row exists')

const response = await deleteWorkspace()
assert.equal(response.status, 200, await response.text())
assert.equal(await countFor('agent_workspace'), 0, 'agent memory outlived the workspace')
})

test('[integration] deletion takes the notes an agent wrote about people', async () => {
await seedDeletableWorkspace()
await writeClimateNote()
assert.equal(await countFor('agent_climate'), 1, 'precondition: the row exists')

const response = await deleteWorkspace()
assert.equal(response.status, 200, await response.text())
assert.equal(await countFor('agent_climate'), 0, 'climate notes outlived the workspace')
})

test('[integration] a row the sweep already reached is still removed', async () => {
// The company_id path has to keep working — the fix adds a second sweep, it
// does not replace the first.
await seedDeletableWorkspace()
await pool.query(
`INSERT INTO agent_workspace (agent_id, path, body, company_id, updated_at)
VALUES ($1, 'skills/x/SKILL.md', 'ok', $2, NOW())`,
[AGENT_ID, COMPANY_ID],
)
const response = await deleteWorkspace()
assert.equal(response.status, 200, await response.text())
assert.equal(await countFor('agent_workspace'), 0)
})

test('[integration] another workspace keeps its own agents data', async () => {
// Deleting by owner must not reach past the workspace being deleted.
await seedDeletableWorkspace()
await pool.query(
`INSERT INTO participants
(id, company_id, kind, name, role, initial, avatar_bg, status, system_prompt)
VALUES ('agent-bystander', 'co-orphan-other', 'agent', 'Bystander', 'ops', 'B', '#123456', 'avail', 'test')`,
)
await pool.query(
`INSERT INTO agent_workspace (agent_id, path, body, meta, updated_at)
VALUES ('agent-bystander', 'memory/MEMORY.md', 'untouched', '{}'::jsonb, NOW())`,
)
await writeFuseStyleMemory()

const response = await deleteWorkspace()
assert.equal(response.status, 200, await response.text())

const { rows } = await pool.query<{ count: string }>(
`SELECT COUNT(*)::text AS count FROM agent_workspace WHERE agent_id = 'agent-bystander'`,
)
assert.equal(Number(rows[0].count), 1, 'the other workspace lost data')
})

// ── and the writer that produced the orphans in the first place ─────────────

test('[integration] a write through the agent filesystem carries its workspace', async () => {
// Pins the other half: the sweep above cleans up rows already written, this
// stops new ones being written tenant-less. Straight SQL in the tests above
// cannot catch a regression here, because it bypasses the endpoint.
await seedDeletableWorkspace()
const token = signAgentToken({ agentId: AGENT_ID, companyId: COMPANY_ID })
const response = await fetch(`${runtimeBase}/runtime/fs/write`, {
method: 'PUT',
headers: { 'content-type': 'application/json', authorization: `Bearer ${token}` },
body: JSON.stringify({ path: 'memory/MEMORY.md', body: 'written through the agent fs' }),
})
assert.equal(response.status, 200, await response.text())

const { rows } = await pool.query<{ company_id: string | null }>(
`SELECT company_id FROM agent_workspace WHERE agent_id = $1 AND path = 'memory/MEMORY.md'`,
[AGENT_ID],
)
assert.equal(rows.length, 1, 'the write did not land')
assert.equal(rows[0].company_id, COMPANY_ID, 'the row was written without a workspace')
})
15 changes: 13 additions & 2 deletions server/src/agents/runtime/fs-endpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,23 @@ export function attachFsEndpoints(
const meta = p.startsWith('memory/')
? await memoryMetaForWrite(c.sub, { path: p, conversationId: body?.conversationId ?? null })
: metaForPath(p)
// company_id is what workspace deletion sweeps on
// (`DELETE FROM agent_workspace WHERE company_id = $1`). Every other writer
// of this table — skills.ts, cli.ts, router.ts, the baseline backfill —
// supplies it from the agent's participants row; this endpoint did not, so
// files written through the agent's own filesystem landed with a NULL
// tenant and outlived the workspace they were written in. The scalar
// subquery keeps that a single round trip, and still inserts when the
// participant lookup finds nothing, exactly as before. The ON CONFLICT arm
// heals rows already written without it.
await pool.query(
`INSERT INTO agent_workspace (agent_id, path, body, meta, updated_at)
VALUES ($1, $2, $3, $4::jsonb, NOW())
`INSERT INTO agent_workspace (agent_id, path, body, meta, company_id, updated_at)
VALUES ($1, $2, $3, $4::jsonb,
(SELECT company_id FROM participants WHERE id = $1), NOW())
ON CONFLICT (agent_id, path) DO UPDATE
SET body = EXCLUDED.body,
meta = COALESCE(agent_workspace.meta, EXCLUDED.meta),
company_id = COALESCE(EXCLUDED.company_id, agent_workspace.company_id),
updated_at = NOW()`,
[c.sub, p, text, JSON.stringify(meta)],
)
Expand Down
23 changes: 23 additions & 0 deletions server/src/api/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1823,9 +1823,32 @@ api.delete('/companies/:id', safe(async (req, res) => {
'agent_workspace', 'agent_memory', 'agent_log', 'agent_tasks',
'agent_climate', 'computers',
] as const
// The subset of the above that is keyed by the agent itself. Every one of
// these has an agent_id column; `computers` is deliberately absent (it is
// keyed by the machine, not an agent).
const agentOwnedTables = [
'agent_events', 'agent_runs', 'agent_triages',
'agent_workspace', 'agent_memory', 'agent_log', 'agent_tasks',
'agent_climate',
] as const
for (const table of softScopedTables) {
await client.query(`DELETE FROM ${table} WHERE company_id = $1`, [companyId])
}
// Sweep by owner as well, because company_id on these tables is a soft
// reference that some writers have historically left unset or wrong:
// agent_climate's column carries DEFAULT 'personal' and neither of its two
// INSERT sites names it, so every climate row in every workspace is
// labelled 'personal'; agent_workspace rows written through the agent's own
// filesystem endpoint carried a NULL tenant. Those rows are still owned by
// an agent that is about to cease existing, and a workspace deletion that
// leaves an agent's memory and its notes about people behind is not a
// deletion. Deleting by owner needs no backfill migration to reach the rows
// already written that way.
if (agentIds.length > 0) {
for (const table of agentOwnedTables) {
await client.query(`DELETE FROM ${table} WHERE agent_id = ANY($1::text[])`, [agentIds])
}
}
if (agentIds.length > 0) {
await client.query(`DELETE FROM board_mention_reads WHERE user_id = ANY($1::text[])`, [agentIds])
}
Expand Down
Loading