Skip to content
Merged
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
152 changes: 150 additions & 2 deletions packages/plugin/src/hooks/magic-context/read-session-db.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,15 @@ describe("isMidTurnFromOpenCodeDb", () => {
expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(false);
});

it("does not release mid-turn for synthetic user messages after a stale tool-calls tail", () => {
it("does not release mid-turn for synthetic-part user messages after a stale tool-calls tail", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "agent nudge", synthetic: true }, 200);
insertUser(db, "session-1", "user-1", { content: "agent nudge" }, 200);
insertPart(db, "session-1", "user-1", "part-1", {
type: "text",
text: "agent nudge",
synthetic: true,
});

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(true);
});
Expand Down Expand Up @@ -143,11 +148,154 @@ describe("isMidTurnFromOpenCodeDb", () => {
expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(false);
});

it("does not release mid-turn for marker-part user messages after a stale tool-calls tail", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "✉ Inbox from peer" }, 200);
insertPart(db, "session-1", "user-1", "part-1", {
type: "text",
text: "✉ Inbox from peer",
metadata: {
marker: {
kind: "inbox",
from: "Peer Session",
sessionId: "ses_peer0000000000000000000",
},
},
});

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(true);
});

it("releases mid-turn for an @mention operator prompt with a synthetic agent part", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "do the thing @research-deep" }, 200);
insertPart(db, "session-1", "user-1", "part-1", {
type: "text",
text: "do the thing @research-deep",
});
insertPart(db, "session-1", "user-1", "part-2", {
type: "agent",
name: "research-deep",
synthetic: true,
});

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(false);
});

it("releases mid-turn for a partless user message (vacuous-ALL fence)", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "new turn" }, 200);
// No parts inserted — partless messages must count as real.

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(false);
});

it("releases mid-turn when a user message has a marker part AND a real text part", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "real input with marker" }, 200);
insertPart(db, "session-1", "user-1", "part-1", {
type: "text",
text: "✉ Inbox from peer",
metadata: { marker: { kind: "inbox" } },
});
insertPart(db, "session-1", "user-1", "part-2", {
type: "text",
text: "real input with marker",
});

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(false);
});

it("is not mid-turn when there is no assistant message", () => {
const db = createMidTurnDb();

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(false);
});

it("does not release mid-turn for an ignored-only user part after a stale tool-calls tail", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "status notification" }, 200);
insertPart(db, "session-1", "user-1", "part-1", {
type: "text",
text: "## Claude Routing Status",
ignored: true,
});

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(true);
});

it("releases mid-turn when a user message has an ignored part AND a real text part", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "notification + real input" }, 200);
insertPart(db, "session-1", "user-1", "part-1", {
type: "text",
text: "## Claude Quotas",
ignored: true,
});
insertPart(db, "session-1", "user-1", "part-2", {
type: "text",
text: "actually do the thing",
});

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(false);
});

it("does not release mid-turn when ignored is numeric 1 (truthy variant)", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "status notification" }, 200);
insertPart(db, "session-1", "user-1", "part-1", {
type: "text",
text: "## Claude Quotas",
ignored: 1,
});

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(true);
});

it("does not release mid-turn for interrupt marker parts after a stale tool-calls tail", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "interrupt" }, 200);
insertPart(db, "session-1", "user-1", "part-1", {
type: "text",
text: "interrupt",
metadata: {
marker: {
kind: "interrupt",
intent: "abort",
origin: "parent",
},
},
});

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(true);
});

it("does not release mid-turn for message marker parts after a stale tool-calls tail", () => {
const db = createMidTurnDb();
insertAssistant(db, "session-1", "assistant-1", { finish: "tool-calls" }, 100);
insertUser(db, "session-1", "user-1", { content: "peer message" }, 200);
insertPart(db, "session-1", "user-1", "part-1", {
type: "text",
text: "peer message",
metadata: {
marker: {
kind: "message",
peer: "subagent",
expectReply: false,
},
},
});

expect(isMidTurnFromOpenCodeDb(db, "session-1")).toBe(true);
});
});

