Fix: harden guided checkpoint reuse - #142
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9c026c563
鈩癸笍 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".
| index($0, "\"name\" : \"" key "\"") { in_entry = 1; found = 1 } | ||
| in_entry && index($0, "\"path\" : \"" expected_path "\"") { path_matches = 1 } | ||
| in_entry && /^ }/ { exit(path_matches ? 0 : 1) } | ||
| END { if (!found || !path_matches) exit 1 } |
There was a problem hiding this comment.
Parse compatible schemas independently of JSON formatting
When a valid pre-existing schema.json was minified, reformatted, or serialized with path before name, these whitespace- and order-sensitive patterns do not find the entry even though the CLI's JSONDecoder accepts it. Both guided scripts then return 65 for a fully compatible checkpoint schema, so the path must be obtained through structural JSON parsing rather than matching the CLI writer's current pretty-print layout; the identical helper in examples/release-pipeline/run.sh has the same failure.
Useful? React with 馃憤聽/ 馃憥.
| in_entry && index($0, "\"path\" : \"" expected_path "\"") { path_matches = 1 } | ||
| in_entry && /^ }/ { exit(path_matches ? 0 : 1) } | ||
| END { if (!found || !path_matches) exit 1 } | ||
| ' "$APS_HOME/schema.json" |
There was a problem hiding this comment.
Resolve APS_HOME the same way as the CLI
When APS_HOME contains a supported tilde path such as APS_HOME='~/.agents/codex', aps expands it in APSPaths.resolve, but this direct file access tries to open a literal ~/.../schema.json relative to the working directory. The preceding CLI calls therefore operate on the intended root while validation fails, potentially after adding the first key; use the CLI-resolved root rather than concatenating the raw environment value. The release-pipeline helper has the same direct access.
Useful? React with 馃憤聽/ 馃憥.
| "$aps_bin" key add "$name" \ | ||
| --type "$type" --storage FileState --path "$path" --doc "$doc" \ | ||
| "$@" >/dev/null |
There was a problem hiding this comment.
Validate every existing key before adding missing ones
When an incompatible collision occurs on a later checkpoint key while one or more earlier keys are absent, each earlier ensure_key call adds its key before the later collision is examined. For example, a root containing only an incompatible blocker leaves currentIssue, workingBranch, phase, and testsPassed added even though the script exits 65, contradicting the change's stated fail-before-mutation behavior. Both scripts need a complete validation preflight before performing any key add operations.
Useful? React with 馃憤聽/ 馃憥.
Summary
Test Plan
fledge lanes run verify