fix(update_project): parse string-typed fields instead of iterating over characters#45825
Conversation
When output.fields is a JSON-encoded string (agent double-encoding), the old code iterated over character indices and created bogus SINGLE_SELECT project fields named 0, 1, 2, … Add resolveFieldsObject() which: - Parses a string value via JSON.parse, warns and returns null on failure - Explicitly handles null after parsing (e.g. JSON 'null') - Rejects arrays and other non-object types with a warning Both call sites in updateProject now use this helper. Fixes #44845 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
fields instead of iterating over characters
There was a problem hiding this comment.
Pull request overview
This PR hardens update_project against agent outputs that mistakenly double-encode fields as a JSON string, preventing Object.entries() from iterating character indices and creating junk project fields.
Changes:
- Added
resolveFieldsObject()to coercefieldsinto a plain object (including parsing valid JSON strings) and warn/skip on invalid types. - Routed both
applyFieldUpdatescall sites throughresolveFieldsObject()to avoid accidental per-character iteration. - Added tests covering JSON-string
fields, invalid JSON strings, and array inputs.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/update_project.cjs | Adds resolveFieldsObject() and uses it at both field-update call sites to prevent accidental junk field creation. |
| actions/setup/js/update_project.test.cjs | Adds regression tests for double-encoded fields and other invalid fields shapes. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Low
| core.warning("update_project: `fields` must be a JSON object; skipping field updates"); | ||
| return null; | ||
| } | ||
| return /** @type {Record<string, unknown>} */ fields; |
| expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining('Failed to create field "NonExistentField"')); | ||
| }); | ||
|
|
||
| it("warns and skips field updates when fields is a JSON-encoded string (double-encoded)", async () => { |
|
@copilot please run the
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Implemented fixes in 742360c. Investigated failing |
🤖 PR Triage
Score breakdown: Impact 28 + Urgency 18 + Quality 14 Fixes Run §29462479491
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in b2851bb. I investigated the failing step and updated |
|
🎉 This pull request is included in a new release. Release: |
When an agent double-encodes
fieldsas a JSON string (e.g."fields": "{\"Lifecycle\":\"Sandbox\"}") instead of a plain object,Object.entries()iterates over character indices["0","{"], ["1","\""], …, silently creating oneSINGLE_SELECTproject field per character. The board gets polluted with numeric junk fields that require manual GraphQL cleanup.Root cause
Both call sites in
updateProjectpassedoutput.fieldsdirectly toapplyFieldUpdatesafter only a truthiness +Object.keys().lengthcheck — no type guard.Changes
resolveFieldsObject()helper — added beforeapplyFieldUpdates:fieldsis astring→ attemptJSON.parse; warn and returnnullon failurenullafter parse (e.g. JSON'null')core.warningBoth call sites in
updateProject(draft issue path and regular issue path) now route throughresolveFieldsObjectbefore invokingapplyFieldUpdates