fix(ops): make named operator checks fail on blocked readiness with advisory aliases explicitly named - #133
Conversation
…dvisory aliases explicitly named The named readiness/operator npm commands (client:check, prospect:check, send:setup, prospect:site-check, market:parity, market:proof-check) ran the underlying check scripts without --strict, so blocked readiness exited 0 every time. Wire --strict into each named command so a blocked fixture fails loudly with no hidden flag, keep report-only behavior under explicitly named :advisory aliases, and add a fixture-driven strictness test (blocked fixtures fail, green fixtures pass, advisory aliases exit 0) to the required ci/test gates. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (4)
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
![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: 50fc6614e4
ℹ️ 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".
| delete agencyConfig.senderEmail | ||
| delete agencyConfig.senderPhysicalAddress | ||
| delete agencyConfig.dkimSelector | ||
| delete agencyConfig.senderDomain |
There was a problem hiding this comment.
When DNS is unavailable or silently drops queries, deleting these fields does not prevent network access because agencyConfig() merges the fixture over defaults that still provide senderEmail: "hello@tinystudio.io". Both send:setup and market:parity therefore perform DNS lookups; the new required CI test can hit its 120-second timeout, leave descendant checker processes running, and even treat the resulting status: null as the expected nonzero verdict. Set the sender fields to explicit empty values so the fixture actually overrides the defaults and remains deterministic.
Useful? React with 👍 / 👎.
| "client:check": "node scripts/check-client-readiness.mjs --strict", | ||
| "client:check:advisory": "node scripts/check-client-readiness.mjs", |
There was a problem hiding this comment.
When a prospect or client is still being prepared, growth-brain/quality/readiness-gates.md tells operators to run these base commands, separately reserves explicit strict invocations for before sending, and states that draft status is acceptable. Injecting --strict here makes that documented draft workflow exit 1, while the new advisory aliases are never mentioned. Update the draft examples to use client:check:advisory and prospect:check:advisory, and remove the now-redundant explicit strict examples.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…le with main after PRs #135/#104/#99/#112/#100/#105) The PR #130 branch had already been reconciled with origin/main through c617364 (PRs #131 + #133/#134/#132/#116). Since then origin/main moved to d4f3ef4 with six more merges (#135 #104 #99 #112 #100 #105). Pull them in so PR #130 can be reviewed against current main without weakening the capture/rollback/reverification implementation or the PR #131 fail-closed missing-secret gate. The merge resolved cleanly: - PR #130 capture/rollback logic (publish-public-site.mjs canonical_deployment, rollbackTo, restored identity re-verify, accept-again) is intact. - PR #130 test file (scripts/test-pages-release.mjs) is intact and wired into the npm ci/test scripts. - PR #131 fail-loud missing-secret workflow step in .github/workflows/deploy-public-site.yml is preserved byte-for-byte. - The new origin/main changes touch operator surfaces and the deploy bundle prep; none of them modify the publish-public-site.mjs rollback contract or the Pages token gate, so there were no conflict markers.
…on main (#130/#132/#133) (#197) Record the decision for the inbox backlog line (tinystudio-in priority=normal AUG13-16). Investigation shows every item is already in main: - truthful-reversible-pages-release: LANDED via PR #130 (ea9eab2); release lane fails truthfully on missing credentials and rolls back bad uploads (scripts/test-pages-release.mjs passes 38/38). - service-backup-restore-roundtrip: LANDED via PR #132 (4a7f442); restore mode in service-state-backup.mjs. - operator-checks-strict: LANDED via PR #133 (c617364); named operator checks fail on blocked readiness with advisory aliases explicitly named. No code change required; this closes the backlog by recording the decision.
1 participant
What
The named readiness/operator npm commands previously ran their check scripts without
--strict, so blocked readiness exited 0 every time — a false-green for operators. This change wires--strictinto each named command so blocked readiness fails loudly with no hidden flag, and preserves report-only behavior under explicitly named:advisoryaliases that required gates never use.Commands made strict by default
client:check→check-client-readiness.mjs --strictprospect:check→check-prospect-readiness.mjs --strictsend:setup→check-outbound-sender-setup.mjs --strictprospect:site-check→check-recording-sites.mjs --strictmarket:parity→check-market-parity-readiness.mjs --strictmarket:proof-check→check-market-proof-run.mjs --strictEach keeps an explicitly named
<command>:advisoryalias running the script without--strict(report only).Regression test
New
scripts/test-operator-check-strictness.mjs, wired into bothnpm run ciandnpm run test, builds blocked fixtures (blocked client, blocked prospect, unreachable site, no proof rows) and asserts:ci/testgates run the detector and never invoke:advisoryaliasesAlso fixed
check-client-readiness.mjsandcheck-prospect-readiness.mjsto parse their positional path argument correctly when--strictis supplied by the npm script.Validation
node scripts/test-operator-check-strictness.mjs→ passednode scripts/test-active-operator-surfaces.mjs→ passednode scripts/test-client-readiness-contract.mjs→ passednode scripts/test-outbound-send-readiness.mjs→ passednode scripts/test-direction-proof-gate.mjs→ passednode scripts/test-service-engine.mjs→ passedclient:check1,prospect:check1,send:setup1,market:parity1,market:proof-check1 (nonzero);prospect:site-check0 (no queued prospects — green state)send:setup:advisory0,prospect:site-check:advisory0,market:parity:advisory0npm run ciandnpm run testreach and pass the new strictness test; the chain stops later atcheck-retention-automation.mjs, which fails identically on pristineorigin/main(pre-existing: requires the canonical main checkout to point at the TinyStudio repo)