Skip to content

Expose portable Python requirement validation - #88

Open
omry wants to merge 1 commit into
pr86from
pr87
Open

Expose portable Python requirement validation#88
omry wants to merge 1 commit into
pr86from
pr87

Conversation

@omry

@omry omry commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Add canonical direct-root requirement validation and normalized distribution-name extraction to the Python provider. Direct URLs, environment markers, embedded whitespace, extras, and non-canonical specifier sets are rejected so an immutable catalog root cannot depend on external location or runtime state.

Add canonical interpreter-version validation for the major.minor and major.minor.patch forms used by portable binding compatibility lists, rejecting non-numeric, non-canonical, and out-of-range components.

Scope authority

Task: PTD-01 — Expose Portable Python Requirement Validation, defined in docs/PORTABLE_TOOL_DEFINITION_IMPLEMENTATION_PLAN.md (PR #86 in this stack), under the normative Portable Tool Definition Design.

Acceptance criteria:

  • exact and ranged roots normalize deterministically;
  • URLs, markers, extras, malformed roots, and ambiguous versions fail;
  • go.mod remains stable under go mod tidy;
  • go test ./internal/providers/python passes.

Local review findings, applied

  • extras are rejected outright. The local review first made them canonical by requiring sorted unique ordering, which still contradicted this task's acceptance criterion that roots containing extras must fail; remote review caught that, and a root selecting optional dependency groups is not an exact immutable coordinate;
  • ValidatePackageRootRequirementV1 was removed. It was exported but called by nothing here, at the stack tip, or in either retired source;
  • limit and identity tests were added alongside the existing positive and negative coverage.

Exclusions: record types, catalog behavior, Python resolver changes, and module dependency promotion, which belongs to PTD-04. The two remaining exports have no in-tree caller until PTD-04 consumes them. Source: parked extraction source b39985d247e5 from retired PR #83.

@omry
omry changed the base branch from main to pr86 August 17, 2026 15:55
@omry
omry marked this pull request as ready for review August 17, 2026 15:56
@omry

omry commented Aug 17, 2026

Copy link
Copy Markdown
Owner Author

@codex review 192789f

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for security reviews. Please try again later.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 192789f925

ℹ️ 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".

Comment thread internal/providers/python/package_request.go
@omry
omry marked this pull request as draft August 17, 2026 16:11
@omry

omry commented Aug 17, 2026

Copy link
Copy Markdown
Owner Author

Returning to draft: remote review for this slice is deferred. The stack PTD-01 through PTD-11 is being built with local review and full local checks only, then reviewed bottom-up once review capacity is available.

@omry

omry commented Aug 18, 2026

Copy link
Copy Markdown
Owner Author

PR-cycle state — maintained automatically. Do not edit.

{
  "approval": {
    "approved_revision_fingerprint": "sha256:7b91d58b0363881670a07a4d9cba3ecc8d8925ca16ed986cf4c380b766b76be1",
    "evidence": {
      "delivery_deferral_ledger_digest": "sha256:4f53cda18c2baa0c0354bb5f9a3ecbe5ed12ab4d8e11ba873c2f11161202b945",
      "required_checks": [
        [
          "Linux CI checks",
          "success"
        ],
        [
          "Target smoke (darwin-amd64)",
          "success"
        ],
        [
          "Target smoke (darwin-arm64)",
          "success"
        ],
        [
          "Target smoke (linux-amd64)",
          "success"
        ],
        [
          "Target smoke (linux-arm64)",
          "success"
        ],
        [
          "Windows host smoke (windows-amd64)",
          "success"
        ],
        [
          "Windows host smoke (windows-arm64)",
          "success"
        ]
      ],
      "review_request": {
        "body_digest": "sha256:e80a6b217a42e7878d73afb02459dc8e32dc73354642c3405b1307257f8cb909",
        "id": 5326930387
      },
      "review_result": {
        "body_digest": "sha256:970d1c76c3f0c170651273976561998f48ead5dfeda68b83623d045ace6a35b9",
        "id": 5326952263
      },
      "revision_fingerprint": "sha256:7b91d58b0363881670a07a4d9cba3ecc8d8925ca16ed986cf4c380b766b76be1",
      "thread_dispositions": {
        "resolved": 1,
        "total": 1
      }
    },
    "evidence_fingerprint": "sha256:65edae080837d3f61ae178377f961b0fbd7ed085afd3aeca2fa32bd98aead531",
    "rounds": 2
  },
  "candidate_revision_fingerprint": "sha256:7b91d58b0363881670a07a4d9cba3ecc8d8925ca16ed986cf4c380b766b76be1",
  "checks": {
    "conclusion": "success",
    "count": 7
  },
  "delivery_deferrals": [],
  "findings": [
    {
      "classification": "current-slice defect",
      "disposition": "fixed in 30e47213b9be; thread resolved",
      "evidence": [
        "the normative design is silent on extras in package roots",
        "the plan's PTD-01 acceptance, approved at PR 86 head 0eb14331b9fa, requires extras to fail",
        "no current binding requirement uses extras: greenlet>=3.1.1,<4.0.0, playwright==1.61.0, pyee>=13,<14"
      ],
      "id": "reject-extras-from-portable-package-roots",
      "location": "internal/providers/python/package_request.go:87",
      "named_invariant": "PTD-01 acceptance: URLs, markers, extras, malformed roots, and ambiguous versions fail",
      "note": "this session's local review added sorted-unique extras canonicality, which moved further from the acceptance criterion instead of satisfying it",
      "severity": "P2",
      "source_head": "4b37074779bde6c1eb6edaac0bc36fa486b66295",
      "source_thread_created": "2026-08-17T15:58:25Z",
      "thread_state": "unresolved, not outdated"
    }
  ],
  "pull_request": {
    "base": {
      "ref": "pr86",
      "sha": "0eb14331b9fa58b7e4c62d3e46ef942ebaac470d"
    },
    "head": {
      "ref": "pr87",
      "sha": "30e47213b9be0046677f5e2b67c7b53c76fcba0a"
    },
    "number": 88,
    "repository": "github.com/omry/reploy"
  },
  "review": {
    "request": "@codex, regular review 30e47213b9be0046677f5e2b67c7b53c76fcba0a",
    "result": "Codex Review: Didn't find any major issues \u2014 reviewed commit 30e47213b9"
  },
  "revision_fingerprint": "sha256:7b91d58b0363881670a07a4d9cba3ecc8d8925ca16ed986cf4c380b766b76be1",
  "rounds": [
    {
      "applied_fix_footprint": "internal/providers/python/package_request.go (-24/+5), package_request_test.go (extras moved to negative cases)",
      "attempt": 1,
      "checks": "focused tests, nox -s go-test, gofmt, go vet, go mod tidy stability \u2014 all pass",
      "disposition_changes": "1 thread resolved as verified addressed",
      "effective_diff_digest": "sha256:c3b3d2d6a4f0e522",
      "finding_identities": [
        "reject-extras-from-portable-package-roots (thread PRRT_kwDOTFQCkM6Z01YV)"
      ],
      "named_invariants": [
        "PTD-01 acceptance: roots containing extras must fail"
      ],
      "proposed_fix_footprint": "reject bracketed extras in PackageRootDistributionNameV1",
      "resulting_head": "30e47213b9be0046677f5e2b67c7b53c76fcba0a",
      "reviewed_head": "4b37074779bde6c1eb6edaac0bc36fa486b66295",
      "root_cause": "the slice validated and accepted extras; the local review had made them canonical rather than rejected"
    },
    {
      "applied_fix_footprint": "none",
      "attempt": 2,
      "disposition_changes": "none; the two inline messages are the round-1 thread, already resolved",
      "finding_identities": [],
      "named_invariants": [],
      "note": "round 2 was required only because the round-1 fix moved the head, leaving the clean result attributable to the pre-fix commit",
      "proposed_fix_footprint": "none",
      "resulting_head": "30e47213b9be0046677f5e2b67c7b53c76fcba0a",
      "reviewed_head": "30e47213b9be0046677f5e2b67c7b53c76fcba0a",
      "root_cause": "none \u2014 clean review on the post-fix head"
    }
  ],
  "scope_authority": {
    "docs/PORTABLE_TOOL_DEFINITION_DESIGN.md": "sha256:a7acc70b0b1e390cd4d7d0e60b0f30e6a17500f6a12828195d4b5e8d083ff536",
    "docs/PORTABLE_TOOL_DEFINITION_IMPLEMENTATION_PLAN.md": "sha256:900a01621dacdffd2f391a366a1542bc9f054528bb2175cd6683e5ec839b401a"
  },
  "version": 6
}

@omry
omry marked this pull request as ready for review August 18, 2026 09:16
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for security reviews. Please try again later.

@omry

omry commented Aug 18, 2026

Copy link
Copy Markdown
Owner Author

@codex, regular review 4b37074

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 4b37074779

ℹ️ 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".

Add canonical direct-root requirement validation and normalized distribution-name extraction to the Python provider. Direct URLs, environment markers, embedded whitespace, malformed extras, and non-canonical specifier sets are rejected so an immutable catalog root cannot depend on external location or runtime state.

Add canonical interpreter-version validation for the major.minor and major.minor.patch forms used by portable binding compatibility lists, rejecting non-numeric, non-canonical, and out-of-range components.

These helpers are exposed for the portable tool catalog slices that follow and change no existing provider behavior. Delivers PTD-01 of docs/PORTABLE_TOOL_DEFINITION_IMPLEMENTATION_PLAN.md.
@omry

omry commented Aug 18, 2026

Copy link
Copy Markdown
Owner Author

@codex, regular review 30e4721

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 30e47213b9

ℹ️ 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".

@omry

omry commented Aug 18, 2026

Copy link
Copy Markdown
Owner Author

Slice-ownership accounting, recorded retroactively.

The parked source b39985d247e5 defines ValidatePackageRootRequirementV1, which this slice did not land. That drop was correct but was never recorded, and the Slice Ownership Rule requires that no source hunk be silently dropped.

The function is a one-line wrapper that calls PackageRootDistributionNameV1 and discards the distribution name, returning only the error. Its only consumers in the parked commit are its own tests; no production code in that commit calls it. Every landed caller needs the name, so they call PackageRootDistributionNameV1 directly and the wrapper adds nothing but an exported surface with no user.

Recorded as an exclusion rather than an omission: dead export, no production caller in the parked source, superseded by direct use of the function it wrapped.

This is the only unrecorded drop in the PR 83 parked source. Cumulative accounting for that source now stands at 142 symbols, 140 landed, 2 removed with recorded rationale, the other being validateReferenceUnderV1 which was deleted during PTD-04 review once the reference-shape sweep left it with no callers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved PR reviewed and approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant