Close the version- and platform-gate gaps found by the integrity audit - #4
Merged
Conversation
… audit Each of these is a place where a rule the codebase states plainly was enforced on one path and not on a second. The container format 0.1 path never consulted the runtime's platform allowlist. `decide_restore` gates the pair twice -- once against what the image targets, once against what this runtime accepts -- and its comment says an image cannot reach Windows arm64 "even with every archive checksum recomputed to match". A 0.1 image carries no contract to argue with, so it reached exactly that pair by being older. `legacy_decision` now applies the runtime side first, before the exact-Python and exact-runtime checks, so an unverified pair is reported as such rather than as a version mismatch. `doctor` tested the operating system and the architecture against two literal sets and never tested the pair. It answered "Compatibility: OK" with exit 0 on Windows arm64, a host where every 0.2 restore is refused. It now asks the runtime's own question. Its inline copy of the architecture normalization table is gone; `normalized_architecture` was already imported and already used by `_resume`. The graph codec version was declared twice: in `abi`, where the execution contract advertises it as a capability, and again as a literal in `codec`. They agree today and nothing tied them together, so a bump would have left the codec writing bytes the contract did not describe. `read_plan` validated the entry set, the checksums and two version fields, then returned a plan whose remaining fields were unchecked. `inspect-upgrade` prints fifteen of them by subscript, so a plan the reader accepted produced a KeyError traceback rather than the refusal this module promises everywhere else. The required field set is now checked where the plan is read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ucible The stress preflight accepted any count at or above 160 while the suite was 402. It exists to catch a mistyped module name repeating nothing; a floor that loose also passes a regression dropping more than half of discovery. It now compares against the count docs/TESTING.md publishes, so the two cannot drift apart silently either. The differential harness defaulted to 5 checkpoints while the workflow and every published corpus used 6, so a plain local run could not reproduce the archived evidence. It also recorded only the raw platform.machine(), which spells one ISA `x86_64` on Linux and `AMD64` on Windows, making two runs on the same architecture look like two architectures; cli_proof.py and live_migration/proof.py already record the normalized name. The workdir default was a POSIX path. Both proofs were re-run after these changes and reproduce the committed artifacts exactly: 1,576 migration cases with 0 silent-incorrect and 0 ambiguous, and 204 cross-Python cases with 0 silent mismatches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two documents pointed at an `artifacts/` directory that has never existed here. The skip table listed five skips and omitted the static undefined-name gate, which skips on every host unless pylint is importable. Four documents cited `d328397` as though it were a commit in this repository; it is the merge commit GitHub generates per pull-request run and is not reachable from any branch, so a reader could only conclude the citation was wrong. The suite-size guard covered README, STATUS, TESTING and ROADMAP but not the release notes, which is why the 0.5.0a1 notes had already drifted once. The notes for the release being prepared describe the current suite, so they are now checked. The 0.4.0a1 notes are not: like a completed roadmap milestone, they record what was true when that release shipped. Counts move to 407 with the regression tests added alongside. The two adversarial duplicate-entry fixtures write a name twice on purpose and CPython warns about it. Those were the only warnings in the run log, so a real one would have blended in. The warning is now scoped to the two lines that provoke it; the refusal behaviour they prove is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 9 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 selected for processing (8)
📝 WalkthroughWalkthroughThe PR tightens platform and codec compatibility checks, validates required migration plan fields, updates cross-runtime validation defaults, suppresses expected fixture warnings, and synchronizes test-count and verification documentation. ChangesContinuum validation updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/test_image_refusals.py (1)
494-499: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFilter only the expected duplicate-entry warning.
Line 498 suppresses every
UserWarningraised byarchive.writestrin this scope. A future warning from this write can therefore be hidden. Use a message-specific filter for the expected duplicate-name warning.Proposed fix
with warnings.catch_warnings(): - warnings.simplefilter("ignore", UserWarning) + warnings.filterwarnings( + "ignore", + category=UserWarning, + message=r"Duplicate name: .*", + ) archive.writestr("manifest.json", entries["manifest.json"])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_image_refusals.py` around lines 494 - 499, Update the warnings filter around archive.writestr in the duplicate-entry fixture to suppress only the expected duplicate-name warning by matching its message, while allowing unrelated UserWarning instances to surface.continuum/migration.py (1)
833-839: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd regression coverage for omitted plan fields.
The supplied migration tests do not exercise a plan that omits a member of
PLAN_FIELDS. Verify that coverage exists elsewhere. If it does not, add a test that recomputes the archive checksum and assertsREFUSE_MALFORMED_PLAN.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@continuum/migration.py` around lines 833 - 839, Add regression coverage for the missing-field branch in the plan validation flow around PLAN_FIELDS and MigrationRefused. Create a migration test with one required plan field omitted, recompute the archive checksum, run the migration, and assert that it fails with REFUSE_MALFORMED_PLAN; reuse existing test helpers and verify whether equivalent coverage already exists before adding a new test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/stress.yml:
- Around line 69-77: The documented count capture in the workflow currently
exits under pipefail before the explicit missing-count check can run. Update the
documented assignment near the `documented` variable to use a guarded
no-match-safe capture, and replace the `head` pipeline stage with a
pipefail-safe selection method so multiple matches do not trigger SIGPIPE;
preserve the existing diagnostic and equality validation.
In `@validation/cross_python/differential.py`:
- Around line 789-792: Update the argparse definition for --workdir in the
differential runner so its default includes a unique per-run component,
preventing concurrent invocations from sharing the same directory and
deterministic image files. Preserve explicitly supplied --workdir values and
keep the existing temporary-directory base.
---
Nitpick comments:
In `@continuum/migration.py`:
- Around line 833-839: Add regression coverage for the missing-field branch in
the plan validation flow around PLAN_FIELDS and MigrationRefused. Create a
migration test with one required plan field omitted, recompute the archive
checksum, run the migration, and assert that it fails with
REFUSE_MALFORMED_PLAN; reuse existing test helpers and verify whether equivalent
coverage already exists before adding a new test.
In `@tests/test_image_refusals.py`:
- Around line 494-499: Update the warnings filter around archive.writestr in the
duplicate-entry fixture to suppress only the expected duplicate-name warning by
matching its message, while allowing unrelated UserWarning instances to surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 373001c8-c71c-4bc5-b8fa-9a9acff50446
📒 Files selected for processing (17)
.github/workflows/stress.ymlAUDIT.mdPORTABILITY.mdREADME.mdSTATUS.mdcontinuum/abi.pycontinuum/cli.pycontinuum/codec.pycontinuum/migration.pydocs/RELEASE_NOTES_0.5.0a1.mddocs/TESTING.mdtests/test_cli.pytests/test_documentation_consistency.pytests/test_execution_abi.pytests/test_image_refusals.pytests/test_migration.pyvalidation/cross_python/differential.py
All three came from the CodeRabbit review on #4. Each was reproduced before being changed. The discovery gate could not report why it failed. Under `set -Eeuo pipefail` a no-match `grep` aborts the step at the assignment, so the explicit "docs/TESTING.md states no count" diagnostic on the next line was unreachable -- the job died with no explanation, in the one gate whose whole purpose is explaining a discovery drift. Reproduced directly: $ printf 'no count here\n' > f.md $ bash -c 'set -Eeuo pipefail d=$(grep -oE "[0-9]{2,4} tests" f.md | head -1 | cut -d" " -f1) test -n "$d" || { echo GUARD; exit 1; }' (no output, exit 1) Captured with `|| documented=""` so the guard is reached, and `sed -n 1p` replaces `head -1` so a second match cannot SIGPIPE the producer under pipefail. Both paths verified: the missing-count case now prints its diagnostic, and a two-match file still yields the first count. The duplicate-entry fixtures suppressed every UserWarning. Scoping by category alone would also swallow an unrelated future warning raised by that same `writestr`, which inverts the intent of scoping it at all. Now matched on the `Duplicate name: ` message. Both suites still refuse the image and the run log still carries zero UserWarnings. The differential harness defaulted every local run to one shared work directory while writing deterministic `{stem}-{safe_point}.cont` image names, so two concurrent local runs would overwrite and delete each other's images. Default is now per-process. CI is unaffected: `cross-python-cli-proof.yml` passes `--workdir "$RUNNER_TEMP/differential"` explicitly. 407 tests, 0 failures, 5 skips, 0 warnings on CPython 3.12.13 and 3.13.14. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review on #4 found that this branch added a `PLAN_FIELDS` requirement to `read_plan` without a test that omits one. A refusal branch no test reaches is indistinguishable from a refusal branch that does not work, which is precisely the class of gap the audit this branch came from was looking for. Drops each of the 17 names in turn rather than one representative field: a tuple of required names is only a contract if every entry is enforced. The archive checksum is recomputed for each variant, so the refusal is attributable to the field check and not to a mismatched digest, and the omitted field name is asserted to appear in the message. Confirmed the test detects the absence of what it guards. With the `missing` computation in `read_plan` neutralized, it fails 17 times, once per field. The other review comment on this push was already addressed: it flagged the blanket `simplefilter("ignore", UserWarning)` in `test_image_refusals.py`, which b2a7982 had already narrowed to the `Duplicate name: ` message. The review was raised against b6014b5. Published counts 407 -> 408. 408 tests, 0 failures, 5 skips on CPython 3.12.13 and 3.13.14. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes from a repository-integrity and conflict audit of the tree at
dbc032f. No feature work: every change closes a gap between what a document or gate claims and what the code actually does.Correctness
Legacy images could reach a platform this runtime refuses.
abi.legacy_decisionapplied only the image's declared target set, so a container-format 0.1 image restored on a pair absent fromabi.VERIFIED_PLATFORMS, while the same program at format 0.2 was refused withunsupported-platform. The runtime's own allowlist now runs first, before either exact check — an older container format is a reason to be stricter, never a way to reach an unverified pair.doctoraccepted a pair the runtime rejects. It checked the OS and architecture axes independently, which accepts the whole 3x2 product; Windows arm64 satisfies both axes and is still refused at restore.doctornow asks the same pair question the runtime asks.The graph codec version was written in two places.
codec.pycarried its own literal alongsideabi.GRAPH_CODEC_VERSION, so a bump in one would let the encoder stamp a version the decoder rejects. Both sites now read the ABI constant. (Verified no import cycle:abiimports only.and.errors.)read_planaccepted a structurally incomplete plan. Aplan.jsonmissing required fields passed validation and failed later, further from the cause. It is now checked againstPLAN_FIELDS, derived from a really generated plan rather than from the writer's source.CI and harness
-ge 160against a 402-test suite — a regression dropping half of discovery would have passed the gate that exists to catch exactly that. It now requires exact equality with the count published indocs/TESTING.md.--checkpointsdefault (5) disagreed with the value CI passes (6), so the committed artifact could not be reproduced with default flags. Both fixed;--workdiralso no longer defaults to a POSIX-only path.Documentation and test hygiene
artifacts/tree that is not in this repository.d328397citations now say what that hash is: a pull-request merge commit generated per run, not reachable from any branch here.docs/RELEASE_NOTES_0.5.0a1.mdis now covered by the count guard (shipped release notes stay excluded as historical records); the pylint-conditional skip is documented.UserWarning. Refusal behaviour is unchanged — both readers still reject duplicates before any last-wins comprehension — but the run log is clean, so a real warning cannot hide in the noise.Verification
407 tests, 0 failures, 5 skips, 0 warnings on CPython 3.12.13 and 3.13.14. No test was weakened to pass; the 5 new regression tests fail against the parent commit. Both headline proofs still reproduce the committed artifacts exactly — migration matrix 590/26/959/1 = 1,576 with 0 silent-incorrect and 0 ambiguous, cross-Python 189/8/6/1 = 204 with 0 silent mismatches — the latter now with default flags. The merged tree was run green at 407/407 before this PR was opened.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation