Skip to content

fix(cli): onboarding config rewrites survive comments with apostrophes#70

Merged
BenSheridanEdwards merged 1 commit into
mainfrom
fix/config-comment-parsing
Jul 2, 2026
Merged

fix(cli): onboarding config rewrites survive comments with apostrophes#70
BenSheridanEdwards merged 1 commit into
mainfrom
fix/config-comment-parsing

Conversation

@BenSheridanEdwards

Copy link
Copy Markdown
Owner

Summary

  • findMatchingBrace (the parser behind onboarding's nativeproof.config.ts rewrites) tracked quote state but not comments. An apostrophe in a comment — // don't pin a simulator model — flipped the tracker into "inside string" mode and swallowed real braces. Confirmed effects on legal configs: onboarding failed with "could not update the ios project", and with apostrophes in more than one project's comments it could rewrite the wrong project's "appium:app" while reporting success.
  • Line (//) and block (/* */) comments are now skipped the same way strings are; unterminated comments return "no match" and surface the existing clear error rather than corrupting.

Proof

  • npm run check
    • Passed on fix/config-comment-parsing.
  • npm test
    • Passed: 155/155 (new test: a two-project config with apostrophe + brace-bearing comments updates exactly the iOS block and leaves Android untouched — previously threw).
  • Generated project/device proof, or N/A:
    • Not applicable to devices. The onboarding config-rewrite path is covered by the CLI unit suite and the packaged-CLI smoke test (pack → init → onboard), both green; the failing input was confirmed by repro against unfixed code.
    • Screenshots: Not applicable; CLI text-transformation fix, asserted verbatim in test/cli.test.ts.

Risk

  • Low. Comment-free configs parse identically; comment-bearing configs previously failed or corrupted.

Notes

  • Full device coverage not run: no device interaction in this path.
  • Regex literals inside config expressions (e.g. /{2}/) remain a theoretical desync; tracked separately rather than growing this parser further.

PR Proof Law

  • I followed .agents/DEFINITION_OF_DONE.md and .agents/skills/pr-inline-screenshot-proof/SKILL.md.
  • Screenshots are committed and embedded inline with ![alt](...png?raw=1), or the proof section says Not applicable with the technical reason.
  • The PR body has no bare screenshot links, local paths, relative paths, or proof placeholders.

findMatchingBrace tracked quote state but not comments, so an apostrophe
in a comment (// don't pin a simulator model) flipped it into
inside-string mode and swallowed real braces: onboarding failed on legal
configs, and with certain comment arrangements rewrote the WRONG
project's appium:app while reporting success. Line and block comments
are now skipped like strings.
@BenSheridanEdwards BenSheridanEdwards force-pushed the fix/config-comment-parsing branch from 79fae53 to 02fba50 Compare July 2, 2026 21:00
@BenSheridanEdwards BenSheridanEdwards merged commit a1a78e1 into main Jul 2, 2026
1 check passed
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