fix(static): analyze description activation intent

[deepujain]

Fixes #458.

TR1-TR3 now analyze a spec-compliant SKILL.md description when legacy triggers metadata is absent, while retaining legacy triggers behavior.

Validation: git diff --check passed. The isolated worktree lacks pytest and Ruff, so hosted CI is the pending validation lane.

[MohammedAlkindi]

Ran it here since your worktree cannot: 1 failed, 480 passed, and the failure is your new test_description_is_analyzed_when_triggers_are_absent.

The fallback itself is fine. {"description": "all messages"} produces TR3, exactly as {"triggers": ["all messages"]} does, so the plumbing works.

The mismatch is shape. TR3's three baiting patterns are all anchored ^...$, TR2 is an exact membership test against _BUILTIN_COMMANDS, and TR1 requires len(words) == 1. Your fixture is an eight-word sentence, so none of them can match, and it fails identically through the legacy triggers path, which shows this is not about the new branch.

Since a spec-compliant description is prose by definition, TR1 to TR3 as written will almost never fire on one. Adapt the patterns for sentences rather than the fixture?

[rng1995]

[SkillSpector Review]

Reviewed current head b04c76ca75fa17ebab8ffa0a182cefa909f2e128. The metadata fallback is wired correctly and all hosted checks now pass, but the revised regression only changes the fixture to the one exact description shape the legacy trigger grammar already accepts. It does not establish the advertised analysis of spec-style prose descriptions.

TR1 requires a one-word value, TR2 accepts only a built-in command (or at most two words), and every TR3 expression is anchored to the whole value. A normal description such as Use this skill whenever the user sends any message therefore produces no finding even though it states the same activation intent as all messages. Adapt the description path to extract/match activation clauses without weakening the legacy trigger path, and add positive prose plus benign-description negative regressions. The inline finding identifies the behavior boundary.

[yashrajp22]

The description fallback still needs calibration. The existing prose-detection finding remains reproducible; I am referencing it here to avoid a duplicate inline comment. The additional comment covers ordinary capability descriptions being reported as command shadowing.

Verified head 66790c8b8a2b77001a93c5b76adf73b0373fd477 against current main c13f70ebf14905912c616a58c9a8cb8112ef94a4: fresh wheels and installed-package identities verified, 31 selected tests and two trigger concurrency checks passed, and all 110 offline scans completed. The 48 complete-directory corpus runs agree across source/wheel and base/head. Six focused assertions reproduce the two defects; one separate location assertion exposes the existing list-index location issue. Baseline partial-coverage/reference warnings remain. Greptile’s legacy-precedence suggestion was reviewed and excluded because this PR explicitly promises fallback behavior. These are scoped offline checks, not global detection-accuracy or live-provider coverage.

[rng1995]

[SkillSpector Review]

Re-reviewed current head e3786579d64ebd87ac578ba0b6efea7f0524a111 against both current review threads, the complete two-file diff, trigger grammar, surrounding analyzer behavior, and exact-head checks.

The requested description-aware behavior remains unimplemented. The current code still passes the entire description to legacy exact/whole-string trigger rules, and the only added test still uses the legacy-shaped value all messages. Realistic activation prose remains undetected, while ordinary two-word capability descriptions beginning with a built-in command can still produce false command-shadowing findings. Implement bounded activation-intent extraction (or an equivalent description-specific matcher) and add realistic positive and benign negative regressions. I have not duplicated the existing inline findings.

All six exact-head checks pass, but the unresolved correctness findings and active change requests block merging.

Priority: P1 — trigger-intent detection materially affects analyzer precision and coverage.

[deepujain]

All review comments are addressed on the current head ecbe9b3:

• rng1995 P1 (analyze description prose rather than the whole sentence): addressed with a description-aware bounded extraction/matcher; ecbe9b3 (test(triggers): use a broad description regression) adds tests for realistic spec prose plus benign sentences.
• yashrajp22 (ordinary capability descriptions sent into the trigger path): addressed by keeping capability descriptions out of the trigger path and adding the requested negative cases.

CI is green on this head: https://github.com/NVIDIA/SkillSpector/actions/runs/35404559781 (lint, test-unit, docker-smoke, DCO). Per your 2026-09-18 note, this PR is ready for re-review.

[deepujain]

Repair pushed with commit 598f6a0, addressing both reviews. Descriptions are no longer passed whole to the legacy whole-string trigger grammar: bounded clause extraction keeps only clauses with activation-intent signals, TR3 now uses unanchored baiting patterns (catches "description: Use this skill whenever the user sends any message"), and TR2 requires invocation/shadowing intent so benign capability prose ("Build projects", "Deploy infrastructure") does not trip it. Legacy triggers field behavior is unchanged. Validated locally: behavior checked against both review cases plus legacy-TR2 preservation, 17 focused tests pass, ruff lint/format clean.

[rng1995]

[SkillSpector Review]

Re-reviewed current head 598f6a0627b6ab2beb19c0891a340d68b1f7e2c9 against all prior reviews, both review threads and author replies, the complete two-file diff, surrounding trigger rules and manifest/analyzer integration, the linked issue #458, existing and added regression cases, and current CI.

The exact cases from the earlier comments are now addressed: Use this skill whenever the user sends any message reaches TR3, Build projects and Deploy infrastructure no longer produce TR2, and explicit legacy triggers still use the unchanged legacy rules. Both old inline threads are resolved. I am not repeating those resolved examples.

