Public: anyone can read this; the crew can change it.

A person's conversation with the forge is stored turn by turn and read back after a restart #7666

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

A person's conversation with the forge is now stored turn by turn in the database and reads back after a restart. Nothing calls it yet. It isn't reachable from the feed or the MCP, so under the vision's "done means reachable" bar the issue is only partly done. I wired that into nothing here. The type check and `test/conversations.test.ts` pass, as does `test/schema-migrations.test.ts`. I did not r…

shipped production is serving it deploy 38399ee

opened this

A person's conversation with the forge is now stored turn by turn in the database and reads back after a restart. Nothing calls it yet. It isn't reachable from the feed or the MCP, so under the vision's "done means reachable" bar the issue is only partly done. I wired that into nothing here. The type check and test/conversations.test.ts pass, as does test/schema-migrations.test.ts. I did not run the full suite; the forge runs it on the push.

What changed

  • Migration: 20261101090000-conversation.ts adds conversation and conversation_turn, plus its pin file.

- conversation holds id, user_id (unique, so one per person), claude_session_id, created_at and updated_at. - conversation_turn holds id, conversation_id, role, text and created_at. Role is limited to person, agent, tool or confirm.

  • Core module: src/core/conversations.ts has four functions, all plain data in and out.

- openConversation returns the person's conversation, creating it on first use. - appendTurn adds a turn at the end. - listTurns returns a page with the newest last. before is a turn id: pass the first turn's id to page back, with a default of 50 and a maximum of 200 per page. - setClaudeSession stores the Claude session id. I added it because the issue asks for the column but no function would otherwise write it.

  • Ownership: every call takes the person's id. Another person's conversation id comes back as not_found.
  • Turn text cap: a turn is 1 to 20,000 characters (MAX_TURN_CHARS). Anything longer is refused as text_too_long, not cut. Empty text and an unknown role are refused as well.
  • Tests: test/conversations.test.ts covers one conversation per person, writing turns then reopening the database and reading them back in order, paging, other-person not_found, and the text and role checks.

Not done: no HTTP route, MCP tool, or feed entry.

To try it: run bun test test/conversations.test.ts.

🤖 Generated with Claude Code

Closes #7647

commented

A person's conversation with the forge is now stored turn by turn in the database and reads back after a restart. Nothing calls it yet. It isn't reachable from the feed or the MCP, so under the vision's "done means reachable" bar the issue is only partly done. I wired that into nothing here. The type check and test/conversations.test.ts pass, as does test/schema-migrations.test.ts. I did not run the full suite; the forge runs it on the push.

What changed

  • Migration: 20261101090000-conversation.ts adds conversation and conversation_turn, plus its pin file.

- conversation holds id, user_id (unique, so one per person), claude_session_id, created_at and updated_at. - conversation_turn holds id, conversation_id, role, text and created_at. Role is limited to person, agent, tool or confirm.

  • Core module: src/core/conversations.ts has four functions, all plain data in and out.

- openConversation returns the person's conversation, creating it on first use. - appendTurn adds a turn at the end. - listTurns returns a page with the newest last. before is a turn id: pass the first turn's id to page back, with a default of 50 and a maximum of 200 per page. - setClaudeSession stores the Claude session id. I added it because the issue asks for the column but no function would otherwise write it.

  • Ownership: every call takes the person's id. Another person's conversation id comes back as not_found.
  • Turn text cap: a turn is 1 to 20,000 characters (MAX_TURN_CHARS). Anything longer is refused as text_too_long, not cut. Empty text and an unknown role are refused as well.
  • Tests: test/conversations.test.ts covers one conversation per person, writing turns then reopening the database and reading them back in order, paging, other-person not_found, and the text and role checks.

Not done: no HTTP route, MCP tool, or feed entry.

To try it: run bun test test/conversations.test.ts.

🤖 Generated with Claude Code

Changes requested — before the latest push

Storage is clean and well tested, but nothing reaches it: no route, no MCP tool, no tty command, no feed entry, so a person cannot start, read or resume a conversation (the preview shows nothing new). The vision says a function nobody can call is not done, and the pull says Closes #7647. Fix first: wire the minimum door in this pull, or split the issue and drop Closes so #7647 stays open. If wired, both doors need it (page and src/tty.ts, plus an MCP tool), each checking identity from the verified headers, capping the body by bytes read, and with tests and a README line. Also: (1) setClaudeSession takes an unbounded sessionId; validate length and shape. (2) paging by id < before relies on id() sorting in creation order; make it deterministic (rowid or created_at,id) or test two turns in the same millisecond. (3) the test helper builds SQL by string interpolation; use bound parameters.

commented

This pull is the storage layer for #7647 and does not close it. I split the door off rather than wire it here. The type check and test/conversations.test.ts pass (8 tests), and I pushed the fixes below. I did not run the full suite; the forge runs it on the push.

Why the door is split off. A page, a src/tty.ts command and an MCP tool each need identity from the verified headers, a body capped by bytes read, tests and a README line. That is a separate piece of work from the storage. I opened #7671 for it and made it wait on #7647. Please don't let the forge add "Closes #7647" to this description.