function useTempDataHome(prefix: string): void {
Expand Down
40 changes: 31 additions & 9 deletions packages/plugin/src/hooks/magic-context/read-session-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,18 +139,40 @@ function hasNewerRealUserMessage(
const row = db
.prepare(
`SELECT 1 as one
FROM message
WHERE session_id = ?
AND time_created > ?
AND json_extract(data, '$.role') = 'user'
AND COALESCE(json_extract(data, '$.synthetic'), 0) NOT IN (1, 'true')
FROM message m
WHERE m.session_id = ?
AND m.time_created > ?
AND json_extract(m.data, '$.role') = 'user'
AND NOT (
EXISTS (SELECT 1 FROM part p WHERE p.message_id = m.id)
AND NOT EXISTS (
SELECT 1 FROM part p
WHERE p.message_id = m.id
AND COALESCE(json_extract(p.data, '$.synthetic'), 0) NOT IN (1, 'true')
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
AND json_extract(p.data, '$.metadata.marker.kind') IS NULL

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The whole fix now depends on two external-schema assumptions: (1) that synthetic is persisted on the part row rather than the message row, and (2) that system-event (inbox/s2s) parts expose their kind at data.metadata.marker.kind. If the real OpenCode part shape differs from metadata.marker.kind — e.g. the marker data is stored directly on the part instead of nested under metadata — then marker parts would not be detected as machine-generated, and a user message whose only part is a marker would be misclassified as real, releasing the mid-turn lock (exactly the inverse bug the PR set out to fix). The new tests currently construct fixtures inside insertPart(...) using the same metadata: { marker: { kind: ... } } assumption the code reads, so they are self-confirming and cannot catch a mismatch with the real persisted shape. The PR itself already flags that marker.kind is 'not currently documented'. Please verify the marker path against a real captured OpenCode part row and consider adding a fixture sourced from actual persisted data rather than the assumed shape.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/src/hooks/magic-context/read-session-db.ts, line 152:

<comment>The whole fix now depends on two external-schema assumptions: (1) that `synthetic` is persisted on the `part` row rather than the `message` row, and (2) that system-event (inbox/s2s) parts expose their kind at `data.metadata.marker.kind`. If the real OpenCode part shape differs from `metadata.marker.kind` — e.g. the marker data is stored directly on the part instead of nested under `metadata` — then marker parts would not be detected as machine-generated, and a user message whose only part is a marker would be misclassified as real, releasing the mid-turn lock (exactly the inverse bug the PR set out to fix). The new tests currently construct fixtures inside `insertPart(...)` using the same `metadata: { marker: { kind: ... } }` assumption the code reads, so they are self-confirming and cannot catch a mismatch with the real persisted shape. The PR itself already flags that `marker.kind` is 'not currently documented'. Please verify the marker path against a real captured OpenCode part row and consider adding a fixture sourced from actual persisted data rather than the assumed shape.</comment>

<file context>
@@ -139,18 +139,35 @@ function hasNewerRealUserMessage(
+                   SELECT 1 FROM part p
+                   WHERE p.message_id = m.id
+                     AND COALESCE(json_extract(p.data, '$.synthetic'), 0) NOT IN (1, 'true')
+                     AND json_extract(p.data, '$.metadata.marker.kind') IS NULL
+                 )
+               )
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The self-confirming-fixture criticism is correct on method, and I've addressed it by sourcing the fixtures from real data. Both assumptions verified against a live opencode.db rather than reasoned about.

Assumption 1 — synthetic on part, not message. This is what the PR is fixing, and it's measurable: json_extract(data,'$.synthetic') on the message table returns 0 rows across the entire DB, versus 11,320 rows on part. The old message-level predicate could never match, which is why the valve was inert.

Assumption 2 — metadata.marker.kind. Present and populated: 740 inbox, 168 interrupt, 805 message. Verbatim part.data.metadata from real rows:

{"marker":{"kind":"inbox","from":"","sessionId":""}}
{"marker":{"kind":"interrupt","intent":"abort","origin":"parent"}}
{"marker":{"kind":"message","peer":"subagent","expectReply":false}}

The fixtures now use these shapes (session id and peer name replaced with placeholders) instead of the invented { marker: { kind } }. That closes the gap you identified two ways: the path is confirmed against persisted data, and because the real shapes carry sibling fields beside kind, the tests now also prove the predicate isn't accidentally depending on marker having exactly one key. Coverage widened from inbox alone to all three kinds.

On the inverse-bug risk specifically: I'd rather not rest on fixtures alone, so it's worth noting the ALL-parts semantics gives a second line of defense. For a marker path mismatch to release the lock, the message would need every part to be undetectable — and I checked what happens under ANY-semantics instead: 534 messages would flip, 525 of them genuine human prompts containing @mentions (an @mention appends a synthetic agent part to a real prompt). That asymmetry is why ALL is the correct quantifier here, and it's now covered by an explicit @mention test.

Worth flagging one thing I could not verify: marker.kind is real in persisted data but is not in a published schema I can find, so it remains an undocumented shape we depend on. If it moves, the failure is silent. I'd take a stable discriminator upstream if one is offered.

AND COALESCE(json_extract(p.data, '$.ignored'), 0) NOT IN (1, 'true')
)
)
LIMIT 1`,
)
.get(sessionId, latestAssistantTimeCreated) as ExistenceRow | null;
// OpenCode persists promptAsync/channel-2 synthetic prompts as
// message.info.synthetic, which is the top-level $.synthetic field in the
// message table's data JSON. Those agent-directed nudges should not end a
// still-accumulating tool-use turn, but a later real user message does.
// OpenCode persists synthetic as an annotation on the PART row's data, never
// on the message row. So separating injected from real user messages requires
// a part join. A user message is injected iff it HAS at least one part AND
// EVERY part is machine-generated — where a part is machine-generated if it
// carries either synthetic=true, a marker part (metadata.marker.kind), or
// an ignored flag. Marker parts are deliberately NON-synthetic so the TUI
// renders them as visible system-event lines; they are identified
// structurally. Ignored parts are dropped by opencode's own model-facing
// serializer (message-v2.ts:206): an ignored text part is never pushed into
// the model-facing message, so a message whose parts are all ignored cannot
// constitute a real user turn. ALL-parts semantics is load-bearing: a real
// operator prompt may include a synthetic `agent` part from an @mention —
// classifying that as injected would release the mid-turn lock on genuine
// human input (the inverse bug, and worse). The EXISTS guard on part rows is
// the vacuous-ALL fence: a partless message satisfies "every part is
// machine-generated" trivially, so it must count as real to avoid incorrectly
// suppressing a lock release.
return row?.one === 1;
}

Expand Down
Loading