fix: close installer, worker and Record creation gaps - #305
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. |
|
@codex review Please review the final implementation, especially exact PostgreSQL default authorization/transaction lock, cross-queue fatal error ordering, committed create state after failed navigation and owned installer child cancellation. Native macOS currently failed the spinner terminal-tail assertion; that is being investigated before merge. No release will be tagged until final-head CI and review dispositions are complete. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review Final head is now 6dead0e. Since reviewed implementation74711f6, only the installer PTY regression harness changed: it keeps a slave descriptor until queued final output is drained, then closes it and asserts EOF plus final clear after the last frame. Linux dash, Bash5 and official Bash3.2 pass, including controls that reject missing clear/post-clear frames. Native macOS rerun is required. Please review final head; all original PR300 findings have disposition replies and issues301–304. No unresolved material findings from independent local review. |
![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: 6dead0e914
ℹ️ 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".
|
@codex review Final head91fb7e4 fixes the one P2 finding from review6dead0e: required stored default:null now reaches existing conversion/validation. Actual-function regression fails before and passes after;114 Studio tests/build and real Chromium button/shortcut no-POST/zero/null controls pass. Please review this exact final head. All original four findings and the new required-null finding have explicit dispositions. Native CI reruns before merge. |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Final merge disposition: exact head91fb7e41d7b29f4da8faba8becc54503ef77ada0 passes all five CI checks (Linux, macOS, Windows, Go/TypeScript code quality). Automated final-head code review completed with no further findings; security review of unchanged server/installer implementation74711f6 completed. Required-null P2 is reproduced, fixed, regression-tested and resolved; four originating PR300 findings have exact linked dispositions. Independent review is clear. Local validation includes1,260 Go/PostgreSQL tests/no skips, vet, race suites,114 Studio tests/build and Chromium flows. Proceeding with authorized merge and v0.0.8 release; v0.0.7 remains immutable. |
|
Released and publicly verified in v0.0.8, from merged main Downloaded all ten published assets: nine SHA-256 entries pass, all six archives have correct platform/layout/clean source provenance. v0.0.7 assets remain unchanged. Linux public smoke passed default fresh installation, actual published v0.0.7 CLI replacement and project upgrade, PostgreSQL migration/check/data preservation, module verification, repeat no-op upgrade, healthy HTTP and updated Studio assets. Published installer real-curl signal regressions and archive smoke pass. Validation also includes 1,260 Go/PostgreSQL tests with no individual skips, vet/race checks, 114 Studio tests/builds and production Chromium create/navigation/default regressions. Native CI covers Linux AMD64, macOS ARM64 and Windows AMD64 (PowerShell 7/5.1); the other architectures were cross-built and inspected, not executed natively. Scope boundary: untouched ordinary defaults are omitted, while format-naming inputs and collections remain explicit. Protected defaulted naming tokens still need a separate naming-default contract; hooks receive submitted input and PostgreSQL supplies omitted defaults. No public permission API or schema migration change. |
1 participant
This patch closes four reproduced v0.0.7 edge cases. Parent-only installer signals now interrupt and reap the active curl download; fatal worker errors cancel sibling queues before bounded cleanup while preserving the original error. Studio retains committed-create state after unsuccessful navigation and retries navigation without reposting. Untouched database-backed defaults are omitted from new forms, while required null defaults retain client validation; scoped create authorization uses exact PostgreSQL defaults under the INSERT relation lock, preserving explicit field-write restrictions.
Defaulted format-naming inputs and collection payloads remain explicit. Before-create hooks retain the existing empty-create API contract: omitted defaults are absent from submitted input and PostgreSQL supplies them at insertion. A protected format-naming token still requires a separate naming-default contract.
Closes #301
Closes #302
Closes #303
Closes #304
Validation: full Go/PostgreSQL 17 tests and vet; database/worker/permissions race tests; 114 Studio tests and embedded build; actual Chromium button/shortcut flows for pending, aborted, rejected and successful navigation with mocked API transport; PostgreSQL deny-write/default/precision/date/naming/hook/lock regressions. Real stalled-HTTP installer tests cover latest/archive/checksum requests, parent-only INT/TERM, process reaping, preservation and spinner cleanup. Existing native macOS and Windows CI must pass for this exact head before merge.