fix: keep task completion, cancel, pause, and undo honest
Pending earlier-revision jobs no longer skip completion repair; late cancels preserve real exits; begin clears pause; partial undo keeps snapshots; permissions and worktree failure paths are hardened. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
+39
-7
@@ -979,7 +979,20 @@ static void sandbox_escalate(turn_t *t, cst_tool_ctx_t *ctx,
|
|||||||
cst_perms_allow_tool(a->perms, c->name);
|
cst_perms_allow_tool(a->perms, c->name);
|
||||||
}
|
}
|
||||||
hook_status(t, "rerunning outside the sandbox");
|
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);
|
cst_tool_result_free(tr);
|
||||||
memset(tr, 0, sizeof(*tr));
|
memset(tr, 0, sizeof(*tr));
|
||||||
ctx->sandbox = 0;
|
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) {
|
if (!a->cfg.depth && a->cfg.tool_profile != CST_TOOLS_MINIMAL) {
|
||||||
char why[256];
|
char why[256];
|
||||||
if (cst_runtime_jobs_pending(a->cfg.runtime))
|
/* Do not pre-mark partial before finish(completed): that made the
|
||||||
cst_runtime_finish(a->cfg.runtime, "partial", "Earlier-revision jobs remain active; inspect task_state for their outcomes.", 0, why, sizeof(why));
|
* intentional no-op (already partial → return 0) look like success and
|
||||||
if (cst_runtime_finish(a->cfg.runtime, "completed", body, tools_ran, why, sizeof(why))) {
|
* 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++) {
|
if (!completion_repairs++) {
|
||||||
append_reply(&t, body, NULL, meta_json("progress", proto));
|
append_reply(&t, body, NULL, meta_json("progress", proto));
|
||||||
append_msg(a, CST_ROLE_USER, why, NULL, NULL, NULL, meta_json("completion_repair", proto));
|
append_msg(a, CST_ROLE_USER, why, NULL, NULL, NULL,
|
||||||
turn_free(&t); continue;
|
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));
|
append_reply(&t, body, NULL, meta_json(NULL, proto));
|
||||||
|
|||||||
+10
-1
@@ -83,12 +83,21 @@ static int save(const cst_perms_t *p) {
|
|||||||
cJSON_Delete(root);
|
cJSON_Delete(root);
|
||||||
if (!txt)
|
if (!txt)
|
||||||
return -1;
|
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;
|
int rc = -1;
|
||||||
if (fp) {
|
if (fp) {
|
||||||
rc = fputs(txt, fp) >= 0 && fputs("\n", fp) >= 0 ? 0 : -1;
|
rc = fputs(txt, fp) >= 0 && fputs("\n", fp) >= 0 ? 0 : -1;
|
||||||
if (fclose(fp) != 0)
|
if (fclose(fp) != 0)
|
||||||
rc = -1;
|
rc = -1;
|
||||||
|
if (rc == 0)
|
||||||
|
rc = os_rename_replace(tmp, p->path);
|
||||||
|
if (rc != 0)
|
||||||
|
remove(tmp);
|
||||||
}
|
}
|
||||||
free(txt);
|
free(txt);
|
||||||
return rc;
|
return rc;
|
||||||
|
|||||||
+17
-5
@@ -184,6 +184,9 @@ unsigned long cst_runtime_begin(cst_runtime_t *r, const char *goal) {
|
|||||||
if (!r) return 0;
|
if (!r) return 0;
|
||||||
pthread_mutex_lock(&r->mu);
|
pthread_mutex_lock(&r->mu);
|
||||||
r->revision++; r->evidence = r->failures = r->mutations = r->last_failure = 0;
|
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;
|
replace(&r->goal, goal); replace(&r->summary, ""); r->cause[0] = 0;
|
||||||
snprintf(r->status, sizeof(r->status), "running");
|
snprintf(r->status, sizeof(r->status), "running");
|
||||||
event_locked(r, "task_started", ""); save_locked(r);
|
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 (!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);
|
if (j->destroy) j->destroy(j->data);
|
||||||
pthread_mutex_lock(&r->mu);
|
pthread_mutex_lock(&r->mu);
|
||||||
/* A cancellation request arriving after a successful side effect must not
|
/* Cancellation before/during the run yields 130. A cancel that arrives
|
||||||
* rewrite that success into "cancelled". Keep the request source separately. */
|
* after the run returns must not rewrite a real exit: success stays
|
||||||
j->exit_code = cancelled ? 130 : rc ? (j->cancel ? 130 : 1) : result.exit_code;
|
* completed, and a failed command stays failed (cause still records the
|
||||||
if (j->cancel && j->exit_code != 0) j->exit_code = 130;
|
* 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;
|
if (j->revision == r->revision) r->last_failure = j->exit_code != 0;
|
||||||
j->output = result.out ? result.out : strdup(""); result.out = NULL;
|
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);
|
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;
|
free(out->out); out->out = strdup(cfg->runtime->jobs[i].output); break;
|
||||||
}
|
}
|
||||||
pthread_mutex_unlock(&cfg->runtime->mu);
|
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);
|
free(text);
|
||||||
} else { out->out = text; out->exit_code = 0; }
|
} else { out->out = text; out->exit_code = 0; }
|
||||||
cJSON_Delete(j); out->out_len = out->out ? strlen(out->out) : 0; return 0;
|
cJSON_Delete(j); out->out_len = out->out ? strlen(out->out) : 0; return 0;
|
||||||
|
|||||||
+4
-1
@@ -269,7 +269,10 @@ int cst_undo_restore_last(const char *ws, const char *sid, char *report,
|
|||||||
}
|
}
|
||||||
fclose(fp);
|
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)
|
if (!rep.buf)
|
||||||
cst_buf_puts(&rep, "nothing changed\n");
|
cst_buf_puts(&rep, "nothing changed\n");
|
||||||
snprintf(report, report_sz, "%s", rep.buf);
|
snprintf(report, report_sz, "%s", rep.buf);
|
||||||
|
|||||||
+8
-1
@@ -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; }
|
if (code) { snprintf(err, en, "git worktree add failed"); pthread_mutex_unlock(&git_mu); return -1; }
|
||||||
snprintf(meta, sizeof(meta), "%s.scope.json", out);
|
snprintf(meta, sizeof(meta), "%s.scope.json", out);
|
||||||
FILE *f = fopen(meta, "wb"); if (f) { fputs(files, f); fclose(f); }
|
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 *cst_worktree_diff(const char *ws, const char *wt, int integrate, int *code) {
|
||||||
char prefix[OS_PATH_MAX], meta[OS_PATH_MAX + 16];
|
char prefix[OS_PATH_MAX], meta[OS_PATH_MAX + 16];
|
||||||
|
|||||||
+18
-2
@@ -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) {
|
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;
|
(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) {
|
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));
|
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));
|
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, "partial", "explicit partial", 0, error, sizeof(error)));
|
||||||
assert(!cst_runtime_finish(r, "completed", "must not overwrite 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);
|
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);
|
cst_runtime_close(r);
|
||||||
r = cst_runtime_open(path); assert(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);
|
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);
|
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\"}]}");
|
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));
|
r = cst_runtime_open(path); assert(r && !cst_runtime_jobs_pending(r));
|
||||||
|
|||||||
@@ -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", "../evil") != 0, "reject ..");
|
||||||
EXPECT(cst_undo_snapshot(g_ws, sid, "t4", "/etc/passwd") != 0,
|
EXPECT(cst_undo_snapshot(g_ws, sid, "t4", "/etc/passwd") != 0,
|
||||||
"reject absolute");
|
"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) {
|
static void test_perms(void) {
|
||||||
|
|||||||
Reference in New Issue
Block a user