Merge pull request #1683 from code-yeongyu/fix/issue-1672
fix: guard session_ids with optional chaining to prevent crash (#1672)
This commit is contained in:
@@ -43,6 +43,78 @@ describe("boulder-state", () => {
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
test("should return null for JSON null value", () => {
|
||||
//#given - boulder.json containing null
|
||||
const boulderFile = join(SISYPHUS_DIR, "boulder.json")
|
||||
writeFileSync(boulderFile, "null")
|
||||
|
||||
//#when
|
||||
const result = readBoulderState(TEST_DIR)
|
||||
|
||||
//#then
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
test("should return null for JSON primitive value", () => {
|
||||
//#given - boulder.json containing a string
|
||||
const boulderFile = join(SISYPHUS_DIR, "boulder.json")
|
||||
writeFileSync(boulderFile, '"just a string"')
|
||||
|
||||
//#when
|
||||
const result = readBoulderState(TEST_DIR)
|
||||
|
||||
//#then
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
test("should default session_ids to [] when missing from JSON", () => {
|
||||
//#given - boulder.json without session_ids field
|
||||
const boulderFile = join(SISYPHUS_DIR, "boulder.json")
|
||||
writeFileSync(boulderFile, JSON.stringify({
|
||||
active_plan: "/path/to/plan.md",
|
||||
started_at: "2026-01-01T00:00:00Z",
|
||||
plan_name: "plan",
|
||||
}))
|
||||
|
||||
//#when
|
||||
const result = readBoulderState(TEST_DIR)
|
||||
|
||||
//#then
|
||||
expect(result).not.toBeNull()
|
||||
expect(result!.session_ids).toEqual([])
|
||||
})
|
||||
|
||||
test("should default session_ids to [] when not an array", () => {
|
||||
//#given - boulder.json with session_ids as a string
|
||||
const boulderFile = join(SISYPHUS_DIR, "boulder.json")
|
||||
writeFileSync(boulderFile, JSON.stringify({
|
||||
active_plan: "/path/to/plan.md",
|
||||
started_at: "2026-01-01T00:00:00Z",
|
||||
session_ids: "not-an-array",
|
||||
plan_name: "plan",
|
||||
}))
|
||||
|
||||
//#when
|
||||
const result = readBoulderState(TEST_DIR)
|
||||
|
||||
//#then
|
||||
expect(result).not.toBeNull()
|
||||
expect(result!.session_ids).toEqual([])
|
||||
})
|
||||
|
||||
test("should default session_ids to [] for empty object", () => {
|
||||
//#given - boulder.json with empty object
|
||||
const boulderFile = join(SISYPHUS_DIR, "boulder.json")
|
||||
writeFileSync(boulderFile, JSON.stringify({}))
|
||||
|
||||
//#when
|
||||
const result = readBoulderState(TEST_DIR)
|
||||
|
||||
//#then
|
||||
expect(result).not.toBeNull()
|
||||
expect(result!.session_ids).toEqual([])
|
||||
})
|
||||
|
||||
test("should read valid boulder state", () => {
|
||||
// given - valid boulder.json
|
||||
const state: BoulderState = {
|
||||
@@ -129,6 +201,23 @@ describe("boulder-state", () => {
|
||||
// then
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
test("should not crash when boulder.json has no session_ids field", () => {
|
||||
//#given - boulder.json without session_ids
|
||||
const boulderFile = join(SISYPHUS_DIR, "boulder.json")
|
||||
writeFileSync(boulderFile, JSON.stringify({
|
||||
active_plan: "/plan.md",
|
||||
started_at: "2026-01-01T00:00:00Z",
|
||||
plan_name: "plan",
|
||||
}))
|
||||
|
||||
//#when
|
||||
const result = appendSessionId(TEST_DIR, "ses-new")
|
||||
|
||||
//#then - should not crash and should contain the new session
|
||||
expect(result).not.toBeNull()
|
||||
expect(result!.session_ids).toContain("ses-new")
|
||||
})
|
||||
})
|
||||
|
||||
describe("clearBoulderState", () => {
|
||||
|
||||
@@ -22,7 +22,14 @@ export function readBoulderState(directory: string): BoulderState | null {
|
||||
|
||||
try {
|
||||
const content = readFileSync(filePath, "utf-8")
|
||||
return JSON.parse(content) as BoulderState
|
||||
const parsed = JSON.parse(content)
|
||||
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
|
||||
return null
|
||||
}
|
||||
if (!Array.isArray(parsed.session_ids)) {
|
||||
parsed.session_ids = []
|
||||
}
|
||||
return parsed as BoulderState
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
@@ -48,7 +55,10 @@ export function appendSessionId(directory: string, sessionId: string): BoulderSt
|
||||
const state = readBoulderState(directory)
|
||||
if (!state) return null
|
||||
|
||||
if (!state.session_ids.includes(sessionId)) {
|
||||
if (!state.session_ids?.includes(sessionId)) {
|
||||
if (!Array.isArray(state.session_ids)) {
|
||||
state.session_ids = []
|
||||
}
|
||||
state.session_ids.push(sessionId)
|
||||
if (writeBoulderState(directory, state)) {
|
||||
return state
|
||||
|
||||
@@ -41,7 +41,7 @@ export function createAtlasEventHandler(input: {
|
||||
|
||||
// Read boulder state FIRST to check if this session is part of an active boulder
|
||||
const boulderState = readBoulderState(ctx.directory)
|
||||
const isBoulderSession = boulderState?.session_ids.includes(sessionID) ?? false
|
||||
const isBoulderSession = boulderState?.session_ids?.includes(sessionID) ?? false
|
||||
|
||||
const isBackgroundTaskSession = subagentSessions.has(sessionID)
|
||||
|
||||
|
||||
@@ -65,7 +65,7 @@ export function createToolExecuteAfterHandler(input: {
|
||||
if (boulderState) {
|
||||
const progress = getPlanProgress(boulderState.active_plan)
|
||||
|
||||
if (toolInput.sessionID && !boulderState.session_ids.includes(toolInput.sessionID)) {
|
||||
if (toolInput.sessionID && !boulderState.session_ids?.includes(toolInput.sessionID)) {
|
||||
appendSessionId(ctx.directory, toolInput.sessionID)
|
||||
log(`[${HOOK_NAME}] Appended session to boulder`, {
|
||||
sessionID: toolInput.sessionID,
|
||||
|
||||
@@ -43,7 +43,7 @@ export function getAgentFromSession(sessionID: string, directory: string): strin
|
||||
|
||||
// Check boulder state (persisted across restarts) - fixes #927
|
||||
const boulderState = readBoulderState(directory)
|
||||
if (boulderState?.session_ids.includes(sessionID) && boulderState.agent) {
|
||||
if (boulderState?.session_ids?.includes(sessionID) && boulderState.agent) {
|
||||
return boulderState.agent
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user