fix(analyzer): avoid false parse limits on quoted shell assignments

[deepujain]

Fixes #628.

A valid quoted shell assignment near the start of a longer file could make a benign skill report static_parse_limit: the sweep revisited its closing quote as a new command and consumed unrelated trailing content.

Successfully parsed words now own their closing quotes, including assignment values. Executable substitutions remain independent candidates, and a substitution's interior word stops at its surrounding quote. Comment-origin candidates cannot grant ownership to a later command. This preserves partial coverage for runtime-selected commands with destructive operands, including the two reviewer examples.

Validation: both reviewer examples fail on the previous head and pass after repair, including the analyzer ledger's PARTIAL/static_parse_limit outcome. The shell-assignment, documentation-reconstruction and adjacent static-pattern suites pass 462 tests. The original benign-padding, nested-command and failed-parse controls remain covered. Ruff lint and formatting pass. No shell fixture is executed and no provider-backed integration is claimed.

The conservative assignment check also applies to non-shell text: an unclosed name="value followed by enough content is partial, now pinned by a regression.

[rng1995]

[SkillSpector Review]

Hi @deepujain, thank you for your contribution to SkillSpector — we really appreciate the time you put into this! A few items need attention before it can be merged; details below.

This PR makes each successfully parsed candidate word record the positions of its closing quotes and of any $ inside double quotes; later candidates at those positions are skipped. It also parses a quote directly after = as a word. This removes the #628 triggers: a quoted $(...), or a (/| inside an assignment string, made the command-word sweep re-enter a closing quote and run to EOF.

I statically traced the head against main through _has_shell_command_word_exhaustion, _parse_shell_command_word, _bounded_shell_tokens, _shell_command_strings and _destructive_command_words. The issue's forms are fixed, and failed parses correctly claim nothing. However, owning the $ of a quoted $(...) also removes a fail-closed signal that main deliberately keeps. I did not execute contributor code, so the repros below come from static tracing and should be added as regression tests.

Findings

1. [Blocking] Runtime-selected destructive commands inside double quotes are now reported as complete (static_patterns_tool_misuse.py:1456-1457, skipped at :1745).

   Why main flags them. Main keeps a $(...) start as an independent candidate ("Only executable nested substitutions retain independent command positions"), and _bounded_shell_tokens treats the preceding quote as a wrapper. A quoted runtime command with root operands therefore returns exhaustion (static_parse_limit, partial).

   What changes here. The $ is owned by the enclosing string's parse and never evaluated, and nothing else covers it:

   • the enclosing word's own token check starts after its closing quote;
   • _destructive_command_words skips dynamic words;
   • _shell_command_strings doesn't recognize the clause.

   Static traces:

   • Markdown. SKILL.md containing Run "$(resolve_tool) -rf /". On main, the $ candidate yields tokens -rf / up to the wrapper quote, _has_destructive_root_path fires, and the result is PARTIAL. On this head the result is complete, with no findings, i.e. SAFE.
   • Shell script:

     # see "notes
     $(resolve_tool) -rf /
     # "

     On main, the stray comment quote's parse contains $(, so parsed_through does not advance and line 2 is still evaluated (PARTIAL). On this head, that mis-aligned comment-quote parse claims line 2's $ as a "quoted expansion start", and the result is complete.

   Why CI stays green. The $(printf ...), $($(resolve_tool)/printf ...) and "$(...)" -rf / variants still exhaust, either via limited or via tokens after the closing quote. Non-printf runtime helpers with operands inside the quotes do not.

   Expected fix.

   • Don't take ownership of $(/backtick starts inside double quotes. Instead, stop an interior candidate's word parse at its wrapper quote, as _bounded_shell_tokens already does, so it can't run past the closing quote. Alternatively, apply main's runtime-command operand check to the quoted span before claiming it.
   • Don't let a candidate that began inside a comment or another word's quoted literal grant ownership.
   • Add both repros as regression tests asserting PARTIAL / STATIC_PARSE_LIMIT.

2. [Non-blocking] assignment_quote changes behavior outside shell (:1752). Every =-adjacent quote in every file type (Markdown, Python, HTML, …) is now parsed as a shell word. A mis-paired quote after =, such as Python x='It\'s' or a prose typo like name="value, can now report exhaustion where main returned False; the new test already asserts x="unclosed" plus padding is partial. That is defensible fail-closed behavior, but please cover it in a non-shell file type and mention it in the description.

3. [Non-blocking] Test coverage. All new tests call has_bounded_parse_exhaustion with the default file_type="shell". Please add:

   • Markdown/SKILL.md cases;
   • an end-to-end repro of #628 (SKILL.md plus a padded scripts/run.sh) asserting complete coverage and no AE1;
   • the issue's comment form, # use "$(cmd)" here plus padding;
   • the repros from finding 1.

4. Overlap. These are complementary PRs, not duplicates.

   • #627 (approved) fixes a different root cause: Markdown contractions, and confining exhausted-span evidence to its command. Both PRs edit _has_shell_command_word_exhaustion, in separate hunks (roughly main lines 1728-1791 here vs 1810-1821 there), so they should merge cleanly. Whichever lands second should be rebased and re-run against the parse-limit suites.
   • #465 changes _next_shell_invocation_word/printf-invocation handling for bare $ARGUMENTS (#464) and does not touch this code. It is already conflicting with main: its  sentinel collides with main's _RUNTIME_SHELL_PARAMETER_SENTINEL.

5. FYI, pre-existing and out of scope. On main, a stray " in a comment already hides the parameter form $CMD -rf /. The mis-aligned dynamic word advances parsed_through, so # see "notes / $CMD -rf / / # " is reported complete on main. This deserves its own issue. The fix for finding 1 should not extend that gap to $(...) forms, which main currently handles.

Tests/CI: CI is green at e2080b2f (lint, test-unit, DCO, docker-smoke, OpenCode TS). According to the PR description, the positive tests fail on main.

---

Decision: Changes Requested (reviewed head e2080b2ff7438f56389bbb55bd58677e3d01df60)

[rng1995]

[SkillSpector Review]

Hi @deepujain, thank you for the quick turnaround on the first review and for sticking with this one. The quote-ownership approach is the right fix for #628, and it also unblocks a customer skill we were chasing internally. I re-reviewed 20bb62d against the earlier findings and pushed one maintainer commit, 11d1629, to close the gaps below. Please take a look and tell me if anything in it doesn't match your intent.

Status of the earlier findings on 20bb62d

1. [Blocking] quoted runtime commands. Mostly addressed. $/backtick starts are no longer owned, interior words stop at their wrapper quote, and both repros are PARTIAL with ledger assertions. However, ownership was only withheld from lines that start with #, and the new quote-after-= candidates could also claim a quote on a later line. Each of these is complete on 20bb62d but PARTIAL on main:
   • trailing comment: ls # see "notes / "$(resolve_tool)" -rf / / # "
   • Markdown prose: Set name="value in prose. followed by a fenced "$(resolve_tool)" -rf / and a later "
   • host source: x = f(name="value) / "$(resolve_tool)" -rf / / "
   • prose parameter: Set name="value / $T -rf / / "
   • same line: name="a "$(resolve_tool)" -rf /"
2. [Non-blocking] non-shell behavior. The description note is there, but the new test used the default file_type="shell".
3. [Non-blocking] coverage. The Markdown ledger case is there. The end-to-end #628 bundle and the # use "$(cmd)" here comment form were missing. The comment form was also still partial, because comment lines could claim nothing at all.

Two more regressions against main, found while testing a customer skill

• x="$(printf '%s' "$X")" and model_count="$(printf '%s\n' "$j" | jq -r 'length')" are complete on main but partial here. The quote after = became a candidate and was then checked as a possible runtime-built command.
• A documented `X="${X:-default}"` in a Python docstring: the quote-after-= word treated the span's closing backtick as a new substitution and consumed the rest of the file.

What 11d1629 changes (static_patterns_tool_misuse.py)

• A word that starts in a comment (line-start or trailing #), or at a quote after =, may only claim quotes on its own line. Same-line claims are allowed, which fixes the #628 comment form.
• A quote followed by $ or a backtick is never claimed, since it opens a runtime-selected word, so a mis-paired claim can't hide its operands.
• A quote-after-= candidate only claims ownership. It skips the printf and outer-operand checks, because an assignment value never names the command and main never parsed that position. A multi-line one also doesn't advance parsed_through.
• When a backtick or quote directly encloses the assignment name, the word stops at that delimiter and owns the closing backtick.
• The blanket "line-start comments claim nothing" rule is replaced by the rules above.

Tests added (test_shell_assignment_quotes.py, now 31 tests): the unclosed-assignment test across shell, Markdown and Python; the #628 comment form, a Markdown fence and both printf assignment values with padding; three backtick-enclosed assignment cases with apostrophe padding; the five mis-paired ownership cases above; and an end-to-end #628 CLI bundle asserting complete coverage, no ledger exceptions and no AE1. The 11 targeted tests fail on 20bb62d and pass on 11d1629.

Validation

• This head merged with current main (ec6633e): full non-integration suite 8434 passed, 14 skipped, 4 xfailed. Ruff lint and format pass.

• Fail-closed parity: every probe above, plus the original review repros, matches main's PARTIAL result. One intentional difference is inherited from 20bb62d: echo "$(resolve_tool)" " -rf /" is now complete, which is shell-correct, because " -rf /" is a single quoted argument. main only flagged it through quote mis-pairing.

• Customer skill (skills/vss-build-vision-ai from NVIDIA-AI-Blueprints/video-search-and-summarization#2386, --no-llm):

  Build	Partial files	AE1
  main	16	35
  20bb62d	9	5
  11d1629	8	4
  11d1629 + main + #686	5	0

  The last 4 AE1 on this PR alone come from ${VAR:-default} inside backticks, which #686 fixes. The two PRs merge without conflicts and their test files pass together.

Non-blocking, out of scope

• The 5 remaining partial files come from prose apostrophes and non-shell quoting in YAML, logstash and Python string literals. They are tracked separately.
• Pre-existing and unchanged from main: a trailing-comment quote followed by "/usr/bin/${T}" -rf / is complete on both.
• The branch is 50 commits behind main but has no conflicts.

---

Decision: Approve (reviewed head 11d1629cc394c9fd67c108fdb3021e41047cb70b)