Give each commit attempt a unique telemetry ID - #286
Conversation
|
Warning Review limit reached
Next review available in: 56 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: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (10)
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 |
…371) ## What `devkit ship` printed six biome findings into the gate log and passed the commit anyway (the v0.50.0 ship). The deterministic runner was working correctly — `biome check .` exits **0** when every diagnostic is warn-severity, and `--extra "lint=bun run lint"` is only ever as hard as the script it names. devkit’s own `lint` script is now `biome check --error-on-warnings .`, and the six pre-existing warnings are cleared so the gate starts green. ## The six that were passing | file | rule | | --- | --- | | `cli/__tests__/husky-block.test.mts:418,440` | `noTemplateCurlyInString` | | `cli/__tests__/self-host.test.mts:147` | `noTemplateCurlyInString` | | `gate-engine/coverage/__tests__/produce.test.mts:123` | `noNonNullAssertion` | | `gate-engine/decisions/__tests__/recall-scoring.test.mts:255` | `noNonNullAssertion` | | `skills/correctness/scripts/checklist.mjs:65` | `useOptionalChain` | The three `noTemplateCurlyInString` sites assert literal POSIX-sh `${VAR:-default}` text — suppressed with the same `biome-ignore` line this repo already uses at six other sites. The two non-null assertions are **removed** rather than suppressed: one becomes a real precondition check, the other hoists the fixture literal so no assertion is needed. The optional chain is the rule’s own fix. ## Scope: devkit’s own gate only The consumer emitter (`cli/lib/install/package-json.mts`) and the overlay biome gate (`cli/lib/husky/husky-block.mts`) keep a bare `biome check` on purpose. `biome/base.jsonc` holds `noConsole` at `"warn"` deliberately for consumers, while devkit’s root config turns it off across its whole authored surface — so the flag costs devkit nothing and would cost a consumer every `console.log`. Making consumers strict is a separate decision. ## Regression test `cli/__tests__/self-host.test.mts` now asserts the repo’s real `package.json` lint script carries `--error-on-warnings`, because the strictness lives in a script string that no hook-text assertion can reach. ## Also in the diff - `docs/decisions/gate-opt-out-is-visible-and-detectable.md` — note recording the reverse failure mode: not a gate that silently opts out, but one that prints its findings and exits 0. - `.devkit/skills-manifest.json` — `devkit sync-skills` refreshed the correctness-checklist hash and bumped a stale `devkitRef` v0.49.1 → v0.50.0. ## Deliberately NOT in the diff `cli/lib/husky/husky-block.mts:35` still documents the gate as `biome check .`, which is now stale by one line. Correcting that comment was in the first attempt and the ship blocked: the file is **560 lines against a 510-line size baseline on `origin/main`** (`eslint/baselines/size-lines.json:7`), so `guard-size` rejects any commit that touches it. That debt is pre-existing — it arrived via #360, #348 and #286 without a baseline refresh — and paying it (a file split, or a `guard-size freeze` that launders 50 lines) does not belong in this PR. The load-bearing documentation of the command lives in `cli/lib/husky/self-host.mts`, next to `SELF_HOST_EXTRAS`, and that one is updated. ## Verification `biome check --error-on-warnings .` → 0 · `tsc -p tsconfig.json --noEmit` → 0 · `eslint cli gate-engine` → 0 · `vitest run` → 3681 passed / 1 failed, the failure being `cli/__tests__/review.test.mts` “preserves timeout exit 124” returning 143 (SIGTERM) — a wall-clock-bounded test losing its 1s race under a loaded machine, unrelated to this diff.
What changed
DEVKIT_COMMIT_IDfor every pre-commit attempt instead of using the staged tree hash as the run identitycommit-msghookcommit_treeas correlation metadata while keepingship_idunique per retrydevkit_versionattribution on the rebased run envelopedistartifacts, self-host hook output, size baseline updates, and regression coverageWhy
Plain commit telemetry used
commit-<git write-tree>as its run ID. Retrying an unchanged staged tree therefore reused the same ID, causing the collector to merge separate commit attempts and understate successful commit duration/attempt counts. A staged tree identifies content, not an execution attempt.Each hook execution now owns a unique attempt ID. The tree hash remains available as
commit_treefor comparing identical content without collapsing separate attempts.Validation
Verification bypass
The commit and push used
--no-verifywith explicit maintainer approval. The correctness reviewer hung twice without producing a verdict (approximately eight and six minutes) after the checks above had passed. This known reviewer reliability issue is tracked in autonomous report80f0e222-60e2-4df3-82a8-7cae857a5282/ Shortcut SC-1317. No failing correctness verdict was bypassed.