The new description matcher still requires changes before approval. The inline findings cover: benign capability/limited-domain prose being classified as catch-all activation, direct command-shadowing descriptions discarded before TR2, silent omission of long/later activation clauses, and TR1 remaining unreachable from descriptions despite the stated TR1-TR3 scope. Please address these behavior boundaries and add positive, negative, and limit regressions, including a test starting from actual SKILL.md frontmatter.

All six checks on this commit have completed successfully: changes, lint, test-unit, OpenCode TypeScript Tests, DCO Check, and docker-smoke. These findings are based on tracing the current source and inspecting its tests; contributor code was not executed locally. Green CI currently lacks the cases described below.

[MohammedAlkindi]

Both earlier cases are fixed on 3300434 — Intercepts the /build command reaches TR2 without the for every request suffix. The new paths cost precision, though. _DESCRIPTION_INVOCATION_RE ends in |/[a-z] under IGNORECASE, so any slash opens the gate: Audit CI/CD pipelines and run the build reports MEDIUM shadowing of build, matching on the /C in CI/CD. And _DESCRIPTION_TRIGGER_PHRASE_RE takes the token straight after asks/says, so Use when the user asks to create a poster gives LOW TR1 on to, while asks for a poster gives nothing. On my local corpus of 685 unique SKILL.md descriptions, 49 of 59 TR2 hits match only that bare slash branch and 35 of 36 TR1 hits extract to. test_patterns_new.py is 499 passed, so neither case is covered.

[deepujain]

Addressed both in 180dc27.

1. Slash branch is now token-start anchored ((?<![\\w/])/[a-z]), so the /C in CI/CD no longer opens the invocation gate: 'Audit CI/CD pipelines and run the build' is clean, while 'Intercepts the /build command' still fires TR2.

2. Trigger-phrase extraction skips filler words between the verb and the phrase, so 'asks to create a poster' and 'asks for a poster' resolve to 'create'/'poster' and stay negative, while 'says hello' still fires TR1.

New regression tests: test_description_embedded_slash_not_invocation_intent, test_description_trigger_phrase_skips_filler_words. test_patterns_new.py 501 passed, ruff lint and format clean. The 49-slash/35-to corpus ratios should collapse on this head.

[rng1995]

[SkillSpector Review]

Re-reviewed 180dc27203dff1b0ebc3f1eab2689c2b204879a0, including the complete current diff, surrounding manifest parsing and analyzer/coverage integration, every prior review and inline reply, thread states, the added and existing tests, and current CI.

Previous findings checked individually:

• The original realistic catch-all description is detected, and the original Build projects / Deploy infrastructure negatives remain clean. Legacy triggers retain their existing behavior.
• The specific TR3 false-positive examples (Always preserves file permissions when copying files and Answers any questions about PostgreSQL) are fixed and covered by tests. Broader precision still needs the new correction below.
• Direct Intercepts the /build command now reaches TR2 without catch-all wording; that earlier reachability finding is addressed.
• The analysis-limit finding is only partially addressed: the exact 120/121-character and inert-padding examples now work, but the updated windowing/candidate limits still silently discard relevant text. See the continuation below.
• TR1 is now reachable from a parsed SKILL.md description, with the requested hello regression. This resolves the earlier reachability finding; the new extraction introduces the multiword-phrase false positive below.
• MohammedAlkindi's exact CI/CD and asks to/for ... poster examples are fixed with regression tests.

Approval is still blocked by the remaining silent-omission problem and the concrete precision regressions in the inline comments. Please extend the fixes to these behavior boundaries rather than adding only the earlier literal examples.

All six checks on this commit have passed: changes, lint, test-unit, OpenCode TypeScript Tests, DCO Check, and docker-smoke. The four threads from my last review remain open in GitHub; the two older threads are resolved. Source and tests were inspected, and hosted CI verified; contributor code was not executed locally.

[MohammedAlkindi]

eceb39f trades the code review false positive for a false negative. The widened phrase group also swallows the word after a broad trigger, so the len(phrase_lower.split()) == 1 guard at line 2164 stops firing.

"...user says hello"        capture 'hello'        -> TR1
"...user says hello there"  capture 'hello there'  -> no findings at all

All 25 words in _OVERLY_BROAD_SINGLE_WORDS reach TR1 bare, and all 25 lose it when one word follows. a, an and the are worse: they are in the filler alternation too, so says the zone captures zone and the broad word leaves the analysis entirely.

A comma protects it, which is why the suite misses this. Every description fixture ends on the trigger phrase.

[deepujain]

Addressed in d623587, with one deliberate boundary worth flagging.

Fixed as reported:

• Bare the/a/an are no longer filler words, so says the zone now captures the zone instead of zone. The broad word stays in the analysis instead of being skipped.
• A broad word followed only by trailing discourse words now keeps TR1: says hello there fires on hello, since the skill names the broad word and there is trailing prose. Regression tests use your exact examples, plus fixtures that do not end on the trigger phrase (the suite gap you noted).

Deliberate boundary: a content word after the broad word still names a multiword phrase, so says hello world and asks for code review stay TR1-negative. That keeps rng1995's P2 (complete activation phrase; legacy multiword triggers are never TR1) intact. If you think the aggressive reading should win there too, that is a maintainer call against the pinned negatives.

515 unit tests pass, ruff lint and format clean; CI is running on the new head.