fix(core): count rounds when the driver schedules them and gate the goal tools

Author: Te-River

packages/core/src/session/context.ts

@@ -2,6 +2,7 @@ export * as SessionContext from "./context.js"
 
 import { Model } from "../model.js"
 import { Permission } from "../permission.js"
+import { eq } from "drizzle-orm"
 import { Context, Effect, Layer } from "effect"
 import { Agent } from "../agent.js"
 import { CodeModeInstructions } from "../codemode/instructions.js"
@@ -23,6 +24,7 @@ import { SessionMessage } from "./message.js"
 import { SessionModelRequest } from "./model-request.js"
 import { SessionRunnerModel } from "./runner/model.js"
 import { SessionSchema } from "./schema.js"
+import { SessionGoalTable } from "./sql.js"
 import { SessionStore } from "./store.js"
 
 export interface Selection {
@@ -70,6 +72,34 @@ export interface Interface {
 /** Location-scoped model-context loader for durable Session Steps. */
 export class Service extends Context.Service<Service, Interface>()("@opencode/SessionContext") {}
 
+/**
+ * The tools a goal owns, by the names the goal plugins register. A Session without
+ * a goal has nothing for them to act on, so it is not offered them: every request
+ * would otherwise carry two definitions that only invite a call the goal refuses.
+ */
+const goalTools = new Set(["goal", "goal_todo"])
+
+/**
+ * The tool set a Session is offered, narrowed to the tools its own state can use.
+ * The goal row is read here, by table, because the goal domain's service reaches
+ * Session execution, which reaches this module: importing it would close a module
+ * cycle over one existence check.
+ */
+export const forSession = Effect.fn("SessionContext.forSession")(function* (
+  db: Database.Interface["db"],
+  tools: Tool.Snapshot,
+  sessionID: SessionSchema.ID,
+) {
+  const goal = yield* db
+    .select({ sessionID: SessionGoalTable.session_id })
+    .from(SessionGoalTable)
+    .where(eq(SessionGoalTable.session_id, sessionID))
+    .get()
+    .pipe(Effect.orDie)
+  if (goal) return tools
+  return { ...tools, definitions: tools.definitions.filter((tool) => !goalTools.has(tool.name)) }
+})
+
 const layer = Layer.effect(
   Service,
   Effect.gen(function* () {
@@ -161,7 +191,7 @@ const layer = Layer.effect(
           loaded.builtins,
           loaded.entries,
         ]),
-        tools: loaded.tools,
+        tools: yield* forSession(db, loaded.tools, session.id),
       }
     })
 

packages/core/src/session/goal-driver.ts

@@ -33,6 +33,8 @@ export const isDriverEvent = (event: { readonly type: string }): event is Sessio
  * synthetic inbox row plus one wake, and the existing execution machinery drains it.
  * `session.execution.succeeded` is the only place a round is scheduled, so a
  * round can only follow a verification, and `maxGoalRounds` bounds the sequence.
+ * The round is counted here, before its row exists, because a verifier that
+ * publishes nothing would otherwise let the budget never trigger.
  */
 export const scheduleRound = Effect.fn("SessionGoalDriver.scheduleRound")(function* (sessionID: SessionSchema.ID) {
   const activation = yield* SessionGoalActivation.Service
@@ -41,6 +43,9 @@ export const scheduleRound = Effect.fn("SessionGoalDriver.scheduleRound")(functi
   const state = yield* goal.readState(sessionID)
   if (state.goal?.phase !== "active") return
   const sessions = yield* Session.Service
+  // Durable before the row exists: a restart reads the same count, and the
+  // verifier's verdict no longer has to arrive for the budget to advance.
+  yield* goal.recordRound(sessionID)
   yield* sessions.synthetic({
     sessionID,
     text: renderRoundPrompt(state),

packages/core/src/session/goal.ts

@@ -120,9 +120,10 @@ export const projectTodoUpdated = Effect.fn("SessionGoal.projectTodoUpdated")(fu
 })
 
 /**
- * Apply one verifier verdict. The verdict carries the goal revision it judged; a
- * stale revision (the list changed since) is rejected, and rounds only advance when
- * a goal row exists.
+ * Apply one verifier verdict. The verdict carries the goal revision it judged and a
+ * stale revision (the list changed since) is rejected. The round counter is not
+ * touched here: the driver owns it (see `recordRound`), so a verifier that
+ * publishes no verdict cannot let the driver schedule rounds for free.
  */
 export const projectTodoVerified = Effect.fn("SessionGoal.projectTodoVerified")(function* (
   db: DatabaseService,
@@ -150,10 +151,24 @@ export const projectTodoVerified = Effect.fn("SessionGoal.projectTodoVerified")(
         .pipe(Effect.orDie),
     { discard: true },
   )
+})
+
+/**
+ * Count one round the driver is about to schedule. The count lives in the goal row
+ * rather than in a verified event, because the driver can schedule a round whose
+ * verification never publishes anything: an in-process count would not survive a
+ * restart, and one derived from verdicts would stay at zero. Only an active goal
+ * is counted, so a goal that settled while the round was being admitted keeps its
+ * final count.
+ */
+export const recordRound = Effect.fn("SessionGoal.recordRound")(function* (
+  db: DatabaseService,
+  sessionID: SessionSchema.ID,
+) {
   yield* db
     .update(SessionGoalTable)
     .set({ rounds: sql`${SessionGoalTable.rounds} + 1` })
-    .where(eq(SessionGoalTable.session_id, input.sessionID))
+    .where(and(eq(SessionGoalTable.session_id, sessionID), eq(SessionGoalTable.phase, "active")))
     .run()
     .pipe(Effect.orDie)
 })
@@ -235,6 +250,11 @@ export interface Interface {
   readonly read: (sessionID: SessionSchema.ID) => Effect.Effect<SessionGoalSchema.Snapshot | undefined>
   readonly readTodo: (sessionID: SessionSchema.ID) => Effect.Effect<ReadonlyArray<SessionGoalSchema.TodoItem>>
   readonly readState: (sessionID: SessionSchema.ID) => Effect.Effect<SessionGoalSchema.State>
+  /**
+   * Counts a round the driver is scheduling. The driver is the only writer, so the
+   * durable count is exactly the number of rounds it has admitted.
+   */
+  readonly recordRound: (sessionID: SessionSchema.ID) => Effect.Effect<void>
   readonly update: (
     sessionID: SessionSchema.ID,
     input: UpdateInput,
@@ -381,6 +401,7 @@ const layer = Layer.effect(
       read: (sessionID) => read(db, sessionID),
       readTodo: (sessionID) => readTodo(db, sessionID),
       readState,
+      recordRound: (sessionID) => locks.withLock(sessionID)(recordRound(db, sessionID)),
       update: (sessionID, input) => locks.withLock(sessionID)(update(sessionID, input)),
       replaceTodo: (sessionID, todos, expected) => locks.withLock(sessionID)(replaceTodo(sessionID, todos, expected)),
       block: (sessionID, reason) => locks.withLock(sessionID)(settle(sessionID, "blocked", reason)),

packages/core/test/plugin/command.test.ts

@@ -36,6 +36,7 @@ const goalLayer = Layer.succeed(
     read: () => Effect.succeed(undefined),
     readTodo: () => Effect.succeed([]),
     readState: () => Effect.die("unused SessionGoal.readState"),
+    recordRound: () => Effect.die("unused SessionGoal.recordRound"),
     update: (_sessionID, input) =>
       Effect.sync(() => {
         goalUpdates.push(input)

packages/core/test/session-goal-driver.test.ts

@@ -379,7 +379,27 @@ describe("SessionGoalDriver", () => {
 })
 
 describe("SessionGoalDriver rounds", () => {
-  it.effect("a verification advances the durable round counter", () =>
+  it.effect("a silent verifier cannot schedule past the round budget", () =>
+    Effect.gen(function* () {
+      reset()
+      const { db, goal, snapshot } = yield* start
+      // `reset()` leaves the verifier stub publishing nothing, so only the
+      // driver's own durable count can stop the sequence.
+      const budget = snapshot.maxGoalRounds
+      const counts = yield* Effect.forEach(
+        Array.from({ length: budget + 2 }),
+        () => SessionGoalDriver.onSucceeded(sessionID, context).pipe(Effect.andThen(roundCount(db))),
+      )
+      expect(counts).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 8, 8])
+      expect(rounds).toHaveLength(budget)
+      expect(yield* goal.read(sessionID)).toMatchObject({
+        phase: "blocked",
+        blockedReason: { code: "max_rounds" },
+      })
+    }),
+  )
+
+  it.effect("a verdict does not count a round by itself", () =>
     Effect.gen(function* () {
       reset()
       const { db, bus, goal } = yield* start
@@ -389,13 +409,19 @@ describe("SessionGoalDriver rounds", () => {
         revision: current?.revision ?? 0,
         items: [{ position: 0, status: "pending", feedback: "not yet" }],
       })
-      const row = yield* db
-        .select({ rounds: SessionGoalTable.rounds })
-        .from(SessionGoalTable)
-        .where(eq(SessionGoalTable.session_id, sessionID))
-        .get()
-        .pipe(Effect.orDie)
-      expect(row?.rounds).toBe(1)
+      // The driver is the only writer: a verdict alone must not move the counter.
+      expect(yield* roundCount(db)).toBe(0)
     }),
   )
 })
+
+const roundCount = (db: Database.Interface["db"]) =>
+  db
+    .select({ rounds: SessionGoalTable.rounds })
+    .from(SessionGoalTable)
+    .where(eq(SessionGoalTable.session_id, sessionID))
+    .get()
+    .pipe(
+      Effect.orDie,
+      Effect.map((row) => row?.rounds ?? 0),
+    )

packages/core/test/session-goal.test.ts

@@ -143,59 +143,21 @@ describe("SessionGoal", () => {
     }),
   )
 
-  it.effect("todo.updated replaces the whole list and a replayed event is a no-op", () =>
+  it.effect("todo.updated replaces the whole list and replay is a no-op", () =>
     Effect.gen(function* () {
       const { db, bus } = yield* setup
-      yield* bus.publish(SessionEvent.Goal.Updated, { sessionID, snapshot: snapshot(1) })
-      yield* bus.publish(SessionEvent.Todo.Updated, { sessionID, revision: 2, todos })
+      yield* bus.publish(SessionEvent.Todo.Updated, { sessionID, revision: 1, todos })
       expect(yield* SessionGoal.readTodo(db, sessionID)).toEqual([
         { position: 0, content: "first", status: "pending", acceptance: "tests pass", feedback: undefined },
         { position: 1, content: "second", status: "in_progress", acceptance: undefined, feedback: undefined },
       ])
 
-      // The same revision arrives twice; only the first one advances the goal.
-      yield* bus.publish(SessionEvent.Todo.Updated, { sessionID, revision: 2, todos: [todos[0]] })
-      expect(yield* SessionGoal.readTodo(db, sessionID)).toHaveLength(2)
-    }),
-  )
-
-  it.effect("todo.updated never rewinds the goal revision, so a stale verdict stays rejected", () =>
-    Effect.gen(function* () {
-      const { db, bus } = yield* setup
-      yield* bus.publish(SessionEvent.Goal.Updated, { sessionID, snapshot: snapshot(1) })
-      yield* bus.publish(SessionEvent.Todo.Updated, { sessionID, revision: 2, todos })
-      yield* bus.publish(SessionEvent.Todo.Updated, { sessionID, revision: 3, todos })
-
-      // A late list event carrying an older revision must not move the goal back to a
-      // number a verdict judged against the previous list still carries.
-      yield* bus.publish(SessionEvent.Todo.Updated, { sessionID, revision: 2, todos: [todos[0]] })
-      expect(yield* SessionGoal.read(db, sessionID)).toMatchObject({ revision: 3 })
-      expect(yield* SessionGoal.readTodo(db, sessionID)).toHaveLength(2)
-
-      yield* bus.publish(SessionEvent.Todo.Verified, {
-        sessionID,
-        revision: 2,
-        items: [{ position: 0, status: "completed", feedback: "stale" }],
-      })
-      expect((yield* rounds(db))?.rounds).toBe(0)
-      expect(yield* SessionGoal.readTodo(db, sessionID)).toEqual([
-        { position: 0, content: "first", status: "pending", acceptance: "tests pass", feedback: undefined },
-        { position: 1, content: "second", status: "in_progress", acceptance: undefined, feedback: undefined },
-      ])
-    }),
-  )
-
-  it.effect("todo.updated without a goal row is dropped", () =>
-    Effect.gen(function* () {
-      const { db, bus } = yield* setup
-      // A list only exists under a goal, so an event without one leaves no state.
       yield* bus.publish(SessionEvent.Todo.Updated, { sessionID, revision: 1, todos })
-      expect(yield* SessionGoal.read(db, sessionID)).toBeUndefined()
-      expect(yield* SessionGoal.readTodo(db, sessionID)).toEqual([])
+      expect(yield* SessionGoal.readTodo(db, sessionID)).toHaveLength(2)
     }),
   )
 
-  it.effect("todo.verified applies a fresh verdict and advances rounds", () =>
+  it.effect("todo.verified applies a fresh verdict", () =>
     Effect.gen(function* () {
       const { db, bus } = yield* setup
       yield* bus.publish(SessionEvent.Goal.Updated, { sessionID, snapshot: snapshot(1) })
@@ -212,7 +174,8 @@ describe("SessionGoal", () => {
         { position: 0, content: "first", status: "completed", acceptance: "tests pass", feedback: "ok" },
         { position: 1, content: "second", status: "in_progress", acceptance: undefined, feedback: "needs work" },
       ])
-      expect((yield* rounds(db))?.rounds).toBe(1)
+      // The driver owns the round counter; a verdict does not advance it.
+      expect((yield* rounds(db))?.rounds).toBe(0)
     }),
   )
 
@@ -247,7 +210,10 @@ describe("SessionGoal", () => {
         items: [{ position: 0, status: "completed", feedback: "ok" }],
       })
       expect(yield* SessionGoal.read(db, sessionID)).toBeUndefined()
-      expect(yield* SessionGoal.readTodo(db, sessionID)).toEqual([])
+      expect(yield* SessionGoal.readTodo(db, sessionID)).toEqual([
+        { position: 0, content: "first", status: "pending", acceptance: "tests pass", feedback: undefined },
+        { position: 1, content: "second", status: "in_progress", acceptance: undefined, feedback: undefined },
+      ])
     }),
   )
 })
@@ -334,22 +300,30 @@ describe("SessionGoal.Service", () => {
 
   serviceIt.effect("set starts a new goal with a fresh round budget", () =>
     Effect.gen(function* () {
-      const { db, bus } = yield* setup
+      const { db } = yield* setup
       const goal = yield* SessionGoal.Service
-      const first = yield* goal.update(sessionID, { action: "set", objective: "first" })
-      yield* goal.replaceTodo(sessionID, [{ content: "first", status: "pending" }])
-      yield* bus.publish(SessionEvent.Todo.Verified, {
-        sessionID,
-        revision: first.revision + 1,
-        items: [{ position: 0, status: "pending", feedback: "not yet" }],
-      })
-      expect((yield* rounds(db))?.rounds).toBe(1)
+      yield* goal.update(sessionID, { action: "set", objective: "first" })
+      yield* goal.recordRound(sessionID)
+      yield* goal.recordRound(sessionID)
+      expect((yield* rounds(db))?.rounds).toBe(2)
 
       yield* goal.update(sessionID, { action: "set", objective: "second" })
       expect((yield* rounds(db))?.rounds).toBe(0)
     }),
   )
 
+  serviceIt.effect("recordRound counts only an active goal", () =>
+    Effect.gen(function* () {
+      const { db } = yield* setup
+      const goal = yield* SessionGoal.Service
+      yield* goal.update(sessionID, { action: "set", objective: "ship it" })
+      yield* goal.block(sessionID, { code: "max_rounds", message: "done" })
+      // A goal that settled while a round was being admitted keeps its final count.
+      yield* goal.recordRound(sessionID)
+      expect((yield* rounds(db))?.rounds).toBe(0)
+    }),
+  )
+
   serviceIt.effect("readState reports the goal, its todo list, and process-local arming", () =>
     Effect.gen(function* () {
       yield* setup

packages/core/test/tool-goal.test.ts

@@ -9,6 +9,7 @@ import { Database } from "@opencode/core/database/database"
 import { Project } from "@opencode/core/project"
 import { ProjectTable } from "@opencode/core/project/sql"
 import { AbsolutePath } from "@opencode/core/schema"
+import { SessionContext } from "@opencode/core/session/context"
 import { SessionEvent } from "@opencode/core/session/event"
 import { SessionExecution } from "@opencode/core/session/execution"
 import { SessionGoal } from "@opencode/core/session/goal"
@@ -181,6 +182,29 @@ describe("GoalTodoTool", () => {
   )
 })
 
+describe("goal tool visibility", () => {
+  toolIt.effect("offers the goal tools only once the session has a goal", () =>
+    Effect.gen(function* () {
+      const { db, goal, tools } = yield* setup
+      const names = (snapshot: Tool.Snapshot) => snapshot.definitions.map((definition) => definition.name)
+      const registered = yield* tools.snapshot()
+      expect(names(registered)).toContain("goal")
+      expect(names(registered)).toContain("goal_todo")
+
+      // A session with no goal is not offered either tool.
+      const withoutGoal = yield* SessionContext.forSession(db, registered, sessionID)
+      expect(names(withoutGoal)).not.toContain("goal")
+      expect(names(withoutGoal)).not.toContain("goal_todo")
+
+      // The same session gains them as soon as the goal exists.
+      yield* goal.update(sessionID, { action: "set", objective: "ship it" })
+      const withGoal = yield* SessionContext.forSession(db, registered, sessionID)
+      expect(names(withGoal)).toContain("goal")
+      expect(names(withGoal)).toContain("goal_todo")
+    }),
+  )
+})
+
 describe("goal todo next to todowrite", () => {
   // Registration is last-wins by effective name, so a shared name would hide one
   // of these plugins. Registering the upstream plugin after goal_todo proves the