Skip to content

Commit 7a2d090

Browse files
committed
refactor: remove session persistence from opencode calls
Remove session ID handling to prevent context rot in long-running autonomous loops. Each opencode CLI invocation now starts with a fresh context, relying on prompts and the plan file for continuity between tasks. Changes: - Remove session_id field from Executor struct - Remove resetSession() method and all calls to it - Remove continue_session parameter from runOpencode/runWithRetry - Remove --session argument construction - Remove test for session ID continuation - Simplify deinit() method (no cleanup needed) This ensures opencoder can run indefinitely without accumulating stale context that could degrade AI performance over many cycles. Signed-off-by: leocavalcante <[email protected]>
1 parent 02f0560 commit 7a2d090

File tree

2 files changed

+58
-93
lines changed

2 files changed

+58
-93
lines changed

src/executor.zig

Lines changed: 24 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,17 @@ pub const Executor = struct {
2121
cfg: *const config.Config,
2222
log: *Logger,
2323
allocator: Allocator,
24-
session_id: ?[]const u8,
2524
opencode_cmd: []const u8,
25+
current_child_pid: ?std.posix.pid_t,
2626

2727
/// Initialize executor
2828
pub fn init(cfg: *const config.Config, log: *Logger, allocator: Allocator) Executor {
2929
return Executor{
3030
.cfg = cfg,
3131
.log = log,
3232
.allocator = allocator,
33-
.session_id = null,
3433
.opencode_cmd = "opencode",
34+
.current_child_pid = null,
3535
};
3636
}
3737

@@ -41,16 +41,14 @@ pub const Executor = struct {
4141
.cfg = cfg,
4242
.log = log,
4343
.allocator = allocator,
44-
.session_id = null,
4544
.opencode_cmd = opencode_cmd,
45+
.current_child_pid = null,
4646
};
4747
}
4848

4949
/// Cleanup executor resources
5050
pub fn deinit(self: *Executor) void {
51-
if (self.session_id) |sid| {
52-
self.allocator.free(sid);
53-
}
51+
_ = self;
5452
}
5553

5654
/// Run planning phase
@@ -63,14 +61,7 @@ pub const Executor = struct {
6361
var title_buf: [64]u8 = undefined;
6462
const title = std.fmt.bufPrint(&title_buf, "Opencoder Planning Cycle {d}", .{cycle}) catch "Opencoder Planning";
6563

66-
const result = try self.runWithRetry(self.cfg.planning_model, title, prompt, false);
67-
68-
if (result == .success) {
69-
// Reset session for new cycle
70-
self.resetSession();
71-
}
72-
73-
return result;
64+
return try self.runWithRetry(self.cfg.planning_model, title, prompt);
7465
}
7566

7667
/// Run task execution
@@ -83,15 +74,7 @@ pub const Executor = struct {
8374
var title_buf: [64]u8 = undefined;
8475
const title = std.fmt.bufPrint(&title_buf, "Opencoder Execution Cycle {d}", .{cycle}) catch "Opencoder Execution";
8576

86-
const continue_session = self.session_id != null;
87-
const result = try self.runWithRetry(self.cfg.execution_model, title, prompt, continue_session);
88-
89-
if (result == .success and self.session_id == null) {
90-
// Set session ID after first successful task (allocate owned memory)
91-
self.session_id = try std.fmt.allocPrint(self.allocator, "ses_{d}", .{cycle});
92-
}
93-
94-
return result;
77+
return try self.runWithRetry(self.cfg.execution_model, title, prompt);
9578
}
9679

9780
/// Run evaluation phase
@@ -110,7 +93,7 @@ pub const Executor = struct {
11093
self.log.statusFmt("[Cycle {d}] Evaluation retry {d}/{d}", .{ cycle, attempt + 1, self.cfg.max_retries });
11194
}
11295

113-
const result = self.runOpencode(self.cfg.planning_model, title, prompt, false);
96+
const result = self.runOpencode(self.cfg.planning_model, title, prompt);
11497
if (result) |output| {
11598
// Check for COMPLETE or NEEDS_WORK in output
11699
if (std.mem.indexOf(u8, output, "COMPLETE") != null) {
@@ -150,24 +133,27 @@ pub const Executor = struct {
150133
return "NEEDS_WORK";
151134
}
152135

153-
/// Reset session for new cycle
154-
pub fn resetSession(self: *Executor) void {
155-
if (self.session_id) |sid| {
156-
self.allocator.free(sid);
136+
/// Kill current child process if running
137+
pub fn killCurrentChild(self: *Executor) void {
138+
if (self.current_child_pid) |pid| {
139+
self.log.logFmt("Terminating child process (PID: {d})", .{pid});
140+
std.posix.kill(pid, std.posix.SIG.TERM) catch |err| {
141+
self.log.logErrorFmt("Failed to kill child process: {s}", .{@errorName(err)});
142+
};
143+
self.current_child_pid = null;
157144
}
158-
self.session_id = null;
159145
}
160146

161147
// Internal helper to run with retry logic
162-
fn runWithRetry(self: *Executor, model: []const u8, title: []const u8, prompt: []const u8, continue_session: bool) !ExecutionResult {
148+
fn runWithRetry(self: *Executor, model: []const u8, title: []const u8, prompt: []const u8) !ExecutionResult {
163149
var attempt: u32 = 0;
164150

165151
while (attempt < self.cfg.max_retries) : (attempt += 1) {
166152
if (attempt > 0) {
167153
self.log.logFmt("Retry attempt {d}/{d}", .{ attempt + 1, self.cfg.max_retries });
168154
}
169155

170-
const result = self.runOpencode(model, title, prompt, continue_session);
156+
const result = self.runOpencode(model, title, prompt);
171157
if (result) |output| {
172158
self.allocator.free(output);
173159
return .success;
@@ -201,7 +187,7 @@ pub const Executor = struct {
201187
}
202188

203189
// Run opencode CLI and return output
204-
fn runOpencode(self: *Executor, model: []const u8, title: []const u8, prompt: []const u8, continue_session: bool) ![]u8 {
190+
fn runOpencode(self: *Executor, model: []const u8, title: []const u8, prompt: []const u8) ![]u8 {
205191
var args = std.ArrayListUnmanaged([]const u8){};
206192
defer args.deinit(self.allocator);
207193

@@ -211,14 +197,6 @@ pub const Executor = struct {
211197
try args.append(self.allocator, model);
212198
try args.append(self.allocator, "--title");
213199
try args.append(self.allocator, title);
214-
215-
if (continue_session) {
216-
if (self.session_id) |sid| {
217-
try args.append(self.allocator, "--session");
218-
try args.append(self.allocator, sid);
219-
}
220-
}
221-
222200
try args.append(self.allocator, prompt);
223201

224202
self.log.logVerbose("Running opencode...");
@@ -232,6 +210,10 @@ pub const Executor = struct {
232210

233211
try child.spawn();
234212

213+
// Store PID for potential termination
214+
self.current_child_pid = child.id;
215+
defer self.current_child_pid = null;
216+
235217
// Read stdout
236218
var stdout_list = std.ArrayListUnmanaged(u8){};
237219

@@ -354,7 +336,6 @@ test "Executor.init creates executor" {
354336

355337
const executor = Executor.init(&test_cfg, test_logger, allocator);
356338
try std.testing.expectEqualStrings("opencode", executor.opencode_cmd);
357-
try std.testing.expect(executor.session_id == null);
358339
}
359340

360341
test "Executor.initWithCmd creates executor with custom command" {
@@ -413,7 +394,7 @@ test "runOpencode handles successful execution" {
413394
var executor = Executor.initWithCmd(&test_cfg, test_logger, allocator, mock_path);
414395
defer executor.deinit();
415396

416-
const result = try executor.runOpencode("test/model", "Test", "test prompt", false);
397+
const result = try executor.runOpencode("test/model", "Test", "test prompt");
417398
defer allocator.free(result);
418399

419400
try std.testing.expect(result.len > 0);
@@ -448,43 +429,6 @@ test "runOpencode handles process failure" {
448429
var executor = Executor.initWithCmd(&test_cfg, test_logger, allocator, mock_path);
449430
defer executor.deinit();
450431

451-
const result = executor.runOpencode("test/model", "Test", "test prompt", false);
432+
const result = executor.runOpencode("test/model", "Test", "test prompt");
452433
try std.testing.expectError(error.OpencodeFailed, result);
453434
}
454-
455-
test "runOpencode passes session ID when continuing" {
456-
const allocator = std.testing.allocator;
457-
const test_logger = try createTestLogger(allocator);
458-
defer destroyTestLogger(test_logger, allocator);
459-
460-
const test_cfg = config.Config{
461-
.planning_model = "test/model",
462-
.execution_model = "test/model",
463-
.project_dir = "/tmp",
464-
.verbose = false,
465-
.user_hint = null,
466-
.max_retries = 3,
467-
.backoff_base = 1,
468-
.log_retention = 30,
469-
.max_file_size = 1024 * 1024,
470-
.log_buffer_size = 2048,
471-
.task_pause_seconds = 2,
472-
};
473-
474-
// Get absolute path to mock script
475-
var cwd_buf: [std.fs.max_path_bytes]u8 = undefined;
476-
const cwd = try std.fs.cwd().realpath(".", &cwd_buf);
477-
const mock_path = try std.fs.path.join(allocator, &.{ cwd, "test_helpers/mock_opencode_success.sh" });
478-
defer allocator.free(mock_path);
479-
480-
var executor = Executor.initWithCmd(&test_cfg, test_logger, allocator, mock_path);
481-
defer executor.deinit();
482-
483-
// Set a session ID
484-
executor.session_id = try std.fmt.allocPrint(allocator, "test_session_123", .{});
485-
486-
const result = try executor.runOpencode("test/model", "Test", "test prompt", true);
487-
defer allocator.free(result);
488-
489-
try std.testing.expect(result.len > 0);
490-
}

src/loop.zig

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ const ExecutionResult = @import("executor.zig").ExecutionResult;
1818

1919
/// Shutdown flag for signal handling
2020
var shutdown_requested: bool = false;
21+
/// Signal counter for force kill (2 Ctrl+C = force exit)
22+
var signal_count: u8 = 0;
2123

2224
/// Main execution loop
2325
pub const Loop = struct {
@@ -59,14 +61,17 @@ pub const Loop = struct {
5961
self.log.sayFmt("Hint: {s}", .{hint});
6062
}
6163

62-
self.log.say("Running continuously (Ctrl+C to stop)");
64+
self.log.say("Running continuously (Ctrl+C to stop, Ctrl+C twice to force)");
6365
self.log.say("");
6466

6567
while (!shutdown_requested) {
6668
self.log.say("");
6769
self.log.sayFmt("[Cycle {d}]", .{self.st.cycle});
6870
self.log.setCycle(self.st.cycle);
6971

72+
// Check shutdown before starting any work
73+
if (shutdown_requested) break;
74+
7075
// Planning phase
7176
if (!fsutil.fileExists(self.paths.current_plan) or self.st.phase == .planning) {
7277
const result = self.executor.runPlanning(self.st.cycle) catch |err| {
@@ -133,6 +138,9 @@ pub const Loop = struct {
133138
try self.st.save(self.paths.state_file, self.allocator);
134139
}
135140

141+
// Check shutdown before execution phase
142+
if (shutdown_requested) break;
143+
136144
// Execution phase
137145
self.log.logFmt("[Cycle {d}] Executing tasks...", .{self.st.cycle});
138146

@@ -213,6 +221,9 @@ pub const Loop = struct {
213221
std.Thread.sleep(self.cfg.task_pause_seconds * std.time.ns_per_s);
214222
}
215223

224+
// Check shutdown before evaluation
225+
if (shutdown_requested) break;
226+
216227
// Evaluation phase
217228
const eval_result = evaluator.evaluate(
218229
self.executor,
@@ -253,7 +264,6 @@ pub const Loop = struct {
253264
self.st.task_index = 0;
254265
self.st.current_task_num = 0;
255266
self.st.total_tasks = 0;
256-
self.executor.resetSession();
257267
} else {
258268
// Check if there are actually pending tasks
259269
if (!evaluator.hasPendingTasks(self.paths.current_plan, self.allocator, self.cfg.max_file_size)) {
@@ -272,7 +282,6 @@ pub const Loop = struct {
272282
self.st.task_index = 0;
273283
self.st.current_task_num = 0;
274284
self.st.total_tasks = 0;
275-
self.executor.resetSession();
276285
} else {
277286
self.log.sayFmt("[Cycle {d}] Needs more work, continuing", .{self.st.cycle});
278287
self.st.phase = .execution;
@@ -283,6 +292,13 @@ pub const Loop = struct {
283292
self.log.sayFmt("[Cycle {d}] Complete", .{self.st.cycle -| 1});
284293
}
285294

295+
// Kill any running child process on shutdown
296+
if (shutdown_requested) {
297+
self.log.say("");
298+
self.log.say("Shutdown requested, cleaning up...");
299+
self.executor.killCurrentChild();
300+
}
301+
286302
self.log.say("");
287303
self.log.say("Loop stopped");
288304
}
@@ -307,7 +323,15 @@ pub fn setupSignalHandlers() void {
307323

308324
fn handleSignal(sig: i32) callconv(std.builtin.CallingConvention.c) void {
309325
_ = sig;
326+
signal_count += 1;
310327
shutdown_requested = true;
328+
329+
// On second signal, force exit immediately
330+
if (signal_count >= 2) {
331+
const stderr_file = std.fs.File{ .handle = posix.STDERR_FILENO };
332+
_ = stderr_file.write("\n\nForce killing...\n") catch {};
333+
posix.exit(130); // 128 + SIGINT(2) = 130
334+
}
311335
}
312336

313337
/// Request shutdown (for programmatic use)
@@ -456,7 +480,7 @@ test "Loop.init creates loop with correct fields" {
456480
test "backoffSleep calculates correct sleep time" {
457481
const allocator = std.testing.allocator;
458482

459-
// Create test dependencies with backoff_base = 10
483+
// Create test dependencies with backoff_base = 1 for fast tests
460484
const test_logger = try createTestLogger(allocator);
461485
defer destroyTestLogger(test_logger, allocator);
462486

@@ -470,7 +494,7 @@ test "backoffSleep calculates correct sleep time" {
470494
.verbose = false,
471495
.user_hint = null,
472496
.max_retries = 3,
473-
.backoff_base = 10,
497+
.backoff_base = 1,
474498
.log_retention = 30,
475499
.max_file_size = 1024 * 1024,
476500
.log_buffer_size = 2048,
@@ -492,19 +516,16 @@ test "backoffSleep calculates correct sleep time" {
492516
allocator,
493517
);
494518

495-
// Note: We can't easily test the actual sleep without waiting,
496-
// but we can verify the calculation: backoff_base * 2 = 10 * 2 = 20 seconds
497-
// The backoffSleep function uses: self.cfg.backoff_base * 2
498-
try std.testing.expectEqual(@as(u32, 10), loop.cfg.backoff_base);
519+
// The backoffSleep function uses: self.cfg.backoff_base * 2 = 1 * 2 = 2 seconds
520+
try std.testing.expectEqual(@as(u32, 1), loop.cfg.backoff_base);
499521

500-
// Call backoffSleep - it should sleep for 20 seconds, but we can't verify timing in tests
501-
// Just ensure it doesn't crash
522+
// Call backoffSleep - it should sleep for 2 seconds
502523
const start = std.time.nanoTimestamp();
503524
loop.backoffSleep();
504525
const elapsed = std.time.nanoTimestamp() - start;
505526

506-
// Verify it actually slept (at least 19 seconds to account for scheduling)
507-
try std.testing.expect(elapsed >= 19 * std.time.ns_per_s);
527+
// Verify it actually slept (at least 1.9 seconds to account for scheduling)
528+
try std.testing.expect(elapsed >= (19 * std.time.ns_per_s) / 10);
508529
}
509530

510531
test "Loop state transitions between phases" {

0 commit comments

Comments
 (0)