Fix: maxSteps off-by-one error

.changeset/fix-max-steps-off-by-one.md

@@ -0,0 +1,5 @@
+---
+"@perstack/runtime": patch
+---
+
+Fix maxSteps off-by-one error that caused one extra step to execute

CODE_REVIEW_REPORT.md

@@ -18,7 +18,7 @@ Overall, the codebase is well-structured with strong architectural decisions. Th
 | ✅ Verified   | 5     | Confirmed not an issue / working correctly |
 | 📝 Documented | 1     | Behavior documented, no code change needed |
 | ⏸️ Deferred   | 8     | Low priority / E2E scope / future work     |
-| 🔴 Open       | 7     | Runtime package issues (2025-12-03)        |
+| 🔴 Open       | 6     | Runtime package issues (2025-12-03)        |
 | 🟡 Low Prio   | 4     | Runtime minor issues (2025-12-03)          |
 
 ---
@@ -568,7 +568,7 @@ Benefits:
 
 ### 32. maxSteps Off-By-One Error (Potential Bug)
 
-**Status**: 🔴 **Open**
+**Status**: ✅ **Fixed** — Commit `ced1aa6`
 
 **Category**: Potential Bug  
 **Severity**: Major
@@ -577,22 +577,7 @@ Benefits:
 
 **Issue**: The condition `checkpoint.stepNumber > setting.maxSteps` allows one extra step to execute.
 
-```typescript
-if (setting.maxSteps !== undefined && checkpoint.stepNumber > setting.maxSteps) {
-  return stopRunByExceededMaxSteps(...)
-}
-```
-
-**Expected behavior**: With `maxSteps=3`, steps 1, 2, 3 should execute, then stop.
-
-**Actual behavior**: With `maxSteps=3`, steps 1, 2, 3, 4 execute. The stop happens when `stepNumber=4 > 3`, meaning step 4 has already completed.
-
-**Impact**: Users setting `maxSteps` will get one more step than expected.
-
-**Recommendation**: Change to `>=` or check before incrementing stepNumber:
-```typescript
-if (setting.maxSteps !== undefined && checkpoint.stepNumber >= setting.maxSteps) {
-```
+**Resolution**: Changed `>` to `>=` in the comparison. Now `maxSteps=3` correctly stops after step 3.
 
 ---
 
@@ -940,7 +925,7 @@ const metaInstruction = dedent`
 | #29   | Provider settings discriminated union  | ✅ Fixed        |
 | #30   | Separate types and schemas in core     | ⏸️ Future       |
 | #31   | Skill lazy init for faster startup     | ⏸️ Future       |
-| #32   | maxSteps off-by-one error              | 🔴 Open         |
+| #32   | maxSteps off-by-one error              | ✅ Fixed        |
 | #33   | Sandbox design flaw (workspace/chdir)  | ✅ Fixed        |
 | #34   | Delegate expert not found error        | 🔴 Open         |
 | #35   | Skill object mutation                  | 🔴 Open         |
@@ -970,9 +955,6 @@ const metaInstruction = dedent`
 
 ### Runtime Package Open Issues (2025-12-03)
 
-**Major (Should Fix)**:
-- **#32**: maxSteps off-by-one — users get one extra step than configured
-
 **Minor (Should Fix)**:
 - **#34**: Delegate expert not found — improve error message
 - **#35, #36**: Object mutation side effects — defensive copying recommended
@@ -1002,3 +984,4 @@ const metaInstruction = dedent`
 | `ebf67bc` | Docs: Add JSDoc to all core schema types                           |
 | `f7edeb9` | Refactor: Use discriminatedUnion for provider settings             |
 | `5490ebb` | Refactor: Split SkillManager into separate classes                 |
+| `ced1aa6` | Fix: maxSteps off-by-one error in finishing step                   |

packages/runtime/src/states/finishing-step.test.ts

@@ -37,10 +37,40 @@ describe("@perstack/runtime: StateMachineLogic['FinishingStep']", () => {
     })
   })
 
-  it("stops when max steps exceeded", async () => {
+  it("stops when step number reaches max steps", async () => {
     const setting = createRunSetting({ maxSteps: 3 })
-    const checkpoint = createCheckpoint({ stepNumber: 4 })
-    const step = createStep({ stepNumber: 4 })
+    const checkpoint = createCheckpoint({ stepNumber: 3 })
+    const step = createStep({ stepNumber: 3 })
+    await expect(
+      StateMachineLogics.FinishingStep({
+        setting,
+        checkpoint,
+        step,
+        eventListener: async () => {},
+        skillManagers: {},
+      }),
+    ).resolves.toStrictEqual({
+      type: "stopRunByExceededMaxSteps",
+      id: expect.any(String),
+      expertKey: setting.expertKey,
+      timestamp: expect.any(Number),
+      runId: setting.runId,
+      stepNumber: checkpoint.stepNumber,
+      checkpoint: {
+        ...checkpoint,
+        status: "stoppedByExceededMaxSteps",
+      },
+      step: {
+        ...step,
+        finishedAt: expect.any(Number),
+      },
+    })
+  })
+
+  it("stops when step number exceeds max steps", async () => {
+    const setting = createRunSetting({ maxSteps: 3 })
+    const checkpoint = createCheckpoint({ stepNumber: 5 })
+    const step = createStep({ stepNumber: 5 })
     await expect(
       StateMachineLogics.FinishingStep({
         setting,

packages/runtime/src/states/finishing-step.ts

@@ -1,21 +1,21 @@
 import { createId } from "@paralleldrive/cuid2"
-import { type RunEvent, continueToNextStep, stopRunByExceededMaxSteps } from "@perstack/core"
+import { continueToNextStep, type RunEvent, stopRunByExceededMaxSteps } from "@perstack/core"
 import type { RunSnapshot } from "../runtime-state-machine.js"
 
 export async function finishingStepLogic({
   setting,
   checkpoint,
   step,
 }: RunSnapshot["context"]): Promise<RunEvent> {
-  if (setting.maxSteps !== undefined && checkpoint.stepNumber > setting.maxSteps) {
+  if (setting.maxSteps !== undefined && checkpoint.stepNumber >= setting.maxSteps) {
     return stopRunByExceededMaxSteps(setting, checkpoint, {
       checkpoint: {
         ...checkpoint,
         status: "stoppedByExceededMaxSteps",
       },
       step: {
         ...step,
-        finishedAt: new Date().getTime(),
+        finishedAt: Date.now(),
       },
     })
   }
@@ -25,7 +25,7 @@ export async function finishingStepLogic({
     },
     step: {
       ...step,
-      finishedAt: new Date().getTime(),
+      finishedAt: Date.now(),
     },
     nextCheckpoint: {
       ...checkpoint,