Skip to content

Batch 448: connection editor FieldDescriptor value and defaultValue are optional; messagefoundry-toolkit is a published distribution (BACKLOG #1123, #1192) - #2582

Merged
wshallwshall merged 9 commits into
mainfrom
batch/b448
Oct 28, 2026
Merged

wshallwshall merged 9 commits into
mainfrom
batch/b448

Conversation

@wshallwshall

@wshallwshall wshallwshall commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Batch 448 (security-asvs), engine wave. Two items. ADR 0201 slice 5 of #1192 is on its own PR, #2580, as ADR 0201 section 5 requires.

BACKLOG #1123 -- research an honest pass for ASVS 3.5.5 (postMessage origin check in a VS Code webview)

Merge danger

Reversible: yes -- reverting the squash undoes it; a hand revert of ide/src/ and its test if only this item must go.

Blast radius: the IDE extension's connection editor types and one receiver test suite: ide/src/connectionForm.ts, ide/src/connectionEditorWebview.ts (comment only), ide/src/test/suite/webview-receivers.test.ts. No engine runtime code.

Evidence

  • Before: FieldDescriptor.value and defaultValue were declared required, but describe() / describeUnknown() set them to undefined and JSON drops undefined members, so the declared type over-promised what the webview receives. No fixture exercised a descriptor without them.
  • After: both are ?: unknown. A new FIELDS_BARE fixture joins the connection editor's well-formed list and the ACCEPTS loop accepts it. Mutation control: adding ('value' in f) (and separately ('defaultValue' in f)) to the receiver made the ACCEPTS fields test fail both times; the receiver was restored after each run. npm run typecheck exit 0, npm run compile OK, npm run test:unit 1200 passing, 0 failing.

Item detail

  • Source branch b448-1123-descriptor-optional, head 96e35b0d9f01adbb4e34869784ea24bb3b809923, base 93f6438c7.
  • The origin guard (mfTrusted plus a per-render token) was already at all eight receivers (PRs 1755, 1791, 2196, 2203, 2212), and the Test Bench isArrOf walk by index was built in PR 2196. This change is the one remaining engine defect the batch verification found.
  • The claim on fix(dicom): charge max_object_bytes before the C-STORE object is decoded (BACKLOG #1727) #1123 was not taken: a stale claim from a batch 209 Builder whose worktree no longer exists blocks -Take. The item is cited only in commit bodies.
  • Unread leg: ide build (windows-latest). It alone runs the Extension Host suite, and the Builder did not run that suite locally.
  • Known defects, open round-2 review findings shipped per korus BUILDER.md 4c. Findings 1, 2, 4 and 10 are PRE-EXISTING defects in shipped code that this branch neither causes nor fixes; they need their own ledger rows (handed to the Lander):
    1. Every save drops the default member of an {env, default} setting; the page never reads envDefault. Reproduced in jsdom.
    2. An inline secret literal is deleted on save after a fields rebuild, because preserveValue is never posted back. Reproduced in jsdom.
    3. The FieldDescriptor docs promise preservation that no caller implements.
    4. EDIT and CLONE draw the legacy free-text rows, so an inline secret appears in plain text in an editable box.
    5. A comment says "The two unknown-typed values" and then lists four (wording slip in this branch).
    6. describe() could omit the members instead of setting them to undefined.
    7. FIELDS_OK is delivered live rather than as JSON.
    8. "When value is undefined" is stated twice, and the two differ.
    9. "RENDER-ONLY, never post it" sits beside "posting the descriptor"; the meaning is "never write it back in a save".
    10. preserveValue sends the plaintext inline credential to a webview that never reads it.
    11. The bare.every(...) assertion only re-checks the fixture's own delete loop.

Proposed ledger banner, carried verbatim from the branch's final commit:

#1123 engine half BUILT: the postMessage origin guard (mfTrusted plus per-render token) was already at all eight receivers (PRs 1755, 1791, 2196, 2203, 2212); this change makes FieldDescriptor.value/defaultValue optional to match the JSON the webview receives, with a positive-control receiver fixture. The ASVS 3.5.5 vault amendment and its re-read follow this merge.

BACKLOG #1192 -- ADR 0201 slice 3: messagefoundry-toolkit is a published distribution

Merge danger

Reversible: yes -- a revert of the squash, or a hand revert of the two files.

Blast radius: tests/test_install_instruction_provenance.py (the published/unpublished distribution sets) and one docstring in _refuse_toolkit_command in messagefoundry/__main__.py. Behaviour is unchanged; no CLI subcommand or module moved.

Evidence

  • Before: messagefoundry-toolkit sat in _UNPUBLISHED_DISTRIBUTIONS, and the __main__.py docstring said the toolkit was "not yet on PyPI".
  • After: PyPI probe, 2026-10-27: messagefoundry-toolkit/json 200 (releases 0.5.0, 0.5.1, 0.5.2; earliest uploaded 2026-10-01), /0.5.0/json 200, /0.5.1/json 200, messagefoundry-harness/0.5.1/json 200; controls messagefoundry-toolkit/0.0.0.dev999/json 404 and messagefoundry-toolkit-notreal-zz9/json 404. Every toolkit wheel's provenance names publisher MEFORORG/MessageFoundry, workflow release.yml. The distribution moved to _PUBLISHED_DISTRIBUTIONS; no other distribution changed, so the sets docs(method): a Manager and the Lander may spawn a session #1193 shares are otherwise untouched.

Item detail

  • Source branch b448-1192-slice-3, head 7c0eb57e94f54aea83564b505fb04b8651f0ea0d, base 4e52f4ede.
  • Builder checks on the combined slices 3+5 branch: ruff and ruff format clean, all four mypy legs clean, 1295 passed and 1 skipped over 17 doc, ADR, CLI and boundary modules.
  • Known defects and records for follow-up (open review findings): ADR 0201 still says the PyPI name is unclaimed (line 134), its To resolve box says the owner's PyPI action is "Not done", the README row for 0201 says "No code yet", and the release.yml header still describes the Trusted Publisher as pending. Possible security gap for the owner, unverified: every toolkit wheel's provenance shows environment: None, while ADR 0201 section 1 says the publisher should carry environment pypi. The __main__.py docstring now names slice 4 and will go stale when slice 4 lands.

Proposed ledger banner, carried verbatim from the branch's final commit:

#1192: ADR 0201 slice 3 built (the install-provenance guard lists messagefoundry-toolkit as published after a PyPI probe with controls). Slice 5 is on its own PR; slices 4, 6, 7 and the release-gated 15.2.3 re-score (slice 8) remain.

Combined tree (batch/b448 at the head of this PR, base origin/main 4e52f4e)

  • ruff check .: all checks passed. ruff format --check .: 2152 files already formatted.
  • mypy --platform linux messagefoundry messagefoundry_webconsole messagefoundry_toolkit --exclude messagefoundry/tray/: no issues in 364 files. mypy --platform win32 messagefoundry messagefoundry_toolkit: no issues in 345 files.
  • python scripts/hooks/ledger_check.py --ci: exit 0.
  • Full pytest -n auto: 52239 passed, 1219 skipped, 151 xfailed, 6 failed. None of the six reads a file this PR changes (the diff is three ide/src files, one __main__.py docstring and tests/test_install_instruction_provenance.py); they test this machine's installed hooks and local tooling: test_gate_installed_parity, test_installed_coord_hooks (claim_check.py, coord/coord_root.py), test_steer_inject (CRLF case), test_workflow_shell_syntax, test_durability_hook_rewrite_coverage. A control run on origin/main could not run (the suite refuses a second checkout's tests under this venv), so CI is the arbiter for these.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no new actionable issues were found.

Summary

Makes FieldDescriptor.value and defaultValue optional to match the JSON sent to the connection editor.

  • Connection-editor fields can omit value and defaultValue.
  • The install-provenance checks treat the toolkit as published.

Acknowledgments from wshallwshall, excluded from findings:

  • Existing save behavior loses environment defaults and inline secrets; edit/clone can show inline secrets; preserveValue carries plaintext that the page does not read. These were identified as pre-existing and deferred.
  • Preservation docs, the “two” versus four wording, repeated undefined explanations, render-only wording, explicit undefined members, live fixture delivery, and the fixture’s delete assertion were acknowledged.
  • Stale ADR, README, and release comments, the unverified publisher environment concern, and the docstring’s future slice reference were recorded for follow-up.

Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'origin/b44..." · Reviewed by Greptile

…iew receives them

describe() and describeUnknown() leave both undefined in the ordinary case
(a key the record does not set, a secret, an env reference, a schema param
with no default), and postMessage carries the payload as JSON, which drops
an undefined member. The declared type promised two members the connection
editor page often does not get.

Both are now `?: unknown`. The comment above mfFieldDescriptor says why the
page's shape check does not require them: a presence check would discard
nearly every ordinary "fields" message. No check is added.

A new webview-receivers test delivers a fields message whose every field
lacks both members and requires it to be accepted. Mutation control: adding
a presence check for defaultValue to the receiver fails it.

BACKLOG #1123

Proposed PR title: IDE: FieldDescriptor.value and defaultValue are optional, matching what the connection editor webview receives
Proposed ledger banner: #1123 engine half BUILT: the postMessage origin guard (mfTrusted plus per-render token) was already at all eight receivers (PRs 1755, 1791, 2196, 2203, 2212); this change makes FieldDescriptor.value/defaultValue optional to match the JSON the webview receives, with a positive-control receiver test. The ASVS 3.5.5 vault amendment and its re-read follow this merge.
…ell-formed list

Simplify pass. The standalone test copied the ACCEPTS loop's body, so
FIELDS_BARE (FIELDS_OK with value and defaultValue deleted from every field)
is now a second well-formed "fields" fixture, and the fixture-shape test
pins that FIELDS_OK carries both members and FIELDS_BARE carries neither.
The webview comment points at FieldDescriptor for the reason rather than
restating it. Mutation control: a presence check on value in the receiver
fails the ACCEPTS test.

BACKLOG #1123

Proposed PR title: IDE: FieldDescriptor.value and defaultValue are optional, matching what the connection editor webview receives
Proposed ledger banner: #1123 engine half BUILT: the postMessage origin guard (mfTrusted plus per-render token) was already at all eight receivers (PRs 1755, 1791, 2196, 2203, 2212); this change makes FieldDescriptor.value/defaultValue optional to match the JSON the webview receives, with a positive-control receiver fixture. The ASVS 3.5.5 vault amendment and its re-read follow this merge.
…ned; fixtures carry envDefault and preserveValue

Round one of the diff check. `connection schema --json` always emits
`default` (null when the factory has none), so defaultValue is undefined at
least for a key the schema does not describe and for a schema param with no
`default` member; the first commit's message said "a schema param with no
default", which yields null, not undefined. value is undefined for a secret
and an env reference too, not only an unset key; the doc now says so.

The webview comment no longer calls envDefault and preserveValue
"carried-through": the page never reads either. FIELDS_OK now carries an env
default and an inline secret literal, and the fixture-shape test pins both
members present and absent, so a shape clause refusing either would fail.

BACKLOG #1123

Proposed PR title: IDE: FieldDescriptor.value and defaultValue are optional, matching what the connection editor webview receives
Proposed ledger banner: #1123 engine half BUILT: the postMessage origin guard (mfTrusted plus per-render token) was already at all eight receivers (PRs 1755, 1791, 2196, 2203, 2212); this change makes FieldDescriptor.value/defaultValue optional to match the JSON the webview receives, with a positive-control receiver fixture. The ASVS 3.5.5 vault amendment and its re-read follow this merge.
The toolkit name is claimed on PyPI, so the install-provenance guard moves
it from _UNPUBLISHED_DISTRIBUTIONS to _PUBLISHED_DISTRIBUTIONS. The
unpublished set is empty again, the goal state.

Probe read 2026-10-27 against PyPI's JSON API: 200 for
/pypi/messagefoundry-toolkit/json, /0.5.0/json and /0.5.1/json; 404 for
the control /0.0.0.dev999/json and for an unregistered project name. The
0.5.0 wheel's provenance names publisher MEFORORG/MessageFoundry, workflow
release.yml. The harness at 0.5.1 returned 200 as the ADR's control.

Moves no CLI subcommand or module; that is slice 4. Touches only the one
distribution in the two sets that BACKLOG #1193 shares.

BACKLOG #1192
The comment said "All four distributions"; a pinned count goes stale at
the next distribution. /simplify finding.

BACKLOG #1192
The _refuse_toolkit_command docstring said the toolkit was not yet on
PyPI; it now says the line was written while the name was unclaimed and
slice 3 recorded the claim. The provenance test restores the note that its
detector matches an install verb only, and its probe record names the
harness control, scopes each control path, and rests "this repo's upload"
on the release list plus every wheel's provenance, not on 0.5.0 alone.

BACKLOG #1192

Proposed PR title: ADR 0201 slice 3: messagefoundry-toolkit is a published distribution (BACKLOG #1192)

Proposed ledger banner: #1192: ADR 0201 slice 3 built (the
install-provenance guard lists messagefoundry-toolkit as published after a
PyPI probe with controls). Slice 5 is on its own PR; slices 4, 6, 7 and
the release-gated 15.2.3 re-score (slice 8) remain.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA -- korus roles/BUILDER.md step 11 (BACKLOG #1123)
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks a15b53ed493fd2ce8 round-97871e922a, ad9b315c0278ce4cb round-e18f96dffb; ascii spelling).
Rounds: 2. Findings: 4 confirmed and fixed in round one; 9 rejected or out of scope in round one; 11 open from round two (shipped per 4c; two are pre-existing data-loss defects, verified by the second fork).
Tags read by korus scripts/quality/review_tag.py at korus 20af3e604237db7293729b7048dae3657b738b56 (origin/main); both runs exited 0.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA -- korus roles/BUILDER.md step 11 (BACKLOG #1192, slices 3 and 5, one review over both)
Level: xhigh, from the brief. Tag: xhigh effort → 10 inline angles → dedup (no verify) → sweep → ≤15 findings (forks af5b0ccc480af0b83 round-6733012313, a419f47240e2c5056 round-75b3fdd824).
Rounds: 2. Findings: round 1 had 13 findings: 11 fixed, 2 split out or left open (the separate-PR finding answered by the split branches; the stale ADR 0201 and release.yml records left as row 4 below), 0 rejected. Round 2 had 11 findings, all open, none fixed: round two is the stop.
review_tag.py run from korus origin/main 20af3e604237db7293729b7048dae3657b738b56; exited 0 on both rounds and placed both forks by inheritedWorktreePath.

@wshallwshall wshallwshall added the qa Builder QA record posted; not a merge gate label Oct 27, 2026
@github-actions github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Oct 27, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Manager triage of the two red legs on head 51ca7e4 (the attribution is a reading for the Lander, not a ruling):

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (session 8a04c9f3), 2026-10-28. Enqueuing at pinned head 4f7286e512c2d4d9a01ddebe9a8fd1f0e40e0eb3.

Review proof: the Builder QA lines cover source heads 96e35b0d9 (#1123) and 7c0eb57e9 (#1192). 49ceb9ce, 51ca7e4c and 4f7286e5 are pure merges of main; each tree equals a fresh git merge-tree (exit 0). All nine required contexts are completed and successful on this head.

Open findings, each filed on vault main (vault PR 2734, merged 9ba0487fc). None changes product behaviour or weakens a control through this merge; findings 1, 2, 4 and 10 predate this PR:

@wshallwshall
wshallwshall added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 0585315 Oct 28, 2026
55 of 56 checks passed
@wshallwshall
wshallwshall deleted the batch/b448 branch October 28, 2026 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-red A required check went red. Attribute it before retrying. qa Builder QA record posted; not a merge gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant