Skip to content

T002e: plan 038's fifth bookkeeping batch, the T071 tick, the NUL seam and the T041 NAMES rulings (plan 038) - #1283

Draft
brettheap wants to merge 12 commits into
mainfrom
r2/t002e-bookkeeping-5
Draft

brettheap wants to merge 12 commits into
mainfrom
r2/t002e-bookkeeping-5

Conversation

@brettheap

@brettheap brettheap commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

T002e: plan 038's fifth bookkeeping batch (plan 038)

Bookkeeping: no Arc: line, and no openspec/changes/ path. It encodes rulings already posted on #656 and a landing already merged, each cited by comment id or merge commit, with no new decision. It follows #1279 (e514f9e0, batch 4) in form and redoes nothing #1279 did. Claim: #656 6072906489. The branch is cut from origin/main 93d13d6c.

Realizes / Falsifier: none (bookkeeping). The gates are listed under "Gates" below.

What changed (specs/038-opendox-document-tool-self-maintenance/)

1. The ticks of T071 and T015. Each merge commit was read through GraphQL (pullRequest{mergeCommit{oid}}) before it was written.

2. The NUL seam (the holder, 6072086385 item 4). Postgres jsonb cannot store U+0000 (SQLSTATE 22P05).

  • T041 refuses U+0000 in every string a finding carries, keys and values alike (identity, evidence, locator, message), with FindingRefused and a bounded where. For a pack's output, that refusal is T045's whole-output pack-output-refused. Recorded in T041's Implementation note, with 6072086385 (items 1 and 4) in its Ruled line.
  • T042's _json_text also refuses it, as RefusedError and never a raw driver error, with a test (defence in depth). A new Implementation note, and 6072086385 (item 4) in its Ruled line.
  • T044's families never emit U+0000: a document string that contains it takes the SHA-256 form under a distinct key (6069507373, T044 item 7), never truncation or silent replacement. In T044's text, and in its Ruled line.
  • contracts/health-finding.md gains the bound: the identity, locator, message and evidence rows each carry it, and a Rules bullet states it in full.
  • The character is written "U+0000" everywhere, never as a backslash escape.

3. NAMES (b), (c) and (e) (the holder, 6072197564, on lane 3's REVIEW-W1 T041 NAMES points), and the holder's refinement of (e), 6073087924 (02:34Z, added by the holder at 02:38Z while this PR was a draft). In T041's Implementation note and Ruled line, and in a new Rules bullet and an id-rule pointer in contracts/health-finding.md:

  • (b) a pathless finding's identity.category is from a CLOSED set: an engine category or identity-collision. A pathless original's re-raise under option (D) takes the original's kind, which is itself one of those. Packs never raise pathless findings (T045 refuses an empty-path pack finding, 6072086385 item 3).
  • (c) a pathless entry is bounded at 200 characters, not 40. A per-entry category's entry is also its pack_id, so the 40-character name rule binds it; only entry-refused carries an id that may be malformed, and its form is fixed under item 4 below.
  • (e) no-sandbox, manifest-refused and dispositions-refused take entry "", and a non-empty entry is refused. A run raises ONE no-sandbox finding, never one per entry, as data-model.md § Sandbox probe and canary says ("ONE install-level finding", R2Q16 (a)).
  • (e) refined, 6073087924 (on lane 3's REVIEW-W1 T041 MINOR at health_contract.py:693): a pathless identity's entry is fixed by its category, exactly as the engine builds it.
    • The seven per-entry categories (fetch-failed, digest-mismatch, declaration-refused, pack-crashed, pack-timed-out, pack-bound-hit, pack-output-refused) REQUIRE a non-empty entry, the valid entry's id (6072086385 item 1).
    • no-sandbox, manifest-refused, dispositions-refused and identity-collision REQUIRE entry "".
    • entry-refused takes either "" or a non-empty entry, in the form fixed under item 4 below.
    • The check refuses any other form, so it admits exactly what the engine builds, and T041 adds one test row per case.

4. The rulings on this PR's own review threads (the holder, 6086098003 items 1, 3 and 4, 6088484643 item 5, 6088732352 item 6 with 6082100803, and 6089449884 item 7, 6090537166 item 8, and 6103336585 item 9). In T041's and T042's notes and Ruled lines, and in contracts/health-finding.md:

  • Item 3, path and U+0000 (Codex P1 4230671177). 6072086385 item 4's principle, "every string a finding carries", covers path, and T041 already refuses it (_path refuses C0 and C1 controls and lone surrogates, as the pinned schema does).
    • The contract's path row states the bound, and its U+0000 bullet names path.
    • T041's note records the test row for U+0000 in path, if it lacks one.
    • T042's note records that the store refuses U+0000 in every text parameter it binds, path among them, as RefusedError naming the column, before any statement, with one test row and its mutant.
  • Item 4, entry-refused's identity.entry (Copilot 4230698859). The pinned schema wins, so "the refused entry as written" holds only where the entry conforms. The form is "" for an entry with no id; the id as written when it is a string matching [a-z0-9-]+ of at most 200 characters; otherwise sha256- followed by the SHA-256 hex of the id's canonical text, never truncated and never a schema exception. In T041's note and in the contract. The earlier "as written" wording in (c) and in the 6073087924 bullet is narrowed to point at it.
  • Item 5 (6088484643), the canonical text (Copilot 4232974858). It is Python's json.dumps(id, sort_keys=True, separators=(",", ":"), ensure_ascii=True, allow_nan=True, default=str), falling back to ascii(id) if that raises (item 6 (b) below changes the fallback from repr(id)), and identity.entry is sha256- plus the lowercase SHA-256 hex of the text's ASCII bytes (71 characters). In the contract and in T041's note, with a test row for a mapping id with mixed-type keys.
  • Item 6 (6088732352), on Copilot's three items at 163ed13c.
    • (a) Thread 4234200786, the date-versus-string collision: an ACCEPTED LIMIT under 5988818366. With default=str, a YAML date or timestamp id digests like a malformed string id that spells its str(); a conforming string such as 2026-10-09 is kept as written, so it never collides. It is operator-only and never silent (two findings with one identity raise identity-collision). Stated in the contract's entry-refused text, and noted in T041's note. The helper's "distinct for distinct ids" is qualified by it (Copilot 4234624544, fixed in 60b8a46e).
    • (b) Thread 4234200849, a non-ASCII repr(): FIX. The fallback is ascii(id), so the canonical text is always ASCII and the digest never raises. T041 adds a test row with a non-ASCII key in a mixed-type mapping.
    • (c) 6082100803, which Copilot listed as not encoded: ENCODED. For ENTRY_CATEGORIES, check_finding refuses an entry that is empty, the reserved opendox, or not a 1-to-40-character name, with one test row each. It corrects 6072197564 (c)'s premise: the 40-character rule does not bind through make_finding for a re-raise, whose pack_id is opendox. In the contract and in T041's note, with lane 3's exactness probe (21 mismatches before, 0 required after) and the mutant that drops each new refusal.
  • Item 1, refused-patch. It joins ENGINE_KINDS, a PATHED engine kind and not a category, in T041's note, with one test row. T045's refusal of a declaration naming it is T045's own test.
  • Item 7 (6089449884), the limit widened. The same class as item 6 (a), from Copilot's thread 4234624749 on Archive add-register-edit-lane: first unattended fulfilment observed #97 (T041): the canonical text is not injective across YAML value types. A date or timestamp and its str(), and a non-string mapping key and its JSON spelling ({1: "a"} and {"1": "a"}, True and "true", None and "null"), can share a digest; identity-collision reports any such collision. Encoded with item 6 (a), in the contract's accepted-limit text and in T041's note.
  • Item 8 (6090537166), the second fallback and sets (lane 3's REVIEW-W1 T041 NO-GO at Archive add-register-edit-lane: first unattended fulfilment observed #97, and Copilot 4235176471). In the contract's entry-refused text and in T041's note, with the test rows.
    • (a) The canonical text's second fallback catches ValueError as well as RecursionError (an integer past Python's int-digit limit, such as 0x plus 4000 fs, makes json.dumps and then ascii() raise). An id neither spelling can write takes ONE fixed text, <past the int-digit or recursion limit>. Two such ids share a digest: an accepted limit that identity-collision reports.
    • (b) The default hook spells a set or frozenset as the list of its members sorted by each member's ascii() text, so its digest does not vary with PYTHONHASHSEED. A set reached only through the ascii() fallback keeps Python's order, an accepted limit (at worst that refused entry's finding is raised again each run).
  • Item 9 (6103336585), NIT 2 of lane 3's GO at Archive add-register-edit-lane: first unattended fulfilment observed #97. The int-digit limit is an interpreter setting (PYTHONINTMAXSTRDIGITS), not a constant, so the same huge-integer id can take the fixed text under one setting and be spelled out under another, giving a different digest: an accepted limit, operator-only, the same class as item 8 (b)'s. Stated in the contract's entry-refused text beside item 8's two limits, and in T041's note. NIT 1 (a test row in T041's code) needs nothing from this batch.
  • The canonical call's hook (Copilot 4239576494): the call is written default=<the default hook>, which is str except for sets and frozensets (item 8 (b)), in the contract and in T041's note, so the call no longer names default=str.
  • Item 2 (T015's limits) is recorded nowhere in the plan, as ruled.

tasks.md's header and plan.md each carry a fifth revision paragraph.

5. T016's land confirmation (the holder, 6103915259 item 10), on Copilot's two "previously missed" points on T016's #100. Both land-confirmation issuers state the operation the invocation performs, decided from the same governance reading as landing_refusal(): a merge commit onto main when standalone, a submission to the host's instrument when governed. Recorded in the land confirm control's description in contracts/cli-http-submit-land.md (the terminal prompt, and the browser control), and in T016's Files line, which admits src/opendox/landing_confirm.py for that wording only (the prompt text plus the parameter that carries the operation kind). T016's Ruled line cites it. The nonce answer also gains an operation field (merge or submit), and land refuses a nonce whose operation changed as operation-changed: encoded in the contract's route table and refusal list and in T016's text, with the names read from T016's work in progress (openDox-code wip/r2-t016-park-0110, 743bb8b3), so they are inferred until T016 lands. The tests and the fix are T016's own work.

Files changed (4): tasks.md, plan.md, contracts/health-finding.md and contracts/cli-http-submit-land.md, all under specs/038-opendox-document-tool-self-maintenance/.

Ruled and Decisions

Readings, disclosed

  • (b)'s third member. The ruling reads "an engine category, identity-collision or uncited-disappearance's original kind under option (D) (6069024023 item 2), which is itself one of those". The brief's one-line summary names the first two. The text follows the ruling, with the option (D) clause.
  • The falsifier lines are untouched. The rulings word the tests they require inside the ruling ("with a test (defence in depth)" for T042's _json_text; "a test row and a killed mutant where it adds a guard" for the T041 points; "one test row per case" in 6073087924), and not as falsifier edits. Each is recorded in an Implementation note. The holder directs if any should also sit in a Falsifier line.
  • Line cites.
    • data-model.md:382, which the ruling cites, is the verdict row of § Sandbox probe and canary at :392 in this base. It is cited by section and row, and its quoted words match.
    • health_contract.py:865 and :693 are lane 3's cites in openDox-code, recorded as the rulings word them. They are not checked here.
  • identity-collision's entry. 6073087924 words a collision original's entry as "" through disappearance_identity(). The contract states it without naming the function, as the contract names none elsewhere; T041's note names the rule's source.
  • 6069507373 T044 item 7 is cited for the SHA-256 form, as the ruling cites it. Its own subject, a string over 200 characters, stays an implementation ruling for T044's writer, as T002d: plan 038's fourth bookkeeping batch, ticks T012 and T013, the two early-wave words, and the holder's wave-1 and wave-2 rulings (plan 038) #1279 left it.
  • The finding rows in data-model.md are unchanged: the brief names only the finding contract for the bound.
  • refused-patch is recorded in T041's note only, as the ruling directs. The contract's id rule does not list pathed engine kinds, and T049's entry words the refusal as "a finding against the pack naming refused_patch and reason" without spelling the kind, so the ruling's phrase "T049's entry names the kind" is read as that finding. data-model.md :209, :223 and :369 match the ruling's cites at this base.
  • The pinned schema was read from openDox-spec main (contracts/schemas/opendox-health-finding.schema.yaml, whose only commit is T040 Document content cleanup decision #17 → 7db9438b): path excludes C0 and C1 controls and surrogates, and identity.entry has the pattern ^(?:[a-z0-9-]+)?$.
  • The canonical text is written in the contract and in T041's note as the rulings word it, with the call's arguments on wrapped lines inside one code span.
  • Item 10's file is recorded in T016's Files line only, as the ruling directs. plan.md's single-writer table is not edited for src/opendox/landing_confirm.py, which the ruling does not name there.
  • Item 8's carried-forward note (T041 prints nothing and pack strings pass through verbatim, so T046's CLI and list --json and T057's HTTP and view must escape at their print boundary under one trust model) is for those tasks, not for this batch, and is not encoded here.
  • 6082100803's cite contracts/health-packs-manifest.md:24 is the id line of the manifest example ("opendox is reserved (15.7)"). It is cited by that content in the contract and in T041's note.

NOT encoded

  • Item 11 (6110997237), T016's two accepted limits (the shared redactor leaving a bare non-URL x-access-token:VALUE in prose; T015's submission_object accepting str subclasses). The ruling records them in T016's PR body and nowhere in plan 038's text.
  • (d) (splitting the engine-field set into two named sets). The ruling calls it a code clarity fix with no behaviour change, and the brief says to record it only if the plan names that field set. It does not: grep -rn 'ENGINE_FIELDS\|engine-owned\|engine field' specs/038-.../ finds no such set, only prose on "engine-owned identity keys" and "a pack's own id is ignored", which are unchanged.
  • Lane 3's two MINORs and two NITs beyond the one the ruling cites for (e): they are not named in the plan or the ruling's text.
  • T045's entry: the pack-output refusal is already T045's through 6069024023 item 1's mapping. Nothing in T045 changes, as the brief's Where column does not name it.

Gates (at the head dcc12988)

  • Validator. python3 scripts/validate-openspec-cli-pin.py --all and --all --strict both exit 0, with Totals: 113 passed, 1 failed (114 items). The one failure is the accepted exception, add-chain-attestation's disposition in the pin, as on main.
  • doc-health (scripts/doc-health.py --single-repo .), at dcc12988 against this branch's base 93d13d6c in the same clone: the two reports are byte-identical (441 lines each), so no new findings.
  • Local pytest (-m "not postgres" -n 8, the three gitlinks initialized as CI does) was last run in full at 0eba830b: 4 failed, 9203 passed, 7 skipped. All four are explained. Every commit since touches only specs/038-opendox-document-tool-self-maintenance/.
    • tests/doc-health/test_pin_reachability.py::test_the_resolver_this_probe_shares_with_the_readiness_proof_agrees failed because this writer committed while the run was in progress (it compares HEAD at start and at the end). It passes when re-run alone.
    • Two test_validator_cli.py real-repository nodes timed out at 30 s on a host at a load average of about 90, and pass when re-run alone.
    • test_release_mode_field_is_preserved_on_the_real_repository[--require-realization-realization] times out at 30 s when run alone on this host, and does the same on 93d13d6c in the same clone, as batch 4 recorded. CI runs it green.
  • CI at dcc12988: 15 of 15 checks SUCCESS (rollup SUCCESS, read through GraphQL), including pytest-suite, doc-health-py314, openspec-cli-pin, lane-line and SonarCloud.
    • pytest-suite reports selected=11063 passed=11057 skipped=6 failures=0 errors=0.
    • Its freshness verifier and its vector replay both passed.
  • Merge state: origin/main has moved to b27ae7a2 since this branch was cut at 93d13d6c. GitHub reports the PR MERGEABLE, and none of main's changes touches specs/038-opendox-document-tool-self-maintenance/, so main is not merged into the branch.
  • Threads: 9 in all, every one answered and resolved, 0 open. Codex raised one (4230671177, path and U+0000). Copilot raised eight (4230698859, 4232974858, 4232974918, 4234200786, 4234200849, 4234624544, 4235176471 and 4239576494). Each is answered by the holder's ruling in openDox + openXdox: two open-source layers, an installable app with users, projects and a database; domain descendants pin openXdox (Brett's ruling 2026-09-04) #656 (6086098003, 6088484643, 6088732352, 6089449884, 6090537166 or 6103336585), or by the fix it names.
  • Reviews at earlier heads:
    • Copilot reviewed 219e802c ("Approval recommended"), then 0eba830b, 1d413693, 163ed13c, c8f930cc, 47411923 and 43e3163b (each "Changes recommended", each finding answered above).
    • Codex reviewed 0eba830b once (13:33:59Z, "Reviewed commit 0eba830b21") and opened the one P1 thread above; on every other trigger, through 43e3163b, it answered "usage limits".
    • Copilot's and Codex's state at dcc12988 is requested after this body is posted, and is recorded in the READY-CANDIDATE or CODEX-LIMIT report.

NOT done

refs #656

🤖 Generated with Claude Code

…m and the T041 NAMES rulings (plan 038)

Bookkeeping: rulings already posted on #656 and a landing already merged,
encoded as worded, each cited by comment id or merge commit, with no new
decision. Touches only specs/038-opendox-document-tool-self-maintenance/.

- tasks.md: T071 ticked (openxFactory#1261 -> a020b34, Brett's ratify word
  6023375303); the NUL seam (holder 6072086385 item 4) in T041's note and
  Ruled line, T042's note and Ruled line, and T044's text and Ruled line;
  NAMES (b), (c) and (e) (holder 6072197564) in T041's note and Ruled line;
  the fifth revision paragraph; the task count.
- contracts/health-finding.md: the U+0000 bound on identity, locator, message
  and evidence, and the two Rules bullets (a pathless finding's identity, and
  the NUL seam).
- plan.md: the fifth revision paragraph.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 9, 2026 02:32
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@sourcery-ai

sourcery-ai Bot commented Oct 9, 2026

Copy link
Copy Markdown

Reviewer's Guide

This bookkeeping-only PR updates the plan-038 documentation to encode previously ruled NUL-seam and pathless-finding constraints, records their implementation ownership, and marks T071 complete after its ratification record landed; it introduces no new decisions or source-code changes.

File-Level Changes

Change Details Files
Records the fifth bookkeeping batch and marks the ratification task complete after verifying the merged landing.
  • Adds revision metadata and citations for the batch, rulings, and merge anchor.
  • Ticks T071 and records openxFactory#1261’s merge commit and ratification details.
  • Updates direction-arc task totals without changing decisions or implementation tasks.
specs/038-opendox-document-tool-self-maintenance/tasks.md
specs/038-opendox-document-tool-self-maintenance/plan.md
Defines the health-finding contract’s handling of NUL characters and pathless finding names.
  • Documents U+0000 refusal across identity, locator, message, and evidence strings.
  • Specifies closed pathless categories, the 200-character entry bound, and empty-entry rules for install-level categories.
  • Clarifies pack-output refusal, store-layer defense in depth, and SHA-256 handling for document strings containing U+0000.
specs/038-opendox-document-tool-self-maintenance/contracts/health-finding.md
Propagates the new rulings into T041, T042, and T044 implementation records while preserving the existing falsifiers and task scope.
  • Adds T041 notes for NUL rejection and NAMES points (b), (c), and (e), including required tests and mutant coverage.
  • Adds T042’s RefusedError behavior for NUL-containing JSON text.
  • Records T044’s non-emission guarantee and hash-based representation for affected document strings.
specs/038-opendox-document-tool-self-maintenance/tasks.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI 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.

🟢 Approval recommended

The changes consistently reflect the cited rulings and verified T071 merge metadata.

0 open findings

What changed in this PR

Records plan 038’s fifth bookkeeping batch and aligns health-finding rules with the latest rulings.

Changes:

  • Marks T071 complete with verified landing metadata.
  • Documents U+0000 refusal across finding validation and storage.
  • Defines pathless finding category and entry constraints.
File Description
tasks.md Records rulings, implementation notes, and T071 completion.
plan.md Adds the fifth bookkeeping revision summary.
contracts/​health-finding.md Adds NUL and pathless identity constraints.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…s category, beside (e) (plan 038)

Bookkeeping: holder ruling 6073087924 refines 6072197564 (e), encoded as
worded with no new decision. A pathless identity's entry is fixed by its
category: the seven per-entry categories require a non-empty entry;
no-sandbox, manifest-refused, dispositions-refused and identity-collision
require entry ""; entry-refused takes either. In T041's note and Ruled line,
contracts/health-finding.md (id-rule pointer and Rules bullet), the fifth
revision paragraph of tasks.md, and plan.md's paragraph.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 9, 2026 02:37
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI 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.

🔵 Needs a closer look

The documentation is consistent, but exact-head CI remains incomplete on the draft PR.

0 open findings

🧠 Review effort: Balanced

@brettheap
brettheap requested a balanced review from Copilot October 9, 2026 13:31
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T17:08:46.900735Z dcc1298 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@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: 0eba830b21

ℹ️ 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 specs/038-opendox-document-tool-self-maintenance/contracts/health-finding.md Outdated

Copilot AI 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.

🟡 Changes recommended

The raw entry-refused representation conflicts with the pinned finding schema.

1 open finding

🧠 Review effort: Balanced

Comment thread specs/038-opendox-document-tool-self-maintenance/contracts/health-finding.md Outdated
…s form and refused-patch (plan 038)

Bookkeeping: holder ruling 6086098003, on the review of this PR's own
threads, encoded as worded with no new decision.

- Item 3: the path bound (C0 and C1 controls and lone surrogates excluded, as
  the pinned finding schema does) in contracts/health-finding.md's path row
  and its U+0000 bullet; T041's note and T042's note (the store refuses
  U+0000 in every text parameter it binds).
- Item 4: entry-refused's identity.entry form (empty, the id as written when
  it conforms, otherwise sha256- plus the SHA-256 hex of the id's canonical
  text) in T041's note and the contract; the earlier "as written" wording is
  narrowed to the conforming case.
- Item 1: refused-patch joins ENGINE_KINDS, a pathed engine kind, in T041's
  note.
- Item 2 (T015's limits) is recorded nowhere in the plan, as ruled.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 9, 2026 17:45
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI 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.

🟡 Changes recommended

The canonical ID encoding remains undefined, and the PR description is stale.

2 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread specs/038-opendox-document-tool-self-maintenance/contracts/health-finding.md Outdated
Comment thread specs/038-opendox-document-tool-self-maintenance/tasks.md
…nd entry-refused's digest (plan 038)

Bookkeeping: holder ruling 6088484643 item 5, on Copilot thread 4232974858,
encoded as worded with no new decision. The canonical text of a refused
entry's id is Python's json.dumps(id, sort_keys=True, separators=(",", ":"),
ensure_ascii=True, allow_nan=True, default=str), falling back to repr(id) if
that raises; identity.entry is sha256- plus the lowercase SHA-256 hex of the
text's ASCII bytes (71 characters). In contracts/health-finding.md and
T041's note, with the mixed-type-keys test row; the fifth revision paragraph
and plan.md's paragraph cite it.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 9, 2026 20:14
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI 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.

🟡 Changes recommended

The canonicalization is not total or collision-safe, and a later entry-bound ruling is omitted.

3 open findings
1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Validate per-entry pack_id bounds and reserved opendox value

specs/​038-opendox-document-tool-self-maintenance/​contracts/​health-finding.md:159

This retains the premise that pack_id enforces the 40-character bound, but the later holder ruling 6082100803 explicitly corrects that premise: a pathless re-raise uses pack_id: opendox, so check_finding itself must reject an empty, reserved opendox, or non-1-to-40-character entry for every per-entry category. Without encoding that refinement here and in T041, the documented contract can admit a re-raised entry the engine could not build.

🧠 Review effort: Balanced

Comment thread specs/038-opendox-document-tool-self-maintenance/contracts/health-finding.md Outdated
Comment thread specs/038-opendox-document-tool-self-maintenance/contracts/health-finding.md Outdated
brettheap and others added 2 commits October 9, 2026 20:24
Bookkeeping: T015 landed as openDox-code#96, merge commit c94878be read
through GraphQL (2026-10-09T20:20:05Z). T015 is [x] with a Landed line in
the form of T014's, the Phase 4 task count lists it, and the fifth revision
paragraph and plan.md's paragraph record the tick. T015's accepted limits
are recorded nowhere in the plan (holder 6086098003 item 2).

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…fallback, the str() limit, the per-entry check (plan 038)

Bookkeeping: holder ruling 6088732352 item 6, on Copilot's three items at
163ed13, encoded as worded with no new decision.

- (a) The str() collision of a YAML date with its spelling is an accepted
  limit under 5988818366, stated in the contract's entry-refused text.
- (b) The canonical text's fallback is ascii(id), not repr(id), in the
  contract and T041's note, with a test row for a non-ASCII key in a
  mixed-type mapping.
- (c) Ruling 6082100803: for the per-entry categories check_finding refuses
  an entry that is empty, the reserved opendox, or not a 1-to-40-character
  name; it corrects the premise of 6072197564 (c). In the contract and
  T041's note.

The fifth revision paragraph of tasks.md and plan.md's paragraph cite them.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 9, 2026 21:14
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI 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.

🟡 Changes recommended

The contract contains an impossible uniqueness guarantee, and the PR description and gate evidence are stale.

1 open finding
3 resolved since last review

🧠 Review effort: Balanced

Comment thread specs/038-opendox-document-tool-self-maintenance/tasks.md Outdated
brettheap and others added 2 commits October 9, 2026 21:18
…limit (plan 038)

Copilot's thread 4234624544 at c8f930c: the helper's requirement "distinct
for distinct ids" contradicts the accepted limit of 6088732352 item 6 (a), by
which a YAML date or timestamp id and the string spelling its str() share a
digest. The requirement now reads "but for the accepted limit", in T041's note
and in the contract. No new decision: it follows the ruled limit.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ed limit (plan 038)

Bookkeeping: holder ruling 6089449884 item 7 widens the accepted limit of
6088732352 item 6 (a), encoded as worded with no new decision. The
canonical text is not injective across YAML value types: a date or
timestamp and its str(), and a non-string mapping key and its JSON spelling,
can share a digest; identity-collision reports any such collision. In
contracts/health-finding.md and T041's note, with the fifth revision
paragraph and plan.md's paragraph citing it.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 9, 2026 22:40
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI 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.

🟡 Changes recommended

The refused-entry canonicalization is nondeterministic for YAML sets and differs from the linked implementation’s recursion fallback.

1 open finding
1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Define or reject over-deep values to prevent digest collisions

specs/​038-opendox-document-tool-self-maintenance/​contracts/​health-finding.md:219

The linked T041 implementation has an additional fixed-text fallback when both json.dumps and ascii hit RecursionError, but this “exact canonical text” contract makes ascii(id) terminal and omits the resulting collision class (all such over-deep values share one digest). Align the contract/task and implementation by either encoding that fallback and accepted limit, or refusing/bounding these values instead.

🧠 Review effort: Balanced

Comment thread specs/038-opendox-document-tool-self-maintenance/contracts/health-finding.md Outdated
… sets (plan 038)

Bookkeeping: holder ruling 6090537166 item 8, on lane 3's REVIEW-W1 T041
NO-GO at openDox-code#97 and Copilot thread 4235176471, encoded as worded
with no new decision.

- (a) The canonical text's second fallback catches ValueError as well as
  RecursionError; an id neither spelling can write takes ONE fixed text,
  <past the int-digit or recursion limit>; two such ids share a digest, an
  accepted limit reported by identity-collision.
- (b) The default hook spells a set or frozenset as the list of its members
  sorted by each member's ascii() text; a set reached only through the
  ascii() fallback keeps Python's order, an accepted limit.

In contracts/health-finding.md's entry-refused text and T041's note, with the
test rows; the fifth revision paragraph and plan.md's paragraph cite it.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 10, 2026 23:22
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI 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.

🟡 Changes recommended

The specified default=str call bypasses the required deterministic set canonicalization hook.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread specs/038-opendox-document-tool-self-maintenance/tasks.md Outdated
brettheap and others added 3 commits October 10, 2026 23:32
…ok in the canonical call (plan 038)

Bookkeeping: holder ruling 6103336585 item 9, NIT 2 of lane 3's REVIEW-W1
T041 GO at openDox-code#97, encoded as worded with no new decision. The
int-digit limit is an interpreter setting (PYTHONINTMAXSTRDIGITS), not a
constant, so the same huge-integer id can take the fixed text under one
setting and be spelled out under another, giving a different digest: an
accepted limit beside item 8's two, operator-only. In the contract's
entry-refused text and T041's note, cited in the fifth revision paragraph
and plan.md's paragraph.

Also Copilot's thread 4239576494: the canonical call named default=str,
which bypasses the set hook ruled in 6090537166 item 8 (b). The call now
names the default hook (str, except sets and frozensets spelled sorted by
ascii()), in the contract and T041's note. No new decision.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…names its operation (plan 038)

Bookkeeping: holder ruling 6103915259 item 10, on Copilot's two "previously
missed" points on T016's openDox-code#100, encoded as worded with no new
decision. Both land-confirmation issuers state the operation the invocation
performs, decided from the same governance reading as landing_refusal(): a
merge commit onto main when standalone, a submission to the host's
instrument when governed. In the land confirm control's description in
contracts/cli-http-submit-land.md (the terminal prompt and the browser
control), and in T016's Files line, which admits src/opendox/landing_confirm.py
for that wording only; the fifth revision paragraph and plan.md's paragraph
cite it.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…nged refusal (plan 038)

Bookkeeping, following the holder's ruling 6103915259 item 10: the nonce
answer gains an operation field (merge, a merge commit onto main when
standalone; submit, a submission to the host's instrument when governed),
and land refuses a nonce whose operation changed since it was issued as
operation-changed. In contracts/cli-http-submit-land.md (the route table and
the refusal list) and T016's text. The names are read from T016's work in
progress, openDox-code wip/r2-t016-park-0110 at 743bb8b3, so they are
inferred until T016 lands; the fifth revision paragraph says so.

refs #656

Lane: openxfactory-4
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap requested a balanced review from Copilot October 11, 2026 17:04
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

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

ℹ️ 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 on lines +264 to +266
- T041 refuses U+0000 in every string a finding carries, keys and values alike
(`identity`, `evidence`, `locator` and `message`, and `path`, whose bound is its
row's above), with `FindingRefused` and a bounded `where`. For a pack's

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Reject NUL in the stamped pack version

This “every string” guard still omits pack_version, whose row permits any string and which is copied from health/packs.yaml. If an entry and its declaration use a version containing an escaped U+0000, the pack can pass the equality check and the engine stamps that value into findings and health_runs.pack_pins; T042 then refuses it only at persistence, aborting the run instead of treating the malformed entry as entry-refused. Validate the version before running the pack, or include the stamped field in the contract guard.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Lane: openxfactory-4

Referred to the holder as a QUESTION; no change is made yet.

  • The text encodes 6072086385 item 4 and 6086098003 item 3 as worded: "every string a finding carries", with path named by the second. pack_version is a string a finding carries, stamped by the engine from the manifest entry, and is not named by either ruling.
  • The pinned finding schema (openDox-spec contracts/schemas/opendox-health-finding.schema.yaml, T040 Document content cleanup decision #17, 7db9438b) already excludes U+0000 to U+001F and U+007F to U+009F, and lone surrogates, from pack_version (its pack-version-is-text rule), as it does for path. The contract's pack_version row is silent on it.
  • Whether the contract states that bound, whether T041 refuses it where the engine stamps the version, and whether a manifest entry whose version breaks it is refused before the pack runs (T047's manifest check) is the holder's to rule.

Copilot AI 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.

🔵 Needs a closer look

Nested unordered values can still produce hash-seed-dependent finding identities.

0 open findings

1 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Use recursive canonical keys for deterministic nested set ordering

specs/​038-opendox-document-tool-self-maintenance/​contracts/​health-finding.md:213

ascii() is not a stable ordering key when a member is itself unordered. For example, applying this hook to {frozenset({'a', 'z'}), frozenset({'b', 'y'})} emits the two nested lists in different orders under different PYTHONHASHSEED values, so the promised set/frozenset determinism does not hold. Define the sort key from each member's recursively canonicalized representation and cover a nested-frozenset case across processes.

Medium severity Fix hash-seed-dependent ordering for nested unordered members

specs/​038-opendox-document-tool-self-maintenance/​tasks.md:1319

This repeats the unstable ascii() ordering rule: an unordered member such as a frozenset has a hash-seed-dependent ascii() representation, which can reorder the outer set and change its digest. Specify recursive canonical member ordering here and require the cross-process test to include a nested unordered member, not only a flat set.

🧠 Review effort: Balanced

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants