Skip to content

Add bounded strict portable record decoding - #90

Open
omry wants to merge 1 commit into
pr89from
pr90
Open

Add bounded strict portable record decoding#90
omry wants to merge 1 commit into
pr89from
pr90

Conversation

@omry

@omry omry commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Add schema dispatch and bounded exact JSON decoding for every v1 record family, rejecting duplicate members, unknown and case-variant fields, missing required fields, JSON numbers and nulls, invalid UTF-8, unpaired surrogates, trailing tokens, and payloads past the file, depth, member, and string limits.

Add the canonical value rules the model depends on: record IDs and references, positive decimals, relative and absolute record paths, credential-free HTTPS source URLs, reversible percent-encoded tool version segments, and bounded sorted unique collections.

Scope authority

Task: PTD-03 — Add Bounded Strict Portable Record Decoding, defined in docs/PORTABLE_TOOL_DEFINITION_IMPLEMENTATION_PLAN.md (PR #86 in this stack), under the normative Portable Tool Definition Design.

Acceptance criteria:

  • every schema round-trips;
  • ambiguous encodings and every structural limit have negative coverage;
  • decoding performs no graph or network work.

Sequencing note

In the parked source, decodeRecordV1 called record-local validation and decodeValidationEvidenceV1 called evidence validation. Those validators are owned by PTD-04 and PTD-06, and this slice's non-goals exclude record business rules, so decoding does not call them here — the slices that own those validators wire them in. One assertion in the evidence decode test depends on that wiring and moves with it, as does the reference-collection limit test that exercised it through record validation; this slice covers the same bound directly against validateReferenceListV1.

Exclusions: record business rules, cross-record validation, graph traversal, resolution. Source: parked extraction source b39985d247e5.

@omry
omry changed the base branch from main to pr89 August 17, 2026 16:24
@omry
omry force-pushed the pr90 branch 2 times, most recently from 2626cdd to 26a59fa Compare August 17, 2026 19:31
@omry
omry force-pushed the pr90 branch 2 times, most recently from 20d78fa to 532805a Compare August 17, 2026 21:33
@omry
omry force-pushed the pr90 branch 2 times, most recently from 3a8ea63 to b68d766 Compare August 18, 2026 05:49
@omry
omry force-pushed the pr90 branch 2 times, most recently from 5a8d771 to ddbd2e2 Compare August 18, 2026 06:01
@omry
omry force-pushed the pr90 branch 2 times, most recently from 46e80e7 to 438d7bd Compare August 18, 2026 10:22
@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:d7a8ca6fb99451ef042bf4094699a7067e74a0663848fcd0c44a1af59807ebde",
    "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:84b8dff8f036f7ad76ef94edcb8d31ad42a9a215594b3f11d9e086df2e77e725",
        "id": 5328395368
      },
      "review_result": {
        "body_digest": "sha256:baca04ea444f0b30cb149427ea758c1c99806cd9774b909e44825181add2a85b",
        "id": 5328444278
      },
      "revision_fingerprint": "sha256:d7a8ca6fb99451ef042bf4094699a7067e74a0663848fcd0c44a1af59807ebde",
      "thread_dispositions": {
        "resolved": 1,
        "total": 1
      }
    },
    "evidence_fingerprint": "sha256:a3c9f2d35abbf6521d4fab0cc206b04e56a3f89af4c3f5b9a270f414af641838",
    "rounds": 2
  },
  "checks": {
    "conclusion": "success",
    "count": 7
  },
  "delivery_deferrals": [],
  "findings": [
    {
      "classification": "current-slice defect",
      "disposition": "fixed in 42c6f5fa01b9; also applied to validateAbsoluteRecordPathV1, which had the same gap and was not flagged",
      "id": "reject-backslashes-in-record-paths",
      "location": "internal/toolcatalog/records_decode.go:593",
      "named_invariant": "portable record paths must match their consumer's rule in providerstore.ArtifactDescriptor.Validate, which forbids backslashes",
      "note": "the preflight design-to-code audit did not catch this: the inconsistency is between a validator and its Go consumer, not between code and design",
      "severity": "P2"
    }
  ],
  "preflight_audit": "design decoding rules diffed against implementation before review: reject invalid UTF-8, duplicate member names, and schema-unrepresentable values before semantic decoding, plus lowercase-hex rejection in version segments \u2014 all implemented; aggregate byte, record-count, and graph-depth limits belong to PTD-07 and closure contributions to PTD-09 per the plan",
  "pull_request": {
    "base": {
      "ref": "pr89",
      "sha": "97ae8d8f7daf852c5ba5f1406bad93d12e00de39"
    },
    "head": {
      "ref": "pr90",
      "sha": "42c6f5fa01b9"
    },
    "number": 90,
    "repository": "github.com/omry/reploy"
  },
  "review": {
    "request": "@codex, regular review 42c6f5fa01b9",
    "result": "Codex Review \u2014 no issues on 42c6f5fa01"
  },
  "revision_fingerprint": "sha256:d7a8ca6fb99451ef042bf4094699a7067e74a0663848fcd0c44a1af59807ebde",
  "rounds": [
    {
      "attempt": 1,
      "finding": "backslashes accepted in record paths",
      "resulting_head": "42c6f5fa01b9",
      "reviewed_head": "7e823b69d2fe"
    },
    {
      "attempt": 2,
      "finding": "none \u2014 clean",
      "resulting_head": "42c6f5fa01b9",
      "reviewed_head": "42c6f5fa01b9"
    }
  ],
  "scope_authority": {
    "docs/PORTABLE_TOOL_DEFINITION_DESIGN.md": "sha256:4c8fa535721baf22f67486a46b0b4a04eb7054713d0a892e2d6f9667bba914b6",
    "docs/PORTABLE_TOOL_DEFINITION_IMPLEMENTATION_PLAN.md": "sha256:971113afcedea93da5df3fab42433a1e5f9fea62554f02a3f630e306b9455f00"
  },
  "version": 2
}

@omry
omry marked this pull request as ready for review August 18, 2026 12:43
@omry

omry commented Aug 18, 2026

Copy link
Copy Markdown
Owner Author

@codex, regular review 7e823b6

@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: 7e823b69d2

ℹ️ 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/toolcatalog/records_decode.go Outdated
Add schema dispatch and bounded exact JSON decoding for every v1 record family, rejecting duplicate members, unknown and case-variant fields, missing required fields, JSON numbers and nulls, invalid UTF-8, unpaired surrogates, trailing tokens, and payloads past the file, depth, member, and string limits.

Add the canonical value rules the model depends on: record IDs and references, positive decimals, relative and absolute record paths, credential-free HTTPS source URLs, reversible percent-encoded tool version segments, and bounded sorted unique collections.

Decoding stays free of record business rules, cross-record validation, graph traversal, and network access. Record-local validation and external-evidence validation are wired into decoding by the slices that own them, so decodeRecordV1 and decodeValidationEvidenceV1 do not call them here.

Delivers PTD-03 of docs/PORTABLE_TOOL_DEFINITION_IMPLEMENTATION_PLAN.md.
@omry

omry commented Aug 18, 2026

Copy link
Copy Markdown
Owner Author

@codex, regular review 42c6f5f

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 42c6f5fa01

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

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