Skip to content

fix: refresh after failed mutating exec runs#59

Open
ranveersequeira wants to merge 1 commit intomainfrom
fix/runexec-refresh-on-fail-48
Open

fix: refresh after failed mutating exec runs#59
ranveersequeira wants to merge 1 commit intomainfrom
fix/runexec-refresh-on-fail-48

Conversation

@ranveersequeira
Copy link
Copy Markdown
Collaborator

Summary

  • continue through refresh and validation after a non-zero wrapped command when runExec detects repository mutations
  • preserve the wrapped command exit code instead of masking it with stale-validation exit codes
  • add regressions for both runExec and agentify exec --fail-on-stale when a failing command edits tracked files

Testing

  • node --test test/exec.test.js
  • node --test test/main.test.js
  • npm test

Fixes #48

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac74e5a1d9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +309 to +312
if (preparedSessionMemory) {
await finalizeSessionMemoryRun(root, flags.sessionRecord, preparedSessionMemory, result, config);
}
return result;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Continue validation when refresh fails without strict mode

This catch block now always returns refresh-error, even when --fail-on-stale is not set, so validateRepo() is skipped for every refresh failure. In the previous flow, non-strict runs continued to validation and still reported stale/unsafe changes after a partial refresh; with this change, those checks are never emitted and session records show validation as not run. Please keep the early return only for strict failure cases and let non-strict runs fall through to validation while preserving the wrapped command's exit code.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex fix this

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

  • Updated runExec so refresh failures only short-circuit with phase: "refresh-error" in the strict stale-check case (--fail-on-stale with a non-failing wrapped command). Non-strict paths now continue through validateRepo() and preserve the wrapped command’s exit code behavior. src/core/exec.jsL292-L327
  • Preserved refresh failure context for non-strict runs by appending the refresh error message into the returned stderr while still returning phase: "complete" with validation results. src/core/exec.jsL330-L338
  • Added a regression test that simulates a failing mutating command plus a refresh-time error, and verifies non-strict exec still validates and keeps the command exit code. test/exec.test.jsL154-L192

Testing

  • node --test test/exec.test.js
  • ⚠️ node --test test/main.test.js (one unrelated failing test in this environment: runCli doctor reports MemPalace available via AGENTIFY_MEMPALACE_CMD)

Committed on the current branch (46118d1) and created a follow-up PR with title “fix: continue exec validation after non-strict refresh errors”.

View task →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[reliability] failed runExec children can leave stale artifacts without refresh or validation

1 participant