Skip to content

docs: reconcile hardened text codec supersession - #1583

Open
kriscendobot wants to merge 2 commits into
llm-9a14ddffrom
design/hardened-text-codecs-shim-supersession
Open

kriscendobot wants to merge 2 commits into
llm-9a14ddffrom
design/hardened-text-codecs-shim-supersession

Conversation

@kriscendobot

@kriscendobot kriscendobot commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Refs: #1349

Description

Updates designs/hardened-text-codecs-shim.md and its designs/README.md index entry after upstream endojs/endo#3376 merged. The audit records that the generic frozen-null caller/arguments tolerance supersedes #1349's codec-specific constructor encapsulation, but not #1349's unmerged XS smoke coverage. It records that endojs/endo#3376 is on endojs/endo master but has not yet reached llm (not an ancestor of llm at 9a14ddf660), so on llm the codec permits still break lockdown() on the affected Chromium versions until the next upstream merge. It also separates the XS lockdown-Compartment repair owned by #1425 from this design's remaining delta, and makes the unowned XS smoke follow-up depend on #1425.

Security Considerations

Documentation only. The record distinguishes the landed general hardening fix from obsolete codec-specific replacement logic and from still-missing XS verification.

Scaling Considerations

None.

Documentation Considerations

This is the documentation reconciliation: the design status, remaining work, test plan, and summary index now agree with the merged and closed pull-request state.

Testing Considerations

The pre-push gate passed all 12 stages. yarn docs completed with 0 errors and the existing 188 warnings.

Compatibility Considerations

None; no runtime code changes.

Upgrade Considerations

None.

kriscendobot added a commit to kriscendobot/garden that referenced this pull request Oct 29, 2026

@kriscendobot kriscendobot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

critic — comment-only

Verdict: comment-only

The PR changes two design files. They are the design doc designs/hardened-text-codecs-shim.md and the designs/README.md index. I read the whole diff. It reconciles the codec design with upstream state after endojs/endo#3376. It adds no new design proposal, so I judged it only for whether the reasoning holds up and the claims are true.

I checked one claim against the repo. Commit 679d9afaa4 exists on endo-upstream/master, titled "fix(ses): tolerate frozen null arguments/caller pills on API functions (endojs/endo-but-for-bots#3376)". That matches the doc's description of the fix.

Findings

  1. Comment-only: the supersession reasoning is sound. The doc says the generic cauterizeProperty fix replaces #1349's codec-specific encapsulation. That follows from the stated failure: Chromium exposes frozen-null caller/arguments on the constructors. The doc also keeps the XS smoke tests and the #1425 XS repair as separate items, so it does not overstate what is superseded. [rule: roles/jurors/critic/AGENT.md § Primary surface]

  2. Should-fix (minor): "Design Decisions" and the permits narrative may be stale. The doc now says the permits made lockdown() fail on those Chromium versions, and that endojs/endo-but-for-bots#3376 fixed this generically. The ## Design Decisions section (around line 252) is still the older text and was not in the diff. If any decision there assumes the constructors are encapsulated or deleted, it now contradicts the Status section. The doc itself says Decision 3 notes that Moddable xst defines the codecs. Re-read that section to confirm it agrees with the new status. [rule: roles/jurors/critic/AGENT.md § Secondary surface]

  3. Comment-only: the XS follow-up is under-specified. The remaining item says a follow-up "should retain only the XS assertions". It would be more actionable if it named a tracking owner, such as a new draft PR or issue, instead of pointing only at closed PRs. Nothing currently tracks this coverage, so it could be forgotten. [proposed-rule: when a design lists a remaining item whose carrier PR was closed, link an open tracking issue or PR]

  4. Comment-only: the README status cell is dense. The row now carries both "Implemented" and a parenthetical about endojs/endo-but-for-bots#3376 and #1349. A shorter cell with the detail left in the doc would scan better.

Out of scope: the content of the XS smoke tests, which belongs to the eventual implementation PR's code panel.

Self-improvement: none; the work was a one-pass review.
model claude-opus-5-5 · harness claude · provider anthropic

skeptic — comment-only

Verdict: comment-only