Your three points

  1. setClaudeSession now accepts only /^[A-Za-z0-9._-]{1,128}$/ and answers bad_session otherwise. A test covers too long, wrong characters and empty.
  2. Paging now orders and pages by insertion order (rowid) instead of by id, so turns written in the same millisecond keep their order.

- before is looked up within the same conversation, so a turn id from another conversation pages nothing. - Two new tests: 120 turns written in one burst page back in 50s with no gaps or repeats, and the cross-conversation before case.

  1. The test helper now inserts users with bound parameters.

What to look at: src/core/conversations.ts and test/conversations.test.ts. The migration is unchanged.

🤖 Generated with Claude Code

Approved — before the latest push

All three earlier asks are met. (1) setClaudeSession validates against /^[A-Za-z0-9._-]{1,128}$/ and answers bad_session, with a test for too long, bad characters and empty. (2) Paging orders by rowid and looks up before inside the same conversation. Tests cover a 120-turn burst and a cross-conversation before. (3) The test helper binds its parameters. Ownership checks, input bounds and bound SQL parameters are all sound, and nothing in the change reaches the network. This is storage only, so there is no page, tty or MCP door yet. The pull says so and tracks the door in #7671, so I accept the split. Next two things: 1) land #7671 so a person can reach this. 2) Non-blocking: the index is (conversation_id, id) but the query orders by rowid, so index (conversation_id) alone or use rowid ordering consistently. Also reflow the over-long doc comment on listTurns.

commented

The full suite on this head squashed onto main (accaa65) was red, so the pull has not joined the merge queue.

Red tests:

  • a real roll of two forge processes on one data directory > an old forge that lingers past a probe period refuses nobody
  • a push in flight on the old forge completes across the roll, and no connection is refused
  • model budget: zero, an own subscription, a capped grant > a capped grant runs, and stops being offered at the cap

The gate's log: /api/v1/repos/trevato/forge/gates/1793560704935-700dad9fa5c8/log

It is proved again on a push.

commented

I reflowed the over-long listTurns doc comment and pushed. I left the index as it is, and I did not touch the three red tests from the full-suite run. This pull is still storage only; the door is #7671.

  • Index: I did not change (conversation_id, id). The migration is already pinned, and a pinned migration's text is history that test/schema-migrations.test.ts guards. Editing it, or adding a second migration to drop and recreate the index, costs more than it saves. The index still finds a conversation's rows; only the sort is separate, and per-person conversations are small.
  • Red suite: the failures are the two-process roll tests and the capped-grant model budget test. This pull touches neither. It adds two tables and one core module that nothing calls, so I can't see how it reaches those tests. The gate re-runs on this push.
  • Checks: the type check and test/conversations.test.ts (8 tests) pass.

Not done in this pull: no page, tty command or MCP tool reaches the conversation yet; that is #7671.

🤖 Generated with Claude Code

Approved

Earlier asks: (1) setClaudeSession validates against /^[A-Za-z0-9._-]{1,128}$/ with a test: met. (2) Paging by rowid, before scoped to the same conversation, burst and cross-conversation tests: met. (3) Test helper uses bound parameters: met. Storage only, every call is owner-scoped, SQL is parameterised, text is capped and role is checked in code and by CHECK. The page, tty and MCP doors are deliberately split to #7671, so there is no UI to look at in the preview. Next: (a) land #7671 so this is reachable; (b) Non-blocking: the index conversation_turn(conversation_id, id) does not match the rowid ordering, so a later migration could index what the query uses. The three red tests in the earlier gate (roll and model budget) are not reached by this change.

commented

The full suite on this head squashed onto main (accaa65) was red, so the pull has not joined the merge queue.

Red tests:

  • a real roll of two forge processes on one data directory > an old forge that lingers past a probe period refuses nobody
  • a push in flight on the old forge completes across the roll, and no connection is refused
  • the crew, with a stand-in Claude Code: the early pull > the pull opens at the first push, in progress, and the run's end fills that same pull in
  • model budget: zero, an own subscription, a capped grant > a new account has zero, says so, and its run is refused
  • model budget: zero, an own subscription, a capped grant > an own subscription runs, for its own repositories only
  • model budget: zero, an own subscription, a capped grant > a capped grant runs, and stops being offered at the cap

The gate's log: /api/v1/repos/trevato/forge/gates/1793560843642-4faf3aaab886/log

It is proved again on a push.

commented

I made no change this round and pushed nothing. The branch is still at 51966fe9.

The red tests: none of the six failures goes through anything this pull adds. This pull adds a core module that nothing imports and two tables. The failing tests are the two-process roll, the stand-in Claude Code early pull, and the model-budget cases. They exercise the forge's roll, the crew runner and the budget code. I can't run those here: the roll tests need unix sockets, which this sandbox skips. So I haven't reproduced the failures. I'm not going to edit tests or code I haven't touched to make the gate green. The merge queue's re-run on this head will show whether they were environmental. If the same ones fail again, the gate log at /api/v1/repos/trevato/forge/gates/1793560843642-4faf3aaab886/log should say why.

The index: I left conversation_turn(conversation_id, id) alone. The migration is pinned, so changing it would break the pin check. Indexing what the query orders by can go in a later migration, with the door in #7671.

The pull is storage only. A person reaches nothing new until #7671 lands.

🤖 Generated with Claude Code

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs