diff --git a/src/agent_loop.c b/src/agent_loop.c index 9fc3188..ed3f021 100644 --- a/src/agent_loop.c +++ b/src/agent_loop.c @@ -979,7 +979,20 @@ static void sandbox_escalate(turn_t *t, cst_tool_ctx_t *ctx, cst_perms_allow_tool(a->perms, c->name); } hook_status(t, "rerunning outside the sandbox"); - if (a->cancel || !cst_runtime_check(a->cfg.runtime, t->revision, 1, 0)) return; + if (a->cancel || !cst_runtime_check(a->cfg.runtime, t->revision, 1, 0)) { + const char *note = + "\n[cst: the user allowed rerunning outside the sandbox, but the " + "task was cancelled or its instructions changed before the " + "rerun; sandbox output above is unchanged]\n"; + cst_buf_t b = {0}; + if (tr->out) + cst_buf_append(&b, tr->out, tr->out_len); + cst_buf_puts(&b, note); + free(tr->out); + tr->out_len = b.len; + tr->out = cst_buf_steal(&b); + return; + } cst_tool_result_free(tr); memset(tr, 0, sizeof(*tr)); ctx->sandbox = 0; @@ -3200,15 +3213,34 @@ static int run_turn_once(cst_agent_t *a, const char *user_text, } if (!a->cfg.depth && a->cfg.tool_profile != CST_TOOLS_MINIMAL) { char why[256]; - if (cst_runtime_jobs_pending(a->cfg.runtime)) - cst_runtime_finish(a->cfg.runtime, "partial", "Earlier-revision jobs remain active; inspect task_state for their outcomes.", 0, why, sizeof(why)); - if (cst_runtime_finish(a->cfg.runtime, "completed", body, tools_ran, why, sizeof(why))) { + /* Do not pre-mark partial before finish(completed): that made the + * intentional no-op (already partial → return 0) look like success and + * skipped the one-shot completion repair. Pending earlier-revision + * jobs fail completed with an explicit reason instead. */ + if (cst_runtime_jobs_pending(a->cfg.runtime)) { + snprintf(why, sizeof(why), + "Earlier-revision jobs remain active; inspect task_state " + "for their outcomes."); if (!completion_repairs++) { append_reply(&t, body, NULL, meta_json("progress", proto)); - append_msg(a, CST_ROLE_USER, why, NULL, NULL, NULL, meta_json("completion_repair", proto)); - turn_free(&t); continue; + append_msg(a, CST_ROLE_USER, why, NULL, NULL, NULL, + meta_json("completion_repair", proto)); + turn_free(&t); + continue; } - cst_runtime_finish(a->cfg.runtime, "partial", body, 0, why, sizeof(why)); + cst_runtime_finish(a->cfg.runtime, "partial", body, 0, why, + sizeof(why)); + } else if (cst_runtime_finish(a->cfg.runtime, "completed", body, + tools_ran, why, sizeof(why))) { + if (!completion_repairs++) { + append_reply(&t, body, NULL, meta_json("progress", proto)); + append_msg(a, CST_ROLE_USER, why, NULL, NULL, NULL, + meta_json("completion_repair", proto)); + turn_free(&t); + continue; + } + cst_runtime_finish(a->cfg.runtime, "partial", body, 0, why, + sizeof(why)); } } append_reply(&t, body, NULL, meta_json(NULL, proto)); diff --git a/src/permissions.c b/src/permissions.c index e1f687d..91ecda4 100644 --- a/src/permissions.c +++ b/src/permissions.c @@ -83,12 +83,21 @@ static int save(const cst_perms_t *p) { cJSON_Delete(root); if (!txt) return -1; - FILE *fp = fopen(p->path, "wb"); + char tmp[OS_PATH_MAX + 8]; + if (snprintf(tmp, sizeof(tmp), "%s.tmp", p->path) >= (int)sizeof(tmp)) { + free(txt); + return -1; + } + FILE *fp = fopen(tmp, "wb"); int rc = -1; if (fp) { rc = fputs(txt, fp) >= 0 && fputs("\n", fp) >= 0 ? 0 : -1; if (fclose(fp) != 0) rc = -1; + if (rc == 0) + rc = os_rename_replace(tmp, p->path); + if (rc != 0) + remove(tmp); } free(txt); return rc; diff --git a/src/runtime.c b/src/runtime.c index dee34f1..86b7406 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -184,6 +184,9 @@ unsigned long cst_runtime_begin(cst_runtime_t *r, const char *goal) { if (!r) return 0; pthread_mutex_lock(&r->mu); r->revision++; r->evidence = r->failures = r->mutations = r->last_failure = 0; + /* A new user turn clears pause: pause is a control for the previous turn, + * not a sticky lock across messages (resume is still available mid-turn). */ + r->paused = 0; replace(&r->goal, goal); replace(&r->summary, ""); r->cause[0] = 0; snprintf(r->status, sizeof(r->status), "running"); event_locked(r, "task_started", ""); save_locked(r); @@ -318,10 +321,16 @@ static void *job_main(void *p) { if (!cancelled) { os_set_cancel_flag(&j->cancel); os_set_output_observer(&observer); rc = j->run(j->data, &j->cancel, &result); os_set_output_observer(NULL); os_set_cancel_flag(NULL); } if (j->destroy) j->destroy(j->data); pthread_mutex_lock(&r->mu); - /* A cancellation request arriving after a successful side effect must not - * rewrite that success into "cancelled". Keep the request source separately. */ - j->exit_code = cancelled ? 130 : rc ? (j->cancel ? 130 : 1) : result.exit_code; - if (j->cancel && j->exit_code != 0) j->exit_code = 130; + /* Cancellation before/during the run yields 130. A cancel that arrives + * after the run returns must not rewrite a real exit: success stays + * completed, and a failed command stays failed (cause still records the + * cancel source). */ + if (cancelled) + j->exit_code = 130; + else if (rc) + j->exit_code = j->cancel ? 130 : 1; + else + j->exit_code = result.exit_code; if (j->revision == r->revision) r->last_failure = j->exit_code != 0; j->output = result.out ? result.out : strdup(""); result.out = NULL; char artifact[OS_PATH_MAX + 64]; snprintf(artifact, sizeof(artifact), "%s.%s", r->path, j->id); @@ -445,7 +454,10 @@ int cst_runtime_shell(const cst_tool_ctx_t *ctx, const ctl_tool_call_t *c, cst_t free(out->out); out->out = strdup(cfg->runtime->jobs[i].output); break; } pthread_mutex_unlock(&cfg->runtime->mu); - out->exit_code = cJSON_GetObjectItemCaseSensitive(j, "exit_code")->valueint; + { + const cJSON *ex = cJSON_GetObjectItemCaseSensitive(j, "exit_code"); + out->exit_code = cJSON_IsNumber(ex) ? ex->valueint : 1; + } free(text); } else { out->out = text; out->exit_code = 0; } cJSON_Delete(j); out->out_len = out->out ? strlen(out->out) : 0; return 0; diff --git a/src/undo.c b/src/undo.c index 62ae104..8f357d2 100644 --- a/src/undo.c +++ b/src/undo.c @@ -269,7 +269,10 @@ int cst_undo_restore_last(const char *ws, const char *sid, char *report, } fclose(fp); } - rm_dir(dir); + /* Keep the snapshot if anything failed so /undo can be retried; only + * drop it when every targeted file was handled. */ + if (!failed) + rm_dir(dir); if (!rep.buf) cst_buf_puts(&rep, "nothing changed\n"); snprintf(report, report_sz, "%s", rep.buf); diff --git a/src/worktree.c b/src/worktree.c index daa6126..2b726a2 100644 --- a/src/worktree.c +++ b/src/worktree.c @@ -87,7 +87,14 @@ int cst_worktree_create(const char *ws, const char *files, char *out, size_t n, if (code) { snprintf(err, en, "git worktree add failed"); pthread_mutex_unlock(&git_mu); return -1; } snprintf(meta, sizeof(meta), "%s.scope.json", out); FILE *f = fopen(meta, "wb"); if (f) { fputs(files, f); fclose(f); } - pthread_mutex_unlock(&git_mu); return f ? 0 : -1; + if (!f) { + cst_buf_puts(&cmd, "git worktree remove --force "); quote(&cmd, out); + free(run(ws, &cmd, &code)); + snprintf(err, en, "cannot write worktree scope manifest"); + pthread_mutex_unlock(&git_mu); + return -1; + } + pthread_mutex_unlock(&git_mu); return 0; } char *cst_worktree_diff(const char *ws, const char *wt, int integrate, int *code) { char prefix[OS_PATH_MAX], meta[OS_PATH_MAX + 16]; diff --git a/tests/test_runtime.c b/tests/test_runtime.c index b3ea2b8..6228f90 100644 --- a/tests/test_runtime.c +++ b/tests/test_runtime.c @@ -26,6 +26,14 @@ static int locked_work(void *p, volatile int *cancel, cst_tool_result_t *out) { static int instant(void *p, volatile int *cancel, cst_tool_result_t *out) { (void)p; (void)cancel; out->out = strdup("archived output"); out->out_len = strlen(out->out); return 0; } +/* Runs to a failed exit even if cancel raced after the side effect. */ +static int fail_then_note_cancel(void *p, volatile int *cancel, cst_tool_result_t *out) { + (void)p; + out->out = strdup("command failed"); out->out_len = strlen(out->out); + out->exit_code = 7; + if (cancel) *cancel = 1; /* late cancel must not rewrite exit 7 → 130 */ + return 0; +} static void write_text(const char *dir, const char *name, const char *text) { char p[8192]; assert(!os_path_join(p, sizeof(p), dir, name)); FILE *f = fopen(p, "wb"); assert(f); assert(fputs(text, f) >= 0); assert(!fclose(f)); @@ -72,11 +80,19 @@ int main(void) { assert(!cst_runtime_finish(r, "partial", "explicit partial", 0, error, sizeof(error))); assert(!cst_runtime_finish(r, "completed", "must not overwrite partial", 0, error, sizeof(error))); s = cst_runtime_json(r); assert(s && strstr(s, "explicit partial")); free(s); - assert(!cst_runtime_controls(r, 1, 0, 1)); + /* Pause sticks across reopen; begin() clears it so a new turn can run. */ + assert(!cst_runtime_controls(r, 0, 1, 1)); cst_runtime_close(r); r = cst_runtime_open(path); assert(r); - assert(!cst_runtime_check(r, 0, 0, 0)); + assert(!cst_runtime_check(r, 0, 1, 1)); s = cst_job_read(r, id, 0); assert(s && strstr(s, "cancelled")); free(s); + unsigned long next = cst_runtime_begin(r, "after pause"); + assert(cst_runtime_check(r, next, 1, 1)); + assert(!cst_job_start(r, "shell", "", 1, fail_then_note_cancel, NULL, NULL, id, + sizeof(id))); + s = cst_job_read(r, id, 2000); + assert(s && strstr(s, "\"status\":\"failed\"") && strstr(s, "\"exit_code\":7")); + free(s); cst_runtime_close(r); write_text(dir, "session.task-v2.json", "{\"revision\":7,\"sequence\":1,\"status\":\"running\",\"jobs\":[{\"job_id\":\"job-7-1\",\"status\":\"running\",\"kind\":\"external\"}]}"); r = cst_runtime_open(path); assert(r && !cst_runtime_jobs_pending(r)); diff --git a/tests/test_undo.c b/tests/test_undo.c index 5ef0e59..6b1352c 100644 --- a/tests/test_undo.c +++ b/tests/test_undo.c @@ -93,6 +93,21 @@ static void test_undo(void) { EXPECT(cst_undo_snapshot(g_ws, sid, "t4", "../evil") != 0, "reject .."); EXPECT(cst_undo_snapshot(g_ws, sid, "t4", "/etc/passwd") != 0, "reject absolute"); + + /* Partial restore must keep the snapshot so a retry is possible. */ + EXPECT(cst_undo_snapshot(g_ws, sid, "t5", "keep.txt") == 0, "snap keep"); + wfile("keep.txt", "after\n"); + { + char target[512]; + snprintf(target, sizeof(target), "%s/keep.txt", g_ws); + unlink(target); + EXPECT(symlink("elsewhere", target) == 0, "make symlink"); + EXPECT(cst_undo_restore_last(g_ws, sid, rep, sizeof(rep)) != 0, + "partial undo fails"); + EXPECT(strstr(rep, "symlink") != NULL, "report names symlink skip"); + EXPECT(cst_undo_count(g_ws, sid) == 1, "snapshot kept after partial fail"); + unlink(target); + } } static void test_perms(void) {