fix(uninstall): match quoted keys/values and inline comments

[iap]

Summary

Follow-up to #97. Hardens scripts/uninstall.sh to also match quoted keys/values and inline comments on the builder entries, so a hand-edited config.yaml can't leave a dangling reference on uninstall.

Problem

The line-based matcher (introduced in #97 to preserve comments) only recognized the exact unquoted forms setup.sh/Hermes actually write. A user who hand-edited their config with quoting or inline comments would leave behind:

• "aws-builder": / 'builder': (quoted provider key)
• - "builder" / - 'builder' (quoted enabled/toolset list item)
• provider: 'builder' (single-quoted model.provider)
• aws-builder: # note / - builder # note / provider: builder # note (inline comments)

These are false-negatives (fail to remove, never corrupt) — the worst case is a visible dangling reference — but the original yaml.safe_load-based cleanup handled all of them, so this closes that robustness gap.

Change

Four small helpers, and the three matchers now route through them:

• _strip_inline_comment(s) — drop a trailing whitespace-preceded # comment (quote-aware).
• _unquote(s) — strip matching single/double quotes.
• _mapping_key(s) — unquoted key of a key: ... line (provider block detection).
• _provider_value(s) — unquoted value after provider: (model.provider detection).
• _is_builder_item now strips comments + quotes before comparing.

Adds 3 regression tests (quoted provider keys/values, quoted list items, inline comments). No behavior change for the unquoted, comment-free forms.

Tests

• tests/test_uninstall.py: 15 tests (3 new) — all pass.
• Full suite: 220 passed, 1 skipped.
• ruff clean (3 local-only N999 from the checkout dir name), ruff format clean.

Greptile Summary

The uninstall cleanup now handles quoted scalar whitespace and inline comments, but valid YAML double-quoted escape sequences are still missed. A Builder configuration written as "bui\x6cder" remains installed because the script does not recognize that YAML resolves it to builder.

Confidence Score: 4/5

Not safe to merge until uninstall recognizes YAML-escaped Builder identifiers.

The reproduced cleanup failure leaves plugin-owned provider, toolset, plugin-list, and model-provider configuration behind when equivalent YAML escape notation is used.

Files Needing Attention: scripts/uninstall.sh

T-Rex Logs

What T-Rex did

• Produced a proof for a posted P1 finding by generating a validation script and associated logs.
• Inspected the literal uninstall control log and the YAML-escaped uninstall failure log to verify evidence for the posted finding.
• Mapped the uninstall script code sections that drive unquote/escape logic and noted the exact locations used in the validation.
• Authored and executed the uninstall unquote-escape validation script and captured before/after logs to document the validation run.

_{Ran code and verified through T-Rex}

Comments Outside Diff (3)

1. General comment

   Uninstall conflates quoted whitespace-preserving scalars with Builder entries

   • Bug
     • A YAML provider key, managed-list item, or model.provider value written as " builder " is semantically distinct from builder, but uninstall removes it. The isolated end-to-end run removed the unrelated provider block, all quoted managed-list entries, and the model provider line.
   • Cause
     • _unquote in scripts/uninstall.sh:92-96 first trims syntax-adjacent whitespace and then calls .strip() on s[1:-1] at line 95. Callers at lines 104, 112, and 120 therefore receive builder for the distinct quoted scalar " builder ", causing the removal predicates at lines 154, 174, and 187 to match.
   • Fix
     • Remove the inner .strip() from quoted-scalar handling (return s[1:-1]) while retaining only the outer trim needed to locate quote delimiters. Add regression cases for quoted keys, list items, and model.provider values containing significant edge whitespace.

   _{Ran code and verified through T-Rex}

2. scripts/uninstall.sh, line 51-54 (link)

   Escaped YAML builder references bypass cleanup

   Valid double-quoted YAML scalars such as "bui\x6cder" decode to builder, but the raw-text presence check reports Builder absent and exits before cleanup. Even if that guard is removed, _unquote only strips delimiters rather than decoding double-quoted YAML escapes, so provider keys, managed list entries, and model.provider values using equivalent escaped scalars do not match the Builder slugs. Uninstall leaves the plugin configuration behind.

   Artifacts

   
   

   Validation script for literal and YAML-escaped builder uninstall cases

   • This authored script creates isolated configurations, invokes the repository’s real uninstall script, and parses both configurations with PyYAML; it is the executable source of the comparison.

   Literal builder uninstall control log

   • The executed literal control shows uninstall removing the provider, three managed list entries, and model provider; it establishes expected cleanup behavior.

   YAML-escaped builder uninstall failure log

   • The executed escaped case shows PyYAML decoding every reference to builder before and after uninstall while the real script reports it absent; the entries survive.

   

   _{Ran code and verified through T-Rex}

   

3. General comment

   Uninstall misses YAML-escaped semantic builder references

   • Bug
     • A configuration containing valid YAML double-quoted "bui\x6cder" values is semantically equivalent to builder. The executed escaped case showed PyYAML reading builder for the provider key, enabled/plugin toolset lists, and model provider, while uninstall reported that builder was absent and left every item unchanged.
   • Cause
     • The script operates on raw text: its early guard searches for literal builder (scripts/uninstall.sh:51-54), and _unquote only removes quote delimiters without decoding YAML double-quoted escape sequences (scripts/uninstall.sh:92-97). Consequently, path and value comparisons never see the decoded slug.
   • Fix
     • Parse/decode quoted YAML scalar tokens before comparing plugin-owned keys and values, and replace the raw literal early-exit check with a detection path that recognizes decoded values. Preserve the existing comment/format-preserving line rewrite strategy if required.

   _{Ran code and verified through T-Rex}

_{Reviews (2): Last reviewed commit: "fix(uninstall): preserve inner whitespac..." | Re-trigger Greptile}