fix: recover scheduled evolution reviews and close render locks - #82
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4471620ec
ℹ️ 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".
| dirname(facultAiEvolutionLoopConfigPath(args.homeDir, args.rootDir)), | ||
| dirname(facultAiEvolutionLoopLockPath(args.homeDir, args.rootDir)), | ||
| facultAiEvolutionLoopReportDir(args.homeDir, args.rootDir), | ||
| facultAiEvolutionReviewDir(args.homeDir, args.rootDir), | ||
| facultAiReconciliationReviewDir(args.homeDir, args.rootDir), |
There was a problem hiding this comment.
This set omits mandatory runtime sinks such as the parent of facultAiReconciliationStatePath and the activity-history directory, as well as conditional writeback, proposal, draft, and journal directories. If one of those sibling directories already exists with narrower permissions or is excluded by the scheduler sandbox, these probes all succeed and preflight reports ready, but the subsequent loop still fails while persisting reconciliation/history or materializing a signal. Include every mutated runtime sink in the checks rather than inferring sibling writability from the loop config directory.
AGENTS.md reference: AGENTS.md:L41-L43
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| console.error(error instanceof Error ? error.message : String(error)); | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| if (json && (sub === "run" || sub === "preflight")) { |
There was a problem hiding this comment.
The new JSON error handler only covers exceptions raised after entering this try, while parseCliContextArgs, resolveCliContextRoot, scope inference, and the evolution-loop import all run beforehand. For example, loop run --project --json from a directory without a resolvable project throws during context resolution and produces an unstructured top-level error instead of the promised {status:"failed", queueAvailable:false} response. Move context setup inside the JSON-aware error boundary so automation can reliably parse every pre-report failure.
AGENTS.md reference: AGENTS.md:L130-L130
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aff5493069
ℹ️ 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".
| for (const path of paths) { | ||
| try { | ||
| await mkdir(path, { recursive: true }); | ||
| const probe = await mkdtemp(join(path, ".fclt-preflight-")); |
There was a problem hiding this comment.
When an existing audit.jsonl, journal events.jsonl, or writeback queue.jsonl is read-only while its parent directory remains writable, this temporary-directory probe succeeds and preflight reports ready, but the loop later fails at appendFile—the audit file is mandatory on every non-dry run. Even after adding all sink directories, preflight must also test append access to existing append-only files so scheduled reviews are not started with a false-ready result.
Useful? React with 👍 / 👎.
| if ( | ||
| sub !== "loop" || | ||
| !rest.includes("--json") || | ||
| (rest[0] !== "run" && rest[0] !== "preflight") | ||
| ) { |
There was a problem hiding this comment.
Fresh evidence after the context-boundary fix is that parseCliContextArgs accepts context flags anywhere, but this check inspects the raw first argument. Thus fclt ai loop --project run --json from outside a project fails context resolution and is rethrown as an unstructured top-level error because rest[0] is --project, despite being semantically identical to the covered loop run --project --json form. Base this decision on the parsed argv/subcommand so every supported ordering preserves the JSON error contract.
AGENTS.md reference: AGENTS.md:L130-L130
Useful? React with 👍 / 👎.
|
🎉 This PR is included in version 2.30.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Scheduled evolution reviews could fail before producing JSON on denied state writes, fail again while automatically drafting a non-Markdown proposal, or truncate a large JSON result when piped. This change adds an environment preflight, preserves unsupported proposals for manual implementation, returns structured pre-report errors, and flushes loop output before exit.
It also closes the render mutation-lock descriptor when advisory-lock acquisition fails, and fixes environment restoration in the CLI test fixture.
Related: HACK-1045.
Validation:
Preflight makes temporary write probes and may create missing state/review directories; it does not invoke reconciliation or change host permissions. Existing schedules remain untouched. Unsupported proposal targets remain approval-needed; no canonical apply capability is added.