The native overlay manifest parse stage refuses four things and none of them are tested

[kstonekuan]

#393 covered the seven refusals in _validate_manifest that one valid-manifest fixture can reach, and #394 landed them. That issue explicitly deferred the rest:

Not in scope: the 20 other CythonOverlayManifestError sites, including the JSON-parsing and canonical-JSON refusals at :638, :749, and the _require_* helpers from :1986 down. Those are worth covering too, and they can follow.

This is that follow-up. These refusals run before _validate_manifest, on the raw bytes of an overlay directory's manifest.json, which is external input.

Verification

Against main at 44e4f50, each refusal neutered in turn, running tests/test_packaging.py:

native overlay manifest is not valid JSON        NOTHING NOTICED
native overlay manifest is not canonical JSON    NOTHING NOTICED
artifacts must be an array                       NOTHING NOTICED
native overlay manifest contains duplicate fields  NOTHING NOTICED

What to do

Cover the parse-stage refusals, in src/hflow/packaging.py:

:638    native overlay manifest is not valid JSON
:675    artifacts must be an array
:749    native overlay manifest is not canonical JSON
:2067   native overlay manifest contains duplicate fields

The canonical-JSON and duplicate-fields ones are the interesting pair. Both exist because a manifest that parses is not necessarily the manifest that was signed: re-serialized JSON with different key order, or a document with a repeated key where json.loads silently keeps the last, would otherwise pass. A test that only feeds it garbage will not reach either.

Pattern to copy

#394 landed the shape at tests/test_packaging.py: build a real overlay with build_cython_overlay, rewrite manifest.json, assert the specific CythonOverlayManifestError, then assert nothing was applied (sources intact, no artifacts, no installed manifest, RECORD byte-identical). The helpers _read_manifest_payload and _write_manifest_payload are already there.

Note that _write_manifest_payload writes with indent=2, sort_keys=True, which is canonical, so a canonical-JSON test needs to write the bytes directly rather than through that helper.

Definition of done

1. Each of the four refusals is the only failure when its own raise is neutered.
2. The canonical-JSON case uses a payload that is valid JSON and semantically identical to the original, differing only in serialization, so it proves the refusal is about form rather than content.
3. The duplicate-fields case uses raw bytes with a repeated key, since no Python dict can express it.
4. Each case asserts nothing was applied, following test(packaging): cover native overlay manifest refusals #394.

Not in scope

The _require_* helpers from :1986 down (must be an object, has unexpected fields, must be an integer, must be positive, must use POSIX separators, must remain within its root, must be a lowercase SHA-256). Sixteen small refusals sharing one shape; they want a parametrized test of their own and would swamp this issue.

Prerequisite

These tests build a real native overlay, so they need a system C compiler (cc). See the development setup in CONTRIBUTING.md; #398 added that prerequisite this afternoon.

Validation

uv sync --locked --all-extras
uv run ruff check
uv run ruff format --check
uv run ty check
uv run pytest -q

[chiruu12]

I'd like to take this one.

Plan is the four parse-stage refusals in tests/test_packaging.py, following the shape #394 landed: build a real overlay, rewrite manifest.json, assert the specific CythonOverlayManifestError, then assert nothing was applied. Noted that the canonical-JSON and duplicate-fields cases need the bytes written directly rather than through _write_manifest_payload, since that helper writes canonically and would defeat both.

I'll check each case by neutering its refusal and confirming the new test fails, so none of them lands as something that passes with the guard gone.

[kstonekuan]

Closed by #419. @chiruu12 this one is on me and I want to be specific about how.

You asked for it at 08:02 with a plan that was correct on every point, including that the canonical-JSON and duplicate-field cases need raw bytes rather than _write_manifest_payload. I never replied. Six hours later I offered the same issue to someone else in a comment on #405, without reading this thread first. @harshitethic had by then already opened a PR for it (#416, at 13:36, auto-closed by the one-open-PR cap while #415 was still under review, then reopened as #419).

So: your claim was first, nobody told you otherwise, and the reason it did not become yours is that I did not do the one thing that reserves an issue here. That is the third time today the same omission has caused this, and repeating "only an assignee reserves an issue" while not assigning anyone is not a policy, it is an excuse. From now on I assign when someone asks, in the same reply.

#420 is the direct follow-up to #419 and it is yours if you want it: two copies of the sample-distribution helper that already disagree in signature. It needs the judgment call your #408 comment showed, namely where shared test fixtures should live in this repo, and there is a real constraint in it (include_record=False has to survive). Say the word and I will assign it before you start, or tell me you would rather have something else and I will find it.

@harshitethic nothing here is a criticism of your PR, which is good work and which you started before any of this got tangled.