Repository navigation
🐛 fix(ui): fill a blank named __proto__ in the preview like the CLI does - #328
hivecommons-hive[bot] wants to merge 2 commits into
Conversation
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Decision needed: @clubanderson should decide whether #328 or #329 lands for #327 — their src/ui.html change is byte-identical, so only one should merge.
Conclusion: the fix is correct, but this PR and #329 make the same change to src/ui.html (identical blob a498032, hunks at lines 838 and 861) for the same issue (#327). One of them should be closed; I have not closed anything.
Correctness (verified against the tree at 7802e8b):
src/ui.html:838Object.create(null)makesscalarValues["__proto__"] = …an own data property instead of a silent no-op.src/ui.html:861Object.assign(Object.create(null), scalarValues)copies it:Object.assignuses [[Set]], and with a null prototype there is no__proto__accessor to intercept, so it lands as an own key.src/parser.ts:76-84—expand()already builds its view withObject.create(null)and gates onObject.hasOwn, so the key is honoured downstream.src/ui.html:863{ ...vals }uses CreateDataProperty, so the per-iteration label path (Object.entries(r.values), line 878) also sees__proto__. The issue's "label omits it" symptom is fixed too, though this PR's test does not assert it.
Difference between the two PRs (tests only):
- #328: one case in
src/ui-builder.test.ts— scalar value, no array vars, so the label branch (results.length > 1) is never exercised. - #329: five cases in
src/ui-interactions.test.ts— scalar and array__proto__, zip and cross modes, asserts theiter-labeltext and the--crossCLI flag.
Both are green on CI and both ship synced dist/ and a changelog fragment. On coverage alone #329 is the more complete of the two; if #329 is preferred, this one can be closed as a duplicate. If #328 is preferred, consider carrying over #329's array/label cases.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Decision needed: @clubanderson should decide whether to keep #329 (broader tests, human contributor) and close this one, or land this one and close #329 — both fix #327 with the identical src/ui.html change.
This duplicates #329. Both PRs close #327 and make the byte-identical change to src/ui.html (git diff of the two heads on that file is empty): src/ui.html:838 const scalarValues = Object.create(null) and src/ui.html:861 const vals = Object.assign(Object.create(null), scalarValues). #328 was opened at 09:39Z, #329 at 09:40Z.
The fix itself is correct. I read the consumers: sharedExpand (src/parser.ts:76-85) copies vals onto a null-prototype view and tests with Object.hasOwn, so an own __proto__ key is honoured; src/ui.html:863 spreads vals into a plain object for the label path, and spread uses CreateDataProperty, so __proto__ survives into Object.entries(r.values) at src/ui.html:880. This matches what the CLI already does in src/resolver.ts:115,174 and src/flags.ts:74. CI is green on this head.
Difference between the two: only tests and changelog filename. This PR adds one scalar case (src/ui-builder.test.ts:280). #329 adds five cases in src/ui-interactions.test.ts covering scalar + array __proto__ values, zip and cross modes, and the iter-label rendering — i.e. it also pins the src/ui.html:880 label path that this PR's test does not exercise.
Flagging for a maintainer: if you agree, this one could be closed in favour of #329, which covers strictly more of the changed behaviour. The two do not conflict (same src/ui.html, dist/ui.html content), so landing either closes #327. I have not closed anything.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Needs a decision, not a merge:
Head: verdict requires_human @b71efe0, 7/7 checks green, DCO yes, soak 0s of 30m0s. What to do: approve this head ( How to release it:
Clear the cause and the hive re-evaluates on its next pass; the stage moves to |
Closes #327
Summary
The builder UI preview was not filling blanks named
{{__proto__}}because it built its values objects as plain object literals (const scalarValues = {}andconst vals = { ...scalarValues }). Assigning to__proto__on a plain object is silently ignored, so the blank remained unfilled in the preview while the generated CLI line correctly emitted--__proto__=value.Changes
src/ui.htmlto buildscalarValueswithObject.create(null)and each iteration'svalswithObject.assign(Object.create(null), scalarValues)so that__proto__is stored as an own property.src/ui-builder.test.tsverifying that a template with{{__proto__}}and a row named__proto__with valueworldrenders asHello world(unfilled), notHello {{__proto__}}(unfilled).dist/ui.htmlanddist/ui-builder.test.jswith the source changes.changelog.d/fixed-ui-proto-blank.md.Validation
Builds and tests were not run locally per hive policy—CI is the verdict.
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88