fix(work-items): make run-conformance's provider check non-vacuous

[kyle-sexton]

Closes #3485

Summary

run-conformance.sh asserted "provider=$PROVIDER" contains "provider=", which is always true. An adapter that returned an empty provider would pass conformance undetected. The e2e-probe half of this issue is already fixed on main.

Fix

The provider read back from WIT_OUT via jq must be non-empty and not the literal null, matching the suite's existing holder-field check. Plugin bumped to 0.39.44.

Verification

scripts/affected-tests.sh --run passed 6 suites, including real conformance runs against jira, linear, and local-markdown. A throwaway binding with provider: "" now fails this case.

Related

N/A

  

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-02T06:23:01.094534Z	2166e79	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 32s —— View job

---

Security review — no issues found

• Gather context (PR diff, changed files)
• Apply skip gate
• Review changed files for security-relevant issues
• Report findings

This PR only touches plugins/work-items/tools/work-item-tracker/conformance/run-conformance.sh (test-assertion logic), plus a version bump in plugin.json and a CHANGELOG.md entry. The fix replaces a vacuous string-containment assertion ("provider=$PROVIDER" contains "provider=", always true) with a proper non-empty/non-null check on $PROVIDER, matching the existing pattern used later in the same suite for the claim holder field (run-conformance.sh#L124-L128).

No new inputs are introduced, no injection surface (command/path/template), no secrets/credentials/tokens, and no GitHub Actions workflow changes. $PROVIDER is only used in shell string comparisons here, not interpolated into a command. This is a self-contained test-harness correctness fix with no security-relevant surface — skip gate applies (criterion 2).

No security issues found.

[github-actions]

Last security-reviewed head: 2166e799dada17431c3ed1f39e87946835b11f54. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2166e799da

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 1m 16s —— View job

---

Claude finished the code review

• Gather context (PR diff, changed files, CLAUDE.md guidelines)
• Understand the request (invoke /review:code-review)
• Review changed files for correctness/maintainability
• Post findings

Summary

Reviewed the 3-file diff against origin/main per the CI code-review lane (/review:code-review). No findings to report.

The fix in run-conformance.sh lines 121-129 is correct:

• The original assert_contains "capabilities names a provider" "provider=$PROVIDER" "provider=" was indeed vacuous — the haystack is constructed from the needle itself ("provider="), so it's a tautology that passes for any value of $PROVIDER, including empty.
• The replacement if [[ -n "$PROVIDER" && "$PROVIDER" != "null" ]]; then pass ...; else fail ...; fi correctly gates on a non-empty, non-"null" provider string.
• This mirrors the existing claim-holder check a few dozen lines later (lines 301-305), so the style is consistent with the file's own conventions rather than introducing a new pattern.
• pass/fail are called with the correct signatures per tests/lib.sh (fail <label> <expected> <actual>), and CASE_NUM/FAILED bookkeeping is preserved identically to what assert_contains did, so the suite's case count is unaffected.
• Version bump (0.39.43 → 0.39.44) and CHANGELOG entry are consistent with the plugin's conventions and mirror the prior 0.39.42 fix for the analogous e2e-probe.sh vacuous-assertion bug — no stale version references found elsewhere in the repo.

Not independently verified: the PR body's claim that scripts/affected-tests.sh --run passed 6 suites including live conformance runs against jira/linear/local-markdown — I did not have execution access (shellcheck/test invocation required approval not available in this run) to re-run that, so it's author-claimed and unverified by me. The logic itself checks out from static review.

[github-actions]

Claude has reviewed this PR 3 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 2m 12s —— View job

---

Claude finished the code review

• Gather context (PR diff, changed files, CLAUDE.md guidelines)
• Invoke /review:code-review and follow its procedure
• Review changed files for correctness/maintainability
• Post inline findings and summary

Summary

Reviewed the diff at 4e375a7 against origin/main per the CI code-review lane. This is a new head since the last review from this lane (which covered 2166e79); the second commit here (4e375a7) addresses the Codex reviewer's earlier finding that a provider literally named null would be wrongly rejected, by switching from a raw-text null check to a JSON-type check. That fix is correct on its own terms, but it introduces one new issue in how it does so.

1 finding, posted inline:

• run-conformance.sh lines 127-133 — On the fail branch, PROVIDER is captured with jq -c instead of jq -r. For a .provider value that's a valid-but-empty string (or any non-string type), this leaves PROVIDER holding literal JSON syntax (e.g. "" — verified with jq -c locally) instead of raw text. $PROVIDER is reused unescaped later as a path segment (line 167) and inside hand-built JSON (lines 168/178), so the embedded quotes/brackets corrupt the synthetic manifest used by the skew-handshake cases, turning one clear failure into a cascade of unrelated jq parse errors. Suggested a single jq -r assignment for PROVIDER (matching pre-PR behavior) with the diagnostic value for fail captured separately.

Everything else checks out:

• plugins/work-items/.claude-plugin/plugin.json version bump (0.39.43 → 0.39.44) is consistent.
• plugins/work-items/CHANGELOG.md entry accurately describes the fix, including the null-provider-name nuance.
• pass/fail call signatures match tests/lib.sh, and fail doesn't abort the run (by design — "run all cases" discipline), which is what makes the cascading-corruption scenario above reachable rather than short-circuited.

Not independently verified: the PR body's claim that scripts/affected-tests.sh --run passed 6 suites including live conformance runs against jira/linear/local-markdown — author-claimed, not re-run by me here.

Branch

[claude]

Claude finished @cursor[bot]'s task in 2m 25s —— View job

---

Claude finished the code review

• Gather context (PR diff, changed files, CLAUDE.md guidelines)
• Invoke /review:code-review and follow its procedure
• Review changed files for correctness/maintainability
• Post inline findings and summary

Summary

Reviewed the diff at 47bc9a1 (3 files) against origin/main per the CI code-review lane. This head adds one commit past the last review (4e375a7 → 47bc9a1), which fixes the correctness bug flagged in that review.

Prior finding resolved: the earlier review found that on the fail branch, PROVIDER was captured with jq -c instead of jq -r, which for an empty-string or non-string .provider would leave PROVIDER holding literal JSON syntax (e.g. "") instead of raw text — corrupting the synthetic manifest reused later as a path segment and inside hand-built JSON (skew-handshake cases). run-conformance.sh lines 127-135 now uses jq -r '.provider' on both branches to set PROVIDER, and reserves the jq -c call strictly for the diagnostic value passed inline to fail(). This matches the suggested fix and removes the corruption path.

No new issues found on this pass:

• The type-based check (jq -e '.provider | type == "string" and length > 0') correctly parses as (type == "string") and (length > 0) under jq's operator precedence, so it rejects empty strings and JSON null while accepting a provider literally named "null" — matching the intent described in the code comment and CHANGELOG.
• pass "capabilities names a provider" / fail "capabilities names a provider" <expected> <actual> call signatures match tests/lib.sh's pass(label) / fail(label, expected, actual).
• Version bump (0.39.43 → 0.39.44) and CHANGELOG entry are accurate and consistent with the fix.

Not independently verified: the PR body's claim that scripts/affected-tests.sh --run passed 6 suites, including live conformance runs against jira/linear/local-markdown — author-claimed, not re-run by me here (no execution access in this session).
· Branch