fix: Keep full Sinex workspace verification - #2652
Conversation
|
@codex review |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: c3552ec47d
ℹ️ 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".
| identity_check = ["git", "diff", "--quiet", "--", "Cargo.toml", "Cargo.lock", "flake.nix", "flake.lock"] | ||
| checkpoint_untracked = true | ||
| verification_operations = ["verify_quick"] | ||
| verification_operations = ["check_default"] |
There was a problem hiding this comment.
In a fresh AgentCTL workspace without an existing dev_services job, this makes automatic workspace verification invoke xtask check, whose ensure_compile_ready path requires live PostgreSQL and deliberately errors rather than starting it; check_default has no dependency that creates the lease. Consequently, publication verification now fails before compilation unless an operator happened to start the matching service job manually, whereas the previous verify_quick operation was self-contained. Add a supported service dependency/start mechanism or select a verifier that does not require the database.
AGENTS.md reference: AGENTS.md:L226-L230
Useful? React with 👍 / 👎.
| identity_check = ["git", "diff", "--quiet", "--", "Cargo.toml", "Cargo.lock", "flake.nix", "flake.lock"] | ||
| checkpoint_untracked = true | ||
| verification_operations = ["verify_quick"] | ||
| verification_operations = ["check_default"] |
There was a problem hiding this comment.
When the PostgreSQL lease is already available and this verifier runs successfully, check_default executes only plain xtask check; xtask/src/commands/check.rs:3-10 defines that default as compile-only, with formatting, linting, forbidden-pattern checks, and Nix checks enabled only by additional flags, and it compiles test targets without executing them. Therefore behavioral test failures and forbidden architecture patterns can still pass the sole automatic publication verifier despite this change's goal of restoring full verification. Configure the publication path to run the broad check and all-test commands rather than the fast check alone.
AGENTS.md reference: AGENTS.md:L212-L214
Useful? React with 👍 / 👎.
c3552ec to
5871590
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58715902c2
ℹ️ 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".
| .map(|value| value.as_str().expect("operation name")) | ||
| .collect::<Vec<_>>(), | ||
| vec!["verify_quick"] | ||
| vec!["check_default"] |
There was a problem hiding this comment.
The descriptor still sets workspace.verification_operations = ["verify_quick"], but this changed assertion expects check_default. Consequently, dev_services_cache_identity_excludes_allocated_ports now fails deterministically whenever the xtask test suite runs, preventing the repository's required broad test gate from passing. Either retain the verify_quick expectation or update the descriptor in the same change.
AGENTS.md reference: AGENTS.md:L212-L214
Useful? React with 👍 / 👎.
5871590 to
4fed24d
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fed24da15
ℹ️ 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".
| description = "Check the Nix flake format without starting development services" | ||
| exec = ["nix", "fmt", "--", "--check", "flake.nix"] | ||
| description = "Run database-free compile, lint, format, pattern, and Nix checks" | ||
| exec = ["xtask", "check", "--full", "--skip-preflight"] |
There was a problem hiding this comment.
When AgentCTL verifies a fresh worktree where dev_services has not already been started, the new --skip-preflight argument bypasses ensure_compile_ready (xtask/src/commands/check.rs:504-510) and proceeds to compilation that expands live SQLx macros. The fresh evidence relative to the earlier review is this newly added hidden flag: it changes the failure path but cannot make compilation database-free, so automatic publication still fails unless the matching service happens to exist; arrange a supported dev_services dependency/start before this operation.
AGENTS.md reference: AGENTS.md:L226-L230
Useful? React with 👍 / 👎.
| description = "Check the Nix flake format without starting development services" | ||
| exec = ["nix", "fmt", "--", "--check", "flake.nix"] | ||
| description = "Run database-free compile, lint, format, pattern, and Nix checks" | ||
| exec = ["xtask", "check", "--full", "--skip-preflight"] |
There was a problem hiding this comment.
When this remains the sole workspace verifier via verification_operations = ["verify_quick"], the new --full argument enables formatting, linting, forbidden-pattern, and Nix checks and compiles test targets, but CheckCommand::execute never runs the tests. The fresh evidence is that the replacement command still contains no xtask test operation, so behavioral failures that compile cleanly can pass the only automatic publication gate; include the required broad test operation alongside this check.
AGENTS.md reference: AGENTS.md:L212-L214
Useful? React with 👍 / 👎.
1 participant
Summary
Declare a truthful Sinex quick verification operation while retaining the full workspace verifier for lane publication.
Problem
The merged descriptor routed workspace verification through a flake-format-only command, dropping compilation, tests, schema checks, and forbidden-pattern checks from the publication gate.
Solution
verify_quickrunsnix fmt -- --check flake.nixin the normal pool without development-service setup.check_defaultremains the workspace verification operation, so compilation, tests, schema checks, and forbidden-pattern checks remain on the workspace publication path. The descriptor test asserts the operation command, pool, and workspace binding.Verification
agentctl job start sinex verify_quick --workspace /realm/worktrees/sinex-feature-packet-sinex-ff2u --wait: succeeded,0 / 1 would have been reformatted.SINNIXD_PRINCIPAL=agent-control SINNIXD_OPERATION=verify_quick nix develop --accept-flake-config --command xtask test -p xtask -E 'test(dev_services_cache_identity_excludes_allocated_ports)': 1 passed, 15 skipped.git fetch origin master && git rebase origin/master: up to date.Residual risk: the full
check_defaultgate was not rerun in this lane because shared normal-pool capacity was occupied by unrelated jobs. It remains the publication verifier.