Skip to content

fix(v3/darwin): preserve empty plist containers; broaden stub detection in test - #5381

Closed
taliesin-ai wants to merge 1 commit into
wailsapp:agent/engineer-mac/8d616a3bfrom
taliesin-ai:agent/engineer-mac/8d616a3b-v2
Closed

taliesin-ai wants to merge 1 commit into
wailsapp:agent/engineer-mac/8d616a3bfrom
taliesin-ai:agent/engineer-mac/8d616a3b-v2

Conversation

@taliesin-ai

Copy link
Copy Markdown
Collaborator

Addresses review feedback on #5312.

  • sanitizePlistValue: return true unconditionally for dict/array types so legitimately empty containers in user plists are preserved (previously keep=false was returned for any empty container, even those with no template stubs).
  • TestOldFormatPlistMigration: broaden the assertion from five named stub checks to a single strings.Contains(contentStr, "{{") check so any surviving template syntax fails the test.

These two commits can be cherry-picked onto the PR #5312 branch directly.

Closes #5312

…on in test

sanitizePlistValue was returning keep=false for empty dict/array values,
silently discarding legitimately empty containers from user plists.
Fixed by returning true unconditionally for both cases.

Test assertion broadened from checking five named stubs to detecting any
surviving Go template syntax (contains "{{"), so new stub forms can't
slip through undetected.
@coderabbitai

coderabbitai Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • v3-alpha
  • master

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a2029df2-3df2-4cee-a179-b90d102f0913

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@leaanthony

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

@leaanthony
leaanthony deleted the branch wailsapp:agent/engineer-mac/8d616a3b May 10, 2026 14:12
@leaanthony leaanthony closed this May 10, 2026
@taliesin-ai

Copy link
Copy Markdown
Collaborator Author

Closing — superseded by #5312 + #5402

This PR was opened as a vehicle for review-feedback fixups while #5312 was blocked. Since then:

  1. PR fix(v3/darwin): strip template stubs from old-format Info.plist on update-build-assets #5312 was merged (commit 921e32961, 2026-05-11) with the full sanitize fix, TestPreserveOriginallyEmptyContainers, and the nuanced empty-container guard (len(val) == 0 early return).
  2. PR fix(v3): mapsEqual test helper panics on slice values #5402 was merged (commit 446ba94c0, 2026-05-11) fixing the mapsEqual slice-panic that fix(v3/darwin): strip template stubs from old-format Info.plist on update-build-assets #5312 introduced.

Master is now in the correct state. This PR, however, is stale against master and causes the "Run Go Tests v3" CI to fail on all three platforms with a panic whenever another PR runs CI:

  • This branch removes TemplateEnrichment (still used in master) → build failure
  • This branch reverts reflect.DeepEqual → == in mapsEqual → slice panic on TestPreserveOriginallyEmptyContainers/originally_empty_array_is_preserved
  • The entire internal/commands test package crashes, blocking unrelated PRs

Request: please close this PR. All its changes are covered by what is already on master.

CC @leaanthony


Taliesin is an AI agent. CC @leaanthony

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.

2 participants