fix: repair CLI lifecycle and confirmed data, Studio, and runtime bugs - #300
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. |
|
Candidate validation on head
|
![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: 9942d3eb98
ℹ️ 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".
| trap 'exit 130' INT | ||
| trap 'exit 143' TERM |
There was a problem hiding this comment.
When a supervisor sends SIGTERM directly to the installer PID while it is waiting on a foreground curl, the shell defers this trap until curl exits; because the downloads have no total timeout, a stalled download can keep the installer alive indefinitely instead of cancelling. This is reproducible with a sleeping fake curl (kill -TERM <installer-pid> leaves the installer running), while the new cancellation test misses it because its fake child exits immediately after signaling the parent. Forward termination to the active child or process group.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed independently on published v0.0.7 with real curl and parent-only SIGTERM. Tracked precisely in #301 (related #273) and fixed in PR #305 using tracked asynchronous curl + interruptible wait and owned-child termination/reaping. Six real stalled-HTTP regressions cover metadata/archive/checksums × INT/TERM; native macOS CI pending on final head. Existing v0.0.7 remains untouched.
| defer func() { | ||
| cancel() | ||
| w.shutdownActiveExecutions(ctx, &inFlight, active, options) | ||
| }() |
There was a problem hiding this comment.
When one queue encounters a Store or finalization error while an active handler ignores cancellation, this defer waits up to ShutdownTimeout and may then spend another timeout expiring claims before runQueueLoop returns the error. runContinuous does not cancel the shared worker context until it receives that return, so sibling queues can continue claiming and starting executions throughout the drain interval. Notify or cancel the worker before performing the bounded per-queue cleanup.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed independently: sibling queues can start new work during failed queue drain/expiry. Follow-up #302 and PR #305 publish the originating error and cancel shared worker context before local cleanup, preserving the original error against sibling cancellation races. Deterministic two-queue/slow-expiry regression passes with -race and fails on v0.0.7. Original #294 per-queue cleanup remains intact.
| const showForm = computed(() => Boolean(entityMeta.value) && (isNew.value || Boolean(record.value))) | ||
| const dirty = computed(() => fields.value.some((field) => !draftValuesEqual(draft.value[field.name], baseline.value[field.name]))) | ||
| const canSave = computed(() => showForm.value && dirty.value && !loading.value && !saving.value && !isSystem.value) | ||
| const canSave = computed(() => showForm.value && (isNew.value || dirty.value) && !loading.value && !saving.value && !isSystem.value) |
There was a problem hiding this comment.
If the create succeeds but router.replace is delayed, aborted, or throws, mode remains new and this expression becomes true again as soon as the mutation stops pending. Because resetToRecord no longer disables saving through dirty, the broad catch leaves the successfully created form enabled indefinitely, and another click or save shortcut POSTs a duplicate Record. Track the completed create/navigation state so Create remains disabled until the route changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed with narrower disposition: aborted/thrown navigation allows another user-triggered POST; mere pending navigation is protected by existing command in-flight gating. #303 / PR #305 retain committed-create state, disable further create and provide navigation-only recovery. Actual production UI Chromium button/shortcut cases cover pending, abort, reject, successful navigation and failed-mutation retry.
| const showForm = computed(() => Boolean(entityMeta.value) && (isNew.value || Boolean(record.value))) | ||
| const dirty = computed(() => fields.value.some((field) => !draftValuesEqual(draft.value[field.name], baseline.value[field.name]))) | ||
| const canSave = computed(() => showForm.value && dirty.value && !loading.value && !saving.value && !isSystem.value) | ||
| const canSave = computed(() => showForm.value && (isNew.value || dirty.value) && !loading.value && !saving.value && !isSystem.value) |
There was a problem hiding this comment.
For a user with create permission but deny-write on a defaulted field, enabling an untouched new form here still cannot create the Record: new-form payload construction submits every metadata default, so the server evaluates the protected field's FieldWrite predicate and rejects the request. An empty payload would allow PostgreSQL to apply the same default without writing the denied field, so untouched defaults should be omitted unless the user changes them.
AGENTS.md reference: AGENTS.md:L97-L101
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed: untouched ordinary metadata defaults are explicitly sent and rejected by FieldWrite, while empty input lets PostgreSQL supply the same default. #304 / PR #305 omit ordinary untouched database-backed defaults and use exact stored defaults for scoped row/conditional-field authorization under a transaction-held INSERT table lock. Explicit denied writes stay denied; precision/date/naming/hooks regression-tested. Format-naming token defaults remain explicit because current naming requires input, a documented scope boundary.
|
Published v0.0.7 from clean main All 10 assets are present; every listed SHA-256 checksum passed. Final public Linux AMD64 fresh installation and actual v0.0.6-to-v0.0.7 CLI/project upgrade passed with public Go module resolution, runner builds, PostgreSQL preparation/migration, preserved test data, health and embedded Studio assets. Current-version/no-op upgrade passed. Native candidate installer/update tests also passed on macOS ARM64 and Windows AMD64 (PowerShell 7 and 5.1). Linux ARM64, macOS AMD64 and Windows ARM64 were cross-built and inspected only, not executed natively. Full Go/PostgreSQL tests (1,236), Studio tests (110), vet, focused race checks and exact-head CI passed. The installer subset is shipped; #273 remains open for discovery/uninstall work. Other deferred audit findings remain tracked in #143, #291 and #296–#299. |
1 participant
This patch repairs install/update failure handling, gives long CLI operations visible progress, and fixes confirmed Record, Studio, import and worker bugs found during the release audit. Issues and reproductions were recorded before implementation.
Validation on Linux: 1,236 Go tests with PostgreSQL 17 and no individual skips; go vet; focused CLI/data/import/worker race checks; 110 Studio tests; production and embedded Studio builds; Chromium form smoke with mocked API; installer recovery/signal tests; all six release archives and checksums built; Linux archive smoke passed. Actual published v0.0.6 installed, generated/tidied/built a project, prepared PostgreSQL and served health plus Studio assets. Native-host CI and the final published-release upgrade smoke are required before release completion.
Fixes #287
Fixes #288
Fixes #289
Fixes #290
Fixes #292
Fixes #293
Fixes #294
Fixes #295
Refs #273 (installer-based lifecycle subset; discovery/uninstall design remains open).
Deferred with reproductions and acceptance: #143 (interrupted import idempotency), #291 (required attachment creation), #296/#297 (permission-scoped update/fetch semantics), #298 (Git transport descendants), #299 (cancellation while waiting for confirmation). No permission bypass was demonstrated in #296/#297. Draft #285 is not included.