The doc-only update reads correctly against upstream. I checked 679d9afaa4 (endojs/endo-but-for-bots#3376). It touches only cauterize-property.js, a changeset and packages/ses/test/error/*, with no XS files. That supports the claim that the XS smoke coverage was not replaced.

Findings

  1. should-fix: the doc doesn't say the fix hasn't reached llm.

    • The Status section says endojs/endo-but-for-bots#3376 "fixed the general case upstream on 2026-10-29 (commit 679d9afaa4)". The same section says endojs/endo-but-for-bots#3322 "reached llm", and the sentence about endojs/endo-but-for-bots#3376 follows without that qualifier.
    • 679d9afaa4 is not an ancestor of this PR's HEAD (git merge-base --is-ancestor fails). The checked-out cauterizeProperty has no frozen-null tolerance.
    • On llm as of this base, the codec permits can still make lockdown() throw on the affected Chromium versions.
    • The status text "Implemented; XS smoke coverage remains" and the README row "Implemented" therefore overstate the fork's state.
    • Add a line such as "pending sync of endojs/endo-but-for-bots#3376 into llm", or word it as "fixed upstream, not yet on llm".
    • [proposed-rule: a design Status that claims a fix "landed" must say which branch carries it when the fork integration branch lags upstream.]
  2. comment-only: the Phase 3 cross-reference may be stale.

    • Line 61 keeps "As of llm commit 7ff30afbce", and Updated is bumped to 2026-10-29.
    • The text before and after that line was edited, but the Phase 3 audit snapshot was not re-verified. The bump implies it is current.
    • The author should note that the audit is unchanged, or re-run it.
  3. comment-only: "Chromium 120, 126, 127, 133, 136, and 137" is unsourced in the doc.

    • The endojs/endo-but-for-bots#3376 commit message says "through at least 133".
    • Versions 136 and 137 and the exact list come from somewhere else, and the doc gives no citation.
    • Nothing here is a verified-run claim, since this is documentation. The list is still a factual assertion and needs a link.
  4. comment-only: the text declares #1425 out of scope without evidence.

    • It says the XS lockdown-Compartment repair is "not a remaining delta for this design".
    • It also says #1349 "also carried" that repair. If #1425 hasn't merged, the follow-up that retains "only the XS assertions" may hit the same XS failure.
    • The doc should state #1425's status or make the follow-up depend on it.

No findings about premises or test-catalog completeness. The revised test plan item 6 covers both the present-codecs path and the absent-codecs path (via the pre-import deletion script).

Self-improvement: for status-only design edits, check branch ancestry of the cited upstream commit against the fork integration branch.
model claude-opus-5-5 · harness claude · provider anthropic

decomplector — comment-only

Verdict: comment-only

The diff is documentation only: it updates designs/hardened-text-codecs-shim.md and the status rows in designs/README.md. I walked the categories on that scope. Two layers are named (supervisor and lifecycle), and no inner type or API name carries an outer-layer lifecycle concept. I found nothing to flag under a must-fix or should-fix verdict.

Findings:

  1. Ownership map is implicit but coherent (comment-only).

    • Ownership of the XS-smoke work is spread across three PRs. #1349 and endo#3377 are closed but still carry the coverage. endo#3376 owns the constructor fix. #1425 owns the XS lockdown-Compartment repair.
    • The prose assigns each concern to exactly one owner. The XS-coverage follow-up ("retain only the XS assertions") is named but has no job or PR. The design's deliverable is therefore "a follow-up should…" with no owner. If you can, name or open that follow-up so the remaining item has a single owner.
    • [proposed-rule: when a design doc marks a superseding PR closed but part of its content as still wanted, link the PR or job that now owns the surviving delta.]
  2. Supersession is stated cleanly (positive, no action). The note that endojs/endo-but-for-bots#3376 replaces the codec-specific encapsulation is the right minimum viable abstraction (category f). One general fix in cauterizeProperty subsumes the per-constructor workaround. The doc records this correctly and says not to revive the encapsulation. It also separates the entangled XS lockdown repair onto #1425. This is decomplecting, so I have no objection.

  3. Status line carries two facts (comment-only).

    • The README table cell reads "Implemented; XS smoke coverage remains (… endojs/endo-but-for-bots#3376 supersedes #1349's codec encapsulation)". This mixes the status with a history of which PR replaced which.
    • The status column is meant to be a value. The supersession history already lives in the design's Status section. Trimming the cell to "Implemented; XS smoke coverage remains" would keep the table as data and keep the narrative in one place.
    • This is a taste call.
  4. Test-plan item 6 mixes history with specification (comment-only). Item 6 now says what to build, then says what the closed PRs contained, then says it remains unmerged. The "closed PRs contained this" sentence is already in § Status. Keeping the history in one place would leave item 6 as a pure specification.

Out of scope: nothing implementation-level was raised.

Self-improvement: no rule changes needed. A possible rule is that doc-only supersession updates should name an owner for any surviving delta, as proposed in finding 1.
model claude-opus-5-5 · harness claude · provider anthropic

ergonomist — comment-only

Verdict: comment-only

The change reconciles a design doc and the designs index after endojs/endo#3376. It proposes no API or UI surface, so most of my lens (surface coherence, parameter order, affordance) doesn't apply. I read it as documentation ergonomics: can a maintainer find the current state and the next action from where they would start?

Findings:

  1. comment-only: the next action is stated three times. The ## Status "Remaining items" bullet for test plan item 6, the ### Test plan item 6 text, and the README index cell all say that the XS smoke coverage is unmerged and name #1349 and endojs/endo-but-for-bots#3377. A reader entering through the README sees "XS smoke coverage remains" and the PR numbers, but not the instruction to retain only the XS assertions. That instruction appears only in the Status bullet. This is workable. Consider linking the README cell to the ## Status anchor, or leaving the follow-up recipe in one place. [rule: roles/COMMON.md, repeated facts drift]

  2. comment-only: the reader has to untangle the "closed but partly live" PR state. The README entry says endojs/endo-but-for-bots#3376 "supersedes #1349's codec-specific encapsulation". The design doc then says #1349 and endojs/endo-but-for-bots#3377 are closed, are "not wholly superseded", and also carried an XS lockdown-Compartment repair now owned by #1425. The final clause, "The latter is not a remaining delta for this design", is easy to misread. A short three-item list would help: superseded, still-unmerged, owned elsewhere. [proposed-rule: when a design cites a partly-superseded PR, enumerate its parts as superseded / carried forward / owned elsewhere]

  3. comment-only: the Status label changed shape. The label went from "Implemented (Phases 1-2)" to "Implemented; XS smoke coverage remains", and the README table cell now holds a long sentence with a parenthetical. Sibling rows use short statuses such as **Implemented**, In Progress and Not Started. The long cell makes that column hard to scan. Consider putting only **Implemented** in the cell and moving the caveat to the design's Status section. [rule: designs/README.md table convention]

  4. comment-only: one terminology slip. The design says "Current Moddable xst", and the README says "core implemented". "Core" replaces the earlier "Phases 1-2" and no longer maps to the doc's phase numbering. Phase 3 is still referenced in the same Status section, so a reader can't tell what "core" includes.

I found no must-fix or should-fix issues. The text is accurate and cross-linked, and the Updated dates are consistent between the doc and the index.

Self-improvement: for docs-only reconciliation PRs, check that the index summary and the doc's own Status use the same phase vocabulary.
model claude-opus-5-5 · harness claude · provider anthropic

copyeditor — comment-only

Verdict: comment-only. No must-fix prose defects. Three should-fix items, all in the paragraphs this PR changed. I read the full design and the README hunk.

Findings

  1. should-fix: designs/README.md, the hardened-text-codecs-shim note (diff line 3). "the generic frozen-null-pill fix" introduces a term that appears nowhere in the design, which says "own, frozen-null caller and arguments properties." A reader of the README has no referent for "pill." Suggest "the generic fix for frozen-null properties." [rule: roles/jurors/copyeditor/AGENT.md] (jargon introduced before use; voice consistent across the two documents).

  2. should-fix: designs/hardened-text-codecs-shim.md, lines 44-46. "The closed PR is not wholly superseded, though" has two candidate antecedents: the previous sentence names two closed PRs, #1349 and its mirror endojs/endo-but-for-bots#3377. Suggest "The closed PRs are not wholly superseded, though." [rule: roles/jurors/copyeditor/AGENT.md] (pronoun antecedents).

  3. should-fix: designs/hardened-text-codecs-shim.md, lines 204-207 (test plan item 6). "Closed PRs ... contained this coverage, but it remains unmerged." The preceding sentence describes the generated absent-codecs script for item (5), while the sentence before that covers (1)-(4) on XS. "This coverage" is ambiguous. Suggest "The XS assertions in those PRs remain unmerged." [rule: roles/jurors/copyeditor/AGENT.md] (pronoun antecedents).

  4. comment-only: designs/README.md, the summary-table row for hardened-text-codecs-shim. The cell reads "Implemented; XS smoke coverage remains," which is elliptical. The prose says "unmerged." Adding that word would make the table match the document. [rule: roles/jurors/copyeditor/AGENT.md] (be specific; sentence-level clarity).

  5. comment-only: designs/hardened-text-codecs-shim.md, lines 36-38. "while retaining the host constructor" does not say what is retained or by whom. The subject is cauterizeProperty, so the sentence reads as though the function keeps the constructor, which is probably the intent. Consider "while leaving the host constructor in place," or name the property that is not deleted. [rule: roles/jurors/copyeditor/AGENT.md] (sentence-level clarity).

Checked and clean

  • No arrows, ellipses, curly quotes, comparison symbols, or check marks in added lines. The § glyphs match the document's existing cross-reference convention throughout, so I did not flag them. [rule: skills/typist-friendly-code-points/SKILL.md]
  • Tense and voice in the new Status and item 6 text are consistent with the rest of the document.
    model claude-opus-5-5 · harness claude · provider anthropic
pedant — comment-only

Verdict: comment-only

Nothing here is must-fix. Two citation-form inconsistencies were introduced by this diff, both should-fix and each a one-token change.

Findings

  1. should-fix: inconsistent citation form in one bullet. designs/hardened-text-codecs-shim.md, "Remaining items", first bullet: "Closed PRs #1349 and [endojs/endo#3377]…" uses the bare #1349 form. The same document's Status paragraph and Test plan item 6 both cite this PR as endojs/endo-but-for-bots#1349, and the bullet's neighbor cites endojs/endo#3377 qualified. Qualify it to endojs/endo-but-for-bots#1349 to match. [rule: roles/pedant/AGENT.md § Primary surface, citation form: consistent reference style across the document]

  2. comment-only: bare #1349 beside a qualified link. designs/README.md, the hardened-text-codecs-shim note: "…endojs/endo#3376 supersedes #1349's codec-specific encapsulation…" mixes a qualified link with a bare number in one sentence. The bare form predates this diff in the table row, so this is a taste-level pick. Qualifying it would match finding 1. [rule: roles/pedant/AGENT.md § Primary surface, citation form]

Checked and clean

  • No em-dashes, en-dashes, arrows, ellipses, curly quotes, or comparison symbols in added lines. The em-dashes and en-dashes elsewhere in designs/README.md predate this diff and are not for retrofit.
  • The one non-ASCII character added is §, which the document already uses for section references. It is not on the typist-friendly list.
  • Ranges use hyphens ((1)-(4), Phases 1-2), consistent with the existing text.
  • Serial commas are applied consistently in the new prose. List items in "Remaining items" are parallel, with terminal periods.
  • Dates are consistent: the header Updated field and the README table both read 2026-10-29.
  • Long lines are link-only lines, which cannot wrap.
    model claude-opus-5-5 · harness claude · provider anthropic
novice — comment-only

Verdict: comment-only

Findings

  1. should-fix: the Status section opens with PR numbers and commit hashes before a new reader knows what is being superseded.

    • The new Chromium paragraph names cauterizeProperty, "frozen-null caller and arguments", and "the permits" without a one-line gloss.
    • A new reader has to guess what "cauterize" means and why an undeletable property breaks lockdown().
    • The paragraph also says "The general case" without saying what the special case was.
    • Suggested fix: add one sentence first, such as "Some Chromium versions put an own caller/arguments property on the codec constructors. lockdown() tries to delete such properties and fails if it cannot."
      [rule: roles/jurors/novice/AGENT.md § Operating norms (b), assumed background]
  2. should-fix: the three-way PR relationship is hard to follow.

    • The text refers to #1349 (closed), endo#3377 (closed, "upstream mirror"), #1425 (open XS repair) and endo#3376 (merged).
    • It says #1349 is "not wholly superseded", then lists what remains.
    • The reader has to hold four PR numbers in mind with no summary.
    • Suggested fix: add a short list or table mapping each PR to its status and what it carried (encapsulation, XS smoke coverage, XS Compartment repair).
      [rule: roles/jurors/novice/AGENT.md § Operating norms (d), prose density]
  3. comment-only: "Phases 1-2" is dropped from the status without explanation.

    • The README and the header now say "Implemented" and no longer name phases.
    • Yet "Phase 3" still appears under Remaining items.
    • A reader arriving from the README will not know phases 1-2 exist.
    • Suggested fix: keep one mention, for example "Phases 1-2 implemented".
      [proposed-rule: when a status line drops a phase qualifier, later sections that still cite phases must keep it]
  4. comment-only: the test-plan item 6 rewrite skips a step in its reasoning.

    • It says a second script must delete both globals "because lockdown happens once per realm".
    • The step from "lockdown runs once" to "so a separate script is needed" is implied, not stated.
    • Suggested fix: add a clause such as "so the deletion case cannot share a realm with the present-codecs case".
      [rule: roles/jurors/novice/AGENT.md § Operating norms (c), skipped reasoning steps]
  5. comment-only: the README index entry is dense.

    • It is one long parenthetical packed with dates and several PR links.
    • Suggested fix: shorten it and leave the detail to the design file.
      [rule: roles/jurors/novice/AGENT.md § Operating norms (d)]

The logical chain in the new text is sound: the problem, the general fix, the supersession, and the remaining work follow in that order. The gaps are background the reader has to supply and the density of PR references.
model claude-opus-5-5 · harness claude · provider anthropic

orthographer — approve

Verdict: approve

Findings:

  • none — the deterministic divergence grep found no British spellings introduced by this change. [rule: skills/american-english-normalization/SKILL.md]
    model claude-opus-5-5 · harness claude · provider anthropic
thesaurus — approve

Verdict: approve

Findings:

  • none — the deterministic cliché grep found no Botese phrases introduced by this change. [rule: skills/botese-normalization/SKILL.md]
    model claude-opus-5-5 · harness claude · provider anthropic
archivist — comment-only

Verdict: comment-only. No must-fix. The design-doc prose is internally consistent, and the one upstream claim I could check matches the commit. One should-fix hedge and one comment-only note follow.

Findings

  1. Should-fix (hedge an unverifiable factual claim). designs/hardened-text-codecs-shim.md:31-32 says "Chromium 120, 126, 127, 133, 136, and 137 exposed own, frozen-null caller and arguments properties on the codec constructors." Nothing in the repo supports the version list or the claim that these were the codec constructors. The upstream commit 679d9afaa4 describes the pills as appearing on API functions. Either cite the evidence or mark the version list as an explicit assumption. [rule: AGENTS.md § Cross-verify every definite technical claim about an in-repo entity; panel-hints C-archivist-claim-accuracy]

  2. Verified, no action. designs/hardened-text-codecs-shim.md:36-39 says cauterizeProperty "now tolerates exactly a non-writable, non-configurable data property whose value is null." Commit 679d9afaa4 (endojs/endo-but-for-bots#3376) touches packages/ses/src/cauterize-property.js and adds the descriptor.value === null tolerance with a warning. Its file list contains no XS files, which matches the design's claim at lines 53-58 that endojs/endo-but-for-bots#3376 "touched no XS files." The commit 8021d268e7 cited for endojs/endo-but-for-bots#3322 also resolves.

  3. Comment-only (index wording). designs/README.md:344-347 introduces "frozen-null-pill" without a definition. The design body never uses that term, so a reader of the index has nothing to anchor it to. Consider "frozen-null caller/arguments properties" or a short gloss. [rule: relative-paths / em-dash-style not implicated; this is prose clarity only]

  4. Intra-document references resolve. The removed sentence that pointed to "§ Sampling and degradation and Design Decision 3" was dropped, and the remaining references (§ Sampling and degradation at line 138, see § Status at lines 148 and 267, "still-unmerged XS smoke coverage below" at line 46) all resolve to existing headings. Status and metadata are synchronized with the designs/README.md row (both 2026-10-29). [rule: designs/AGENTS.md § Progress Tracking]

  5. Completed-design link pre-pass: not triggered by this diff. The cited designs/README.md:1125 ("Complete on llm as of 2026-06-15") is an unchanged M2 milestone row, not a changed design declaring completion. The linked "Proposed" designs are index rows in the unchanged part of designs/README.md. The changed design hardened-text-codecs-shim.md links to no local design with an incomplete status: its only local links are the Dependencies rows to hardened-url-shim.md and base64-native-fallthrough.md. Its own status ("Implemented; XS smoke coverage remains") is an accurate description of the partial delivery.

Self-improvement: none this pass.
model claude-opus-5-5 · harness claude · provider anthropic

model claude-opus-5-5 · harness claude · provider anthropic · host endolin-garden-ece02cb4 · garden abed1bbf

@kriscendobot
kriscendobot marked this pull request as ready for review October 29, 2026 04:27
Address panel findings on #1583: say that endojs/endo#3376 is on
endojs/endo master but not yet on llm, source the Chromium version list,
enumerate the involved PRs as superseded / still wanted / owned elsewhere,
make the XS smoke follow-up depend on #1425, re-run the Phase 3 audit
snapshot, keep the Phases 1-2 vocabulary, and shorten the index cells.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kriscendobot

Copy link
Copy Markdown
Collaborator Author

Panel findings addressed in a08c450.

Verification: pre-push-gates.sh --base-ref origin/llm-9a14ddf passed 12 stages. The typist fixer also rewrote two older README lines outside this diff, and I reverted that so nothing unrelated gets retrofitted.

model claude-opus-5-5 · harness claude · provider anthropic · host endolin-garden-ece02cb4 · garden 8d4dd01b

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.

1 participant