Repository navigation
Bridge: a blank or invisible-character destination record ID settles nothing (stacked on #131) - #157
Conversation
…nothing Work.reconcile (http status lookup) and Session.reconcile (bound executor applied proof) accepted any nonempty string as the destination record ID that a verified reconciled proof cites. An ID that is only spaces, or that carries a control, format, separator or surrogate character (NUL, newline, U+200B, U+202E, U+00A0...), now names no record: the status lookup records destination_record_mismatch and the Session records external_id_missing, as each already does for an empty ID. New predicate syberlabs.evidence.record_id; cases in tests/test_bridge_record_id.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
CI on 86df060: ci (pull_request) https://github.com/SyberLabs/SyberWork/actions/runs/41263947211 · ci (push) https://github.com/SyberLabs/SyberWork/actions/runs/41263943341. These were in progress when this comment was posted. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86df060e46
ℹ️ 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".
| and not any(unicodedata.category(char) in INVISIBLE_CATEGORIES | ||
| or (char != " " and unicodedata.category(char) == "Zs") for char in value)) |
There was a problem hiding this comment.
When a status record or executor proof contains an invisible combining mark such as U+034F, this predicate accepts it because its Unicode category (Mn) is not checked, so Work.reconcile or Session.reconcile can still append a verified reconciled event for an ID that renders indistinguishably from another ID. Reject these characters too so the guard prevents the ambiguous IDs it is intended to catch.
Useful? React with 👍 / 👎.
Stacked on #131 (
feat/bridge-next-after-127, which now includesorigin/mainafter #156 by plain merge). It closes the first item #131's NOTES list under "Not judged": a record ID that is a nonempty string but names no record.The gap
Both reconcile paths copy a destination's record ID into the
external_idof a verifiedreconciledproof, and both accepted any nonempty string:Work.reconcile(httpstatus lookup,syberwork/core.py):isinstance(record["external_id"], str) and record["external_id"].Session.reconcile(bound executorappliedproof,syberlabs/session.py, rule added in Bridge: an applied Session proof without the destination's record id is unverified (stacked on #127) #131): the same test.So
" ","\t","SHIP-1"(zero-width space),"1-PIHS"(right-to-left override) or"SHIP-1\r\nSHIP-2"met acceptance and settled the effect. A blank ID names no record. An ID with an invisible character renders as an ID it is not (in the console, the dashboard, logs), or splits into two lines.Before / after (same tests on each tree)
external_id" "," "verified,reconciled, acceptance met (both paths)unverified/destination_record_mismatch. Session:unverified/external_id_missing. Unresolved, retry blocked\x85destination_record_ambiguous(strict parser)external_id_missing. HTTP: unchanged"SHIP-1","a b","ü-ä","日本-7","x""", non-stringChange
syberlabs/evidence.py: newrecord_id(value). True when the value is a string with a character other than U+0020, and no character is in category Cc, Cf, Cs, Zl or Zp, or is a Zs space other than U+0020.syberwork/core.py: thehttpstatus-record match usesrecord_id. A failure is the existingdestination_record_mismatch, as for an empty ID.syberlabs/session.py: the Bridge: an applied Session proof without the destination's record id is unverified (stacked on #127) #131appliedguard usesrecord_id. A failure is the existingexternal_id_missing, as for an empty ID.docs/RECONCILIATION.md: one new paragraph after Bridge: an applied Session proof without the destination's record id is unverified (stacked on #127) #131's. No G5-protected paragraph is touched (G5 against the parent: 0 changed with or without a decision).minLength: 1is left as it is. Tightening it would change how already-persisted histories validate. 23 conformance golden traces match.Cases (
tests/test_bridge_record_id.py, new, 6 tests)The file is outside the
tests/test_bridge_redteam*.pyglob and adds noCASEStags, so G2 and G3 counts are unchanged. It reuses the loopback destination fromtests/test_bridge_destination_ack.pyand theLostWriteexecutor fromtests/test_session_applied_proof.py. It imports the first as a module, so none of that file's tests are collected twice.RecordIdPredicate.test_unusable_ids_are_refusedRecordIdPredicate.test_usable_ids_and_non_strings"",None, numbers, bool, list, dict and bytes are refusedStatusLookupRecordId.test_status_record_with_an_unusable_id_is_a_mismatchdestination_record_mismatchwith the effect unresolved, commit refused (effect_claimed) and a fresh proposal blocked. The history is schema-valid, there is 1 POST, and there is 1 lookup per check. Positive: a usable ID then reconciles once, and a second reconcile isalready reconciledStatusLookupRecordId.test_usable_ids_reconcileSessionRecordId.test_applied_proof_with_an_unusable_id_is_unverifiedexternal_id_missingbody. Nothing settles, the history is valid, the chain is valid and the retry is blocked. Positive:"v1.4.0"settles onceSessionRecordId.test_a_non_ascii_visible_id_settles"日本-7"settles (false-positive control)Mutation check
record_id→isinstance(value, str) and bool(value)(= the old rule): all 3 refusal tests fail, with 20 failures and 29 errors across subtests. The first blank ID reconciles, and every later subtest then hitsalready reconciled. The 3 positive tests still pass.NOTES (claim ceiling)
httpstatus lookup andSession.reconcilewrite a verifiedreconciledonly for a record ID that has a character other than a space and has no control, format, separator or surrogate character. Thereconciledevents they write cite such an ID." v1"still reconciles); visible confusables (Latinaagainst Cyrillicа); length; private-use and unassigned code points. Categories come from the running Python'sunicodedata, so a code point assigned in a newer Unicode version could be classed differently on 3.11 and 3.13. No test uses one.economic_httpreceipt_matches(syberlabs/economic.py) keeps the nonempty-string rule. Settlement is another lane.syberlabs.evidence.verified_reconciliationstill accepts any truthyexternal_idin persisted events. Tightening it is a migration decision, as Bridge: an applied Session proof without the destination's record id is unverified (stacked on #127) #131 NOTES say.spec/reconciliation.schema.jsonstill allows any string withminLength: 1.CommandPublisher.status(syberlabs/publish.py) reportsappliedfor any nonempty string.Session.reconcilenow refuses the unusable ones downstream.httpwrite reply (Bridge: a non-strict http write reply that names another key or digest is an unknown effect #124/Bridge: a write reply whose later JSON record names another key or digest is an unknown effect (stacked on #124) #132),economic_httpsettlement, G6 mark detection (Bridge (G6): an Attest artifact offered as destination proof never reconciles #127).Local runs (Linux, Python 3.13.5, venv with hypothesis 6.170.14, no Postgres)
scripts/run_shards.py --floor c47e467(the Bridge: an applied Session proof without the destination's record id is unverified (stacked on #127) #131 tip): 1250 active + 5 xfail OK (23 skipped, not counted). Bridge: an applied Session proof without the destination's record id is unverified (stacked on #127) #131 has 1244 active + 5 xfail, so this PR adds 6 active tests.conformance.run: 23 golden traces matchedscripts/g6_bridge_mapping_lint.py: ok (6 checks)scripts/g2_bridge_quota_floor.py: exit 0scripts/g5_protected_text.py --base c47e467: 0 changedCI links follow in a comment once the runs finish. Those runs are evidence for this tree only and do not change required checks.
🤖 Generated with Claude Code