feat: adapter track (onboarding generator, gitea/forgejo, linear) + Lane C spin-off closures

[kyle-sexton]

Closes #3046
Closes #3047
Closes #3048

Summary

Two tracks on one branch.

The adapter track — the container's last three sub-items (#2950, #2952, #2946) plus the SSOT verdict reconciliation: a generator that lets a consumer onboard a tracker this plugin does not bundle, the Gitea/Forgejo adapter generated through it as the designated dogfood, and a hand-built Linear adapter with full verb parity. None of those three issues is closed by this PR, deliberately — each one's acceptance criteria say "passing conformance", and no live tracker instance is reachable from this environment. See "The blocker, stated plainly" below.

The Lane C spin-off closures — #3046, #3047 and #3048 were filed as deferred follow-ups during Lane C. They are worked to completion here rather than carried, and this PR closes all three.

Shipped versions: work-items 0.39.0, review 0.24.0, session-flow 0.30.0, implementation 0.14.0, claude-ops 0.35.0, skill-quality 0.17.2.

Fix

#2950 — adapter-onboarding skill (work-items 0.37.0 → 0.39.0)

work-items:onboard-adapter — interview → live exploration → generate → verify. The generator (scripts/generate-adapter.sh) emits a hardened skeleton from templates rather than re-deriving security posture per provider: token via env-name indirection and never argv, host validation, deny-by-default egress — the jira adapter's guards as the starting point. It also emits the conformance/bindings/<name>.sh the suite needs, so a generated adapter is verifiable rather than merely written.

Split by the interview's own constraint: the deterministic parts are scripted, and the parts that need reasoning — semantic mapping of provider states onto the normalized envelope, the interview itself — stay outside the script.

#2952 — Gitea/Forgejo adapter

Generated through onboard-adapter as the dogfood. Its manifest is the honest part: add-sub-item, list-sub-items, claim, reclaim, and renew-lease are all declared false with sub_item_depth: 0, because Gitea has neither sub-issues nor a lease surface. No verb script exists for any false verb — the contract's declared-false-exits-6 gate handles them, so nothing is stubbed or faked to look supported.

#2946 — Linear adapter

Hand-built as a bundled "major" under the hybrid shipping model. All ten verbs declared true, each with an implementing script and a .test.sh — full parity, including the claim/renew/reclaim lease protocol.

One design note worth surfacing: wit_linear_epoch normalizes fractional seconds off a timestamp before parsing it. That is not tidiness — Linear returns both …:00Z and …:00.500Z, BSD date -f demands an exact format match and fails on the fractional form, and an earlier version of this shape is recorded in-comment as having "silently seen no activity at all". The lease's activity check depends on it.

#3046 — tautological-expectation criterion in the review code lens (review 0.24.0)

The anti-pattern was covered in prose (tdd's anti-patterns-khorikov.md, testing's write.md checklist) and was claimed to also be covered executably by testing:audit's cant-fail-scan.sh. That claim was false, and the scanner concedes it in its own header: rule-recomputed-expectation "detects the decidable core — textually identical sides — not every recomputation shape." A validator ran it over three canonical tautological tests for zero findings — the canonical Khorikov shape (compute expected with the production algorithm in the arrange section, then assert against it) has non-identical sides and does not fire.

The new Code-quality bullet in review/agents/code-reviewer.md asks the question that decides it: what is the expected value's independent source? The round-trip/identity case rides in the same criterion, matching how write.md:78 already pairs them. It cedes ground to the scanner by name rather than overlapping it, per the plugin's existing skip-what-tooling-enforces posture — where both sides are the same expression, the scanner owns the finding. Widening the detector past textually-identical sides is explicitly not in scope: the general shape is undecidable.

Placement went to the agent definition rather than quality-gate/context/criteria.md, because that file is a routing doc — it resolves the project's standards index and carries no criteria of its own.

#3047 — the pre-PR ordering conflict, settled (session-flow 0.30.0, implementation 0.14.0)

pre-pr.md declared its step order "fixed plugin identity … there is no seam to reorder it," while a sibling plugin was reordering it at the handoff point: implement's step 5, titled "Hand off to the pre-PR sequence," prescribed outcome verification before review.

pre-pr.md's order is doctrine. Steps 4–6 (simplify, review the simplify diff, re-test) mutate the diff between review and verification, so a verdict rendered before them describes code that no longer exists by step 8 — and the simplify edits then ship carrying an outcome claim nothing tested them against. The competing reading ("confirm it works before spending review effort") is already served earlier, by step 1 and by implement's own build check and full test pass.

This was a one-surface correction, not a coin flip. verification's own chaining table (skills/confirm/SKILL.md:125,128) already fires on "review gate passes (no blocking findings)" → suggest /verification:confirm, and suggests the PR flow only after a CONFIRMED verdict. The skill that renders the verdict, the skill that lists the sequence, and the plugin that opens the PR all agreed; implement's handoff step was the lone dissenter.

Landed: a new owner doc docs/conventions/pre-pr-ordering/ plus a registry row in PLUGIN-PHILOSOPHY.md — the registry's own trigger ("a new cross-plugin convention lands in an owner doc before a second plugin adopts it") had already fired. pre-pr.md keeps ownership of what each step does and cites the owner for the order; its override-boundary paragraph is corrected from "fixed plugin identity" to fleet identity, since the seam it denied was being exercised by a sibling. implement/SKILL.md's two sites now read review → verify → PR with every presence gate and fallback intact.

#3048 — the SkillUse measurement, taken (claude-ops 0.35.0)

skill-pair-cooccurrence.sh is the repeatable reading. Placement corrects the filing: the item proposed observability, but that plugin's own context/read-routing.md already assigns interpretation of skill-usage data to audit-skill-visibility and keeps only the store, the pipeline, and retention.

It is a proxy and refuses to be read as more. The SkillUse record carries no caller attribution — a PostToolUse hook on the Skill tool receives tool_name, tool_input, and tool_response, and nothing names the skill whose instructions caused the call. So it observes only that both skills fired in the same (project_id, branch) group, ordered by timestamp. The caveat prints in both renderers, prose and --json, because a machine consumer stripping it is the same defect as a human not seeing it. It inherits the skill's refusal: below the 30-day exposure floor (the same constant audit_skill_visibility.py uses) or below a minimum denominator it returns WITHHELD with a reason.

The result, run against the only store reachable here (.claude/observability/skill-usage.jsonl — 17 events, 2026-08-17 → 2026-08-20): implementation:implement fired zero times. The denominator is empty, so the verdict is WITHHELD, not 0% — an empty denominator is a population that was never observed, not a rate of zero. #2936's premise stays UNVERIFIED, now with an instrument attached rather than a hand-wave.

The schema-widening question is decided here rather than filed: do not widen the record. Caller identity is absent from the hook's input, not merely its schema; recovering it would mean reading the session transcript, which turns a bounded telemetry hook into a conversation reader and crosses the boundary observability/context/privacy.md guards. The better signal needs no change and already ships — OTEL's claude_code.skill_activated carries invocation_trigger, separating user-slash from claude-proactive, which is the axis the premise actually asks about.

Merging main, and a defect that surfaced doing it (skill-quality 0.17.2)

Six commits landed on main while this branch was open, three of which bumped plugins this branch also bumps — to the same numbers. Resolved by stacking this branch's entry above main's released one: claude-ops 0.34.0→0.35.0, session-flow 0.29.0→0.30.0, skill-quality 0.17.1→0.17.2.

The two plugin.json conflicts were not the version field. Bumping with jq had rewritten each whole file, re-encoding main's — escapes as literal UTF-8 em-dashes in the description string — an incidental encoding change riding along in a version bump. Resolved by taking main's exact bytes and editing only the version line; review and implementation were then verified to differ from main by the version line alone.

Also caught here: main's audit-skill-visibility/SKILL.md sat at exactly the 200-line soft target, so this branch's inline pair-cooccurrence section pushed it to 234 and tripped a warning. The detail moved to reference/pair-cooccurrence.md — the progressive-disclosure spoke the check recommends — with the pointer folded into an existing scope-boundary row rather than added as a new one, landing back at 200 with 0 errors and 0 warnings.

One incidental win: main's mawk fix (#3058) cleared the 21 assertion failures check-skill.test.sh had been reporting in this tree. It now passes fully.

SSOT reconciliation

Lanes F (#2938) and Y (#2948) were the last two OPEN verdict cells; both filled from each item's closing PR and design comment. Lane C's C10, chain-doc, and premise bullets now record the three closures above.

Review findings fixed on this PR

Six review rounds across two lanes. Every finding is closed; the security lane's final pass at ffc25be reports no new findings and lists each of these as independently re-verified against the source rather than against the commit message.

Security (all confirmed before fixing, not taken on assertion):

• Command injection in the generator — api.scope_pattern was validated only for anchoring and api.sample_scope only for matching it, then both were rendered into single-quoted shell. Confirmed by building the exploit and running it end to end: the generator accepted the spec, emitted readonly …_SCOPE_RE='^';touch …;#'$' (valid shell, bash -n clean), and sourcing it — which every generated verb does — executed the command. Fixed at render(), the one choke point every value passes through.
• display_name reached three dangerous contexts, not one — the first guard covered only the single-quoted case. A double-quoted ${VAR:?…} expands $(…) with no quote to break, and a newline ends a # comment line. Constrained at validation instead, where every neighbouring spec field already is.
• Linear reclaim.sh could strip a live claim — revalidate() filtered the re-read by the original handle, so a concurrent claimer's new lease (different handle) was structurally invisible; and the assignee came from a snapshot taken before every round trip. Both closed, mirroring what claim.sh already did.
• host_suffix was a string suffix, not a domain boundary — a pin of mycompany.com was satisfied by evilmycompany.com, sending the credential to an attacker host without tripping the loud allow_custom_domain opt-out. Fixed in the generator template as well as both adapters, so future generated adapters inherit the fix. The apex is accepted separately, since under a pin of mycompany.com the host mycompany.com is the pinned domain.

Correctness:

• reclaim.sh activity check read only the first page of comments while its sibling helper paginates exhaustively. Linear returns comments oldest-first, so the comments proving a holder is alive are on the last page — the check could not see recent activity at all on a busy item.
• Gitea README marked four fully-implemented verbs as unfinished scaffolds; label pagination had no ceiling (and the ceiling had to qualify the "label not found" message, since truncating there turns "didn't page far enough" into a false "doesn't exist"); the basic-auth conformance binding baked auth_user to a placeholder with no override; the Linear binding's scope comment described GitHub's shape.
• A latent template bug surfaced while verifying the above: the generated test asserted a hardcoded id number of 12, while the id comes from the configurable api.sample_id. Both shipped specs happen to end in #12, so it never showed — but any consumer with a different sample id got a failing test the moment their adapter was generated, from a template that promises the opposite.
• A defect in the new co-occurrence script, caught by its own test: the header promised a malformed row would cost only itself, while jq -s failed the whole file on the first unparsable line. The read is now jq -Rn with fromjson?.

Every fix that could be tested carries a regression test, and each new test was confirmed to fail without its fix rather than merely pass with it.

Verification

Re-run on the merged tree at ffc25be:

• 59/59 adapter, binding, conformance, generator, and co-occurrence test files pass (33 cases in the new co-occurrence suite alone; its two central guards were checked by deleting them and confirming the relevant cases go red)
• scripts/check-shell-portability.sh origin/main → No unexcused GNU-only constructs in 57 shell file(s). Three date -u -d sites in the Linear adapter carry per-site portability-ok: markers with reasons — they are genuine dual-dialect ladders with BSD -j -f fallbacks, and the linter seeds ladder guards only for readlink -f and stat -c, so date has no guard class to recognize them
• shellcheck --rcfile=.shellcheckrc over every changed shell file → exit 0
• check-skill.sh → PASS, 0 errors for onboard-adapter, audit-skill-visibility (0 warnings), implement, and workflow. implement's single warning is a pre-existing 219-line soft-target notice; its line count is unchanged by this PR
• markdownlint-cli2 over all changed Markdown → 0 issues
• check-changelog-parity.sh --check, check-fleet-audit-doc-grammar.sh, check-fleet-finding-test-coverage.sh, check-cross-plugin-source-drift.sh, check-silent-skips.sh, check-discriminating-test-skips.sh → all clean
• The template changes were verified by generating an adapter and running its generated tests, not by asserting on template text — which is how the two-consumer bug in SAMPLE_AUTH_EXTRA and the hardcoded-id-number bug were caught

One note on a deliberate skip: generate-adapter.test.sh probes whether the local ShellCheck accepts --rcfile (0.10.0+) before using it, and skips loudly otherwise. The skill-quality-gate job installs no tooling and gets the runner's older build, which rejects the flag and exits 3 — "invoked with bad syntax", not "issues found" — which the assertion was previously misreading as a lint failure.

The blocker, stated plainly

The three adapter items share one external dependency: no live tracker instance of any kind is reachable from this environment — no gh, no Linear workspace, no Gitea host. What is verified here is offline: mocked-transport unit tests against each adapter's own mock.sh, plus honest capability manifests. What is not verified is a live conformance pass, which is what all three acceptance criteria actually ask for.

This follows the precedent the jira adapter set — ship the adapter, defer the live pass, record the deferral — and it is why this PR closes none of those three. Closing one here would assert a criterion that has not been met. The close-out dry run posted to #2933 caught exactly this and refused to close them too.

To unblock: a reachable throwaway instance for any of the three. Then conformance/run-conformance.sh --binding {gitea,linear} against it, close the corresponding item, and the container reaches 20/20.

Related

• Refs Spec: Shipping-lifecycle absorption (Pocock Shipping section) #2933 — spec container, currently 17/20 and staying there on merge; the close-out dry-run comment on that issue has the full rollup and the derived commit basis
• Refs Adapter-onboarding skill — generator for the provider tail #2950, Gitea/Forgejo adapter — generator dogfood candidate #2952, Linear adapter with full verb parity #2946 — the three adapter items, each remaining open pending its live conformance pass
• Refs Lane C: TDD wiring + zero-assembly default chain #2936 — Lane C, whose three spin-offs this PR closes
• Refs Seam: contract hygiene — dangling ADR 0022, version handshake, label-default SSOT #2942 — the contract-version handshake and normalization-fidelity check this generator was gated on
• Refs Investigation: multi-provider topology — source-of-record + coordination split #2945 — the multi-provider topology decision (one writable coordination provider, N read-only sources) that shapes how a Linear binding pairs with a GitHub one

🤖 Generated with Claude Code

https://claude.ai/code/session_01CnzwTKoTa6xNY7iyEzMYpm

[claude]

Claude finished @kyle-sexton's task in 15m 55s —— View job

---

Security review complete

• Gather context (PR diff, changed files)
• Apply skip gate checks — PR open, security-relevant surface (token/credential handling, egress control, generator templating, lease/concurrency logic), no prior review found for this head → proceeded with full review
• Review generator (generate-adapter.sh + templates) for injection risk
• Review gitea adapter (auth, host validation, scope checks)
• Review linear adapter (auth, GraphQL construction, lease/claim protocol)
• Review CONTRACT.md / conformance bindings / dispatcher changes
• Adversarially verify candidate findings against source (producer ≠ verifier)
• Post final findings

Scope: logic flaws, trust-boundary/authorization gaps, and injection surfaces only, per this repo's CI security-review lane (/review:security-review) — zizmor's static-analysis lane already covers unpinned actions, dangerous triggers, and excessive permissions, so those aren't re-reported here. Three findings survived independent verification against the actual source (not just the diff); each is posted as an inline comment anchored to its line.

CRITICAL — spec.display_name shell injection in the adapter generator

plugins/work-items/skills/onboard-adapter/scripts/generate-adapter.sh#L138-L139

Every other spec field (provider, host_suffix, base_path, scope_pattern, auth_scheme, sample_*) is constrained by an anchored regex or enum specifically so render() can substitute it with zero escaping ("Constrain it ONCE, here, so nothing downstream has to escape it" — the generator's own stated design principle). display_name is checked only for non-emptiness, and it lands unescaped inside: a single-quoted printf literal (templates/common.sh.tmpl#L222, a ' breaks out), a double-quoted ${VAR:?msg} expansion where bash still runs command substitution (templates/conformance-binding.sh.tmpl#L33, no quote-breakout even needed), and several #-comments (a newline turns the remainder of the value into live shell). The skill's own documented Step 4 has the agent run bash common.test.sh immediately after generation — so this fires before any human reviews the generated file. See inline comment for full detail.

IMPORTANT — Linear reclaim.sh uses a stale pre-round-trip snapshot, letting it strip a concurrent claimer's live assignment

plugins/work-items/tools/work-item-tracker/adapters/linear/reclaim.sh#L37 (stale fetch) / #L107-L109 (stale use)

The issue is fetched once before the activity-check and revalidate() round trips. revalidate() only re-checks the lease record, never the assignee. If a concurrent claim.sh takes the same expired lease and reassigns the item during that window, reclaim.sh still compares against the original (now-stale) assignee, matches the old holder, and unassigns — silently stripping the new claimer's live claim. This directly contradicts the invariant the file documents two lines above the bug ("clearing it would strip that claim"), and claim.sh itself avoids the identical gap by re-fetching right before its own analogous check (line 135). See inline comment for the full race trace.

IMPORTANT — Gitea host_suffix pin is a bare glob suffix match, no dot-boundary enforced

plugins/work-items/tools/work-item-tracker/adapters/gitea/common.sh#L225-L227

Unlike jira's hardcoded, dot-prefixed pin (.atlassian.net), Gitea's host_suffix is consumer-supplied free text compared with a plain *"$host_suffix" glob. Neither the generated README nor CONTRACT.md tells the consumer the value needs a leading dot, and the README's own worked example omits the field. A natural "host_suffix": "mycompany.com" is satisfied by a later host of evilmycompany.com, defeating the egress pin without tripping the loud allow_custom_domain=true opt-out. See inline comment for the fix.

Checked, no exploitable issue found

• Token handling in gitea/linear (env-var-name indirection, never argv, curl -K - stdin config) — matches the jira baseline.
• Bare-hostname validation, --proto '=https', no -k/-L — both adapters.
• Linear GraphQL: all variable values pass through the variables JSON object via jq --arg/--argjson, never string-concatenated into query text — no GraphQL injection surface.
• Scope/URL injection: gitea and linear scopes are anchored-regex-validated and exact-matched against the declared scope array before interpolation — not a prefix check.
• run-conformance.sh --binding name is validated against ^[a-z][a-z0-9-]*$ before any path use — no traversal.
• Conformance bindings (gitea.sh/linear.sh) require their throwaway-target env vars via ${VAR:?message} with no fallback to a real binding — cannot accidentally hit a coordination instance.
• work-item-tracker.sh dispatcher change is a fixed export WIT_SEAM_LIB_DIR=... path; dispatch stays bash "$verb.sh" "$@" (array, not eval/bash -c) — no injection.
• wit_linear_epoch's fractional-seconds handling matches its documented purpose; no case found where it misclassifies an expired lease as live or vice versa.
• mock.sh in both adapters is reachable only from *.test.sh files, never from a production code path.

Notes on environment

git fetch/arbitrary script execution weren't approved in this run, so the diff was pulled via gh pr diff instead — didn't block the review, but flagging in case --allowedTools should be widened for future runs.

Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

Claude finished @kyle-sexton's task in 10m 25s —— View job

---

Code review complete

• Gather context (PR diff, CLAUDE.md, REVIEW.md, changed files)
• Review skip gate (open, non-draft, non-trivial → proceeded)
• Review changed files for correctness / maintainability
• Post findings

This repo carries a claude-security-review workflow, so per REVIEW.md this lane scoped out security findings (token handling, injection, etc.) and focused on correctness and maintainability, per /review:code-review's criteria.

Findings (posted inline)

6 findings posted as inline comments, most severe first:

1. adapters/linear/reclaim.sh:64-78 (🔴 Important) — the lease's activity check only fetches the first 50 comments (no pagination/hasNextPage), unlike wit_linear_lease_comments in the same file which paginates exhaustively over the identical connection. On an issue with >50 comments, genuine recent activity past position 50 could be missed, letting reclaim release a lease that's still actively being worked.
2. skills/onboard-adapter/scripts/templates/common.sh.tmpl:222 (🔴 Important) — display_name (validated only for non-emptiness) is spliced unescaped into a single-quoted printf format string in the generated adapter. A display name containing an apostrophe (e.g. "Bob's Tracker") breaks the generated common.sh's shell syntax entirely.
3. skills/onboard-adapter/scripts/generate-adapter.sh:399-403 (🔴 Important) — for auth_scheme: basic, the generated conformance binding hardcodes auth_user to the placeholder ci@example.invalid, with no env-var override (unlike host/scope) and no "fill this in" marker. Not reachable by gitea/linear (they use token/raw), but a real gap for future basic-scheme onboarding.
4. adapters/gitea/README.md:65-74 (🔴 Important) — the verb coverage table marks create-item, get-item, link-blocks, and list-items as unfinished scaffold stubs, but all four are fully implemented and tested.
5. adapters/gitea/create-item.sh:109-115 (🟡 Suggestion) — the label-listing pagination loop has no upper bound, unlike every other paginated loop in this adapter (list-items.sh, common.sh's dependency count), both of which cap against WIT_GITEA_LIST_ITEMS_MAX.
6. conformance/bindings/linear.sh:14 (🟡 Nit) — the WIT_CONFORMANCE_LINEAR_SCOPE doc comment says (e.g. owner/repo), a copy-paste leftover from gitea.sh; Linear's actual scope shape is <workspace>/<TEAMKEY> (e.g. acme/ENG).

Each finding was independently re-verified by reading the actual code (not just the fanned-out review agent's claim) before posting.

Not flagged (checked, found consistent)

• Generator/template docs (SKILL.md, adapter-spec.md, live-exploration.md) match what generate-adapter.sh actually does.
• Gitea's capabilities.json/adapter-spec.json correctly agree with each other, with the "no verb script for a false verb" claim, and with capabilities.test.sh's manifest-vs-filesystem assertion.
• run-conformance.sh, work-item-tracker.sh, and CONTRACT.md's new sections are internally consistent and correctly wire the two new bindings the same way the pre-existing jira binding is wired.
• plugin.json/CHANGELOG.md version ladder (0.37.0 → 0.38.0 → 0.39.0) and README.md/providers.md capability claims all check out against the actual shipped adapters.
• docs/upstream/aihero-shipping-course.md's two-cell verdict fill (Lane F Lane F: /goal route-away row in draft-goal-condition #2938, Lane Y Lane Y: macro/micro lifecycle orchestrator — name, grouping, home #2948) points at real, existing surfaces.
• The wit_linear_epoch fractional-seconds handling flagged by one review pass turned out to have a real edge case (non-Z offsets combined with fractional seconds get silently reinterpreted as UTC) but is unreachable in practice — every actual caller only ever feeds it Linear's createdAt or this adapter's own wit_linear_now_iso output, both always Z-suffixed — so it wasn't posted as a finding.

Per the PR description, live conformance against a real Gitea/Linear instance has not been run in this environment — that's stated by the author as a known, deliberate gap, not something this review can independently verify either.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0812c7c8f

ℹ️ 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".

[github-actions]

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

[github-actions]

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

[claude]

Claude finished @kyle-sexton's task in 5m 49s —— View job

---

Security review complete

• Gather context (PR diff, CLAUDE.md, /review:security-review skill instructions)
• Skip-gate check (open, has security-relevant surface: shell-template code generation, token handling, GraphQL construction → proceeded)
• Review changed files for security issues (injection, token handling, trust boundaries, host/egress validation)
• Independently verify candidate findings against the actual code
• Post findings

Per REVIEW.md/the security-review skill, this lane reports security issues only (no style, test-coverage, or general code-quality commentary — that's /review:code-review's job), and defers supply-chain/permissions/trigger findings to zizmor's static-analysis lane.

Findings (posted inline)

1 finding posted:

1. adapters/linear/reclaim.sh:83-110 (IMPORTANT) — revalidate() only re-checks the lease matching the pre-race handle it started with, so a concurrent claimer's brand-new lease (posted mid-way through the activity-check round trip) is invisible to it. The subsequent unassign then uses a stale assignee snapshot captured before any of the round trips, so it silently clears a live, concurrent claimant's assignment — desyncing the item's visible ownership (what list-frontier and Linear's UI read) from its actual lease state. claim.sh:135 already re-fetches the issue right before its own conditional unassign for exactly this reason; reclaim.sh doesn't apply the same pattern here.

Already fixed, verified by reading the code (not re-reported)

The PR's own last commit (764fc53) fixes a genuine command-injection vulnerability an earlier review pass on this PR caught (a scope_pattern/sample_scope spec value containing ';touch /tmp/PWNED;#' would have broken out of a single-quoted context in generated common.sh and executed on source). I independently re-verified the fix rather than trusting the commit message:

• Grepped every template for '...@@KEY@@...' single-quoted substitution sites and confirmed each one's key (CONFIG_KEY, DISPLAY_NAME, HOST_SUFFIX, PROVIDER, PROVIDER_FUNC, PROVIDER_UPPER, SAMPLE_AUTH_EXTRA, SAMPLE_HOST, SAMPLE_SCOPE, SCOPE_PATTERN, USAGE_ARGS, VERB) is covered by generate-adapter.sh's new QUOTED_CONTEXT_KEYS allowlist/quote_safe() guard at the single choke point (render()) every substitution passes through.
• Confirmed keys not in that allowlist (SAMPLE_ENV, PROVIDER, BASE_PATH, etc.) are already constrained upstream to safe character classes by anchored regexes (e.g. SAMPLE_ENV must match ^[A-Za-z_][A-Za-z0-9_]*$), so a single quote can't reach them either way.
• Confirmed VERB/USAGE_ARGS are derived from a hardcoded verb list and static per-verb templates, not attacker-influenced spec data at all.
• Confirmed the already-shipped Gitea adapter (generated before this fix landed) doesn't itself carry an injected payload — its SCOPE_RE/HOST_SUFFIX values are the safe literals from its own spec.

Not flagged as new findings (checked, found either safe-by-construction or out of scope)

• GraphQL query construction (linear/common.sh, create-item.sh, list-items.sh, etc.): all dynamic values are passed as GraphQL variables via jq --arg/--argjson, never string-concatenated into query text — confirmed by reading every q=/QUERY= site in the adapter. One query embeds $WIT_LINEAR_ISSUE_FIELDS, but that's a hardcoded constant, not spec- or argv-derived.
• Token/credential handling: both new adapters follow the existing jira adapter's precedent exactly — credential fed to curl via -K - stdin config (never argv), --proto '=https' enforced, no -L (redirects not followed), host validated against an anchored hostname regex plus a host_suffix pin that denies custom domains by default. Not new to this PR; pre-existing pattern extended consistently.
• Curl -K config-line injection via the header/url values interpolated into the stdin config string: the credential itself is the operator's own trusted secret (not attacker input), and the url component is built only from a hostname that's already regex-validated to exclude quotes/newlines, plus (for Linear) a hardcoded empty path — no attacker-reachable value flows into that config string.
• renew-lease.sh has no holder-identity check (any caller who knows a lease_comment_id can renew it), but that's consistent with this system's stated threat model — a shared-credential coordination protocol between cooperating sessions, not a mechanism for excluding a malicious holder of the same token/scope — and the file's own header discusses this tradeoff explicitly for the sibling TOCTOU case.
• No .github/workflows changes in this PR, so no Actions trigger/permissions/secrets surface to review.

[claude]

Claude finished @kyle-sexton's task in 5m 18s —— View job

---

Security review complete

• Gather context (PR diff, changed files, prior review comments)
• Skip-gate checks — PR open, security-relevant surface, head advanced since last review (2 new fix commits) → proceeded with full re-review
• Review generator (generate-adapter.sh + templates) for injection risk
• Review gitea adapter (auth, host validation, scope checks)
• Review linear adapter (auth, GraphQL construction, lease/claim protocol)
• Review CONTRACT.md / conformance bindings / dispatcher changes
• Adversarially verify candidate findings against source (producer ≠ verifier)
• Post final findings

Scope: logic flaws, trust-boundary/authorization gaps, and injection surfaces only, per this repo's CI security-review lane (/review:security-review) — zizmor's static-analysis lane already covers unpinned actions, dangerous triggers, and excessive permissions, so those aren't re-reported here.

This is a re-review at HEAD 6750556 (was d0812c7c at the last pass). Two fix commits landed in between (764fc53, 6750556), both scoped to the CRITICAL generator-injection finding.

CRITICAL finding from the prior pass — now fixed, independently re-verified

spec.display_name / api.scope_pattern shell injection in the adapter generator (previously reported at generate-adapter.sh#L138-L139)

Confirmed fixed by reading the actual current source (not just trusting the commit message):

• generate-adapter.sh#L138-L145 now anchors display_name to ^[A-Za-z0-9][A-Za-z0-9\ ._/+-]*$ — this excludes ', `, $, #, and newline, closing all three dangerous contexts the templates put it in (the single-quoted printf in common.sh.tmpl:222, the double-quoted ${VAR:?…} where $(...) still expands in conformance-binding.sh.tmpl:33, and the #-comment/newline sites).
• generate-adapter.sh#L704-L711 adds a quote_safe() choke point invoked for every substitution in render() (both the caller-supplied-pair loop and the global-keys loop), rejecting a single quote in any QUOTED_CONTEXT_KEYS value — this independently closes the Codex-reported scope_pattern/sample_scope exploit (^';touch /tmp/proof;#'$).

I traced every '…@@KEY@@…' substitution site across all seven templates and confirmed each quoted key is either in QUOTED_CONTEXT_KEYS or already constrained upstream to a charset that cannot contain a quote — no gap found. (Dynamic exploit re-execution wasn't available in this run — script execution wasn't approved — so this is a thorough static trace, not a live PoC re-run; the static trace is unambiguous given the anchored regex and the choke-point placement.)

Still open — not addressed by the two fix commits (both were scoped only to the injection finding)

IMPORTANT — Linear reclaim.sh uses a stale pre-round-trip snapshot, letting it strip a concurrent claimer's live assignment
adapters/linear/reclaim.sh#L107-L109 still compares against $WIT_LINEAR_ISSUE fetched once at line 37, before the activity-check round trip and revalidate(). revalidate() only re-checks the lease record, never the assignee — so a concurrent claim.sh that takes the same expired lease during that window gets silently unassigned by this reclaim. claim.sh:135 already avoids the identical gap by re-fetching right before its own analogous check. Posted inline with the full race trace.

IMPORTANT — Linear reclaim.sh activity check only inspects the first 50 comments
adapters/linear/reclaim.sh#L65 has no after cursor / hasNextPage loop, unlike wit_linear_lease_comments two lines above in the same file, which paginates exhaustively over the identical connection. On an issue with >50 comments, genuine post-renewed_at activity beyond position 50 is missed and an actively-worked item gets reclaimed. Posted inline.

IMPORTANT — host_suffix pin is a bare glob suffix match, no dot-boundary enforced — and it's a template-level gap, not gitea-specific
adapters/gitea/common.sh#L225-L227: consumer-supplied host_suffix (e.g. "mycompany.com", no leading dot) is satisfied by a later host of evilmycompany.com — unlike jira's hardcoded, dot-prefixed pin. Tracing the shared generator template confirmed the same unguarded pattern lives at templates/common.sh.tmpl#L203-L205, so it propagates to every future generated adapter, and the hand-built linear adapter has it too at common.sh#L242-L244 (safe by its .linear.app default, reopened by a dot-less consumer override). Posted inline with the fix recommendation (fix once in the template).

Checked, no exploitable issue found (in addition to the prior pass's list)

• Token handling, GraphQL variable binding, scope/URL anchored-regex validation, run-conformance.sh --binding name validation, conformance-binding required-env guards, dispatcher argument handling — all re-checked, unchanged from the prior pass, still sound.
• The new quote_safe() mechanism itself: confirmed it fires on every substitution path in render(), not just one of the two call sites.

Notes on environment

Script execution (running the generator or its test suite directly) wasn't approved in this run, so verification of the injection fix was done by static trace of the source rather than a live exploit re-run. Recommend widening --allowedTools for a future pass if a live exploit re-run is wanted for extra confidence.

Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

Claude finished @kyle-sexton's task in 4m 9s —— View job

---

Code review complete

• Load /review:code-review skill instructions
• Check skip gate — PR open, non-draft, non-trivial, and 4 new commits landed since this lane's last pass at an earlier head → proceeded, scoped to the delta
• Diff since last code-review pass on this PR
• Review changed/updated files for correctness & maintainability
• Adversarially verify candidate findings against actual source
• Post inline comments and final summary

Per REVIEW.md, this lane scopes security findings out (this repo carries a claude-security-review workflow) and focuses on correctness/maintainability. Four commits landed since this lane's prior pass reviewed an earlier head:

• 764fc53 — closes the CRITICAL render()-level quote-breakout (already independently re-verified by the security lane on this PR; not re-reviewed here since it's security-scoped)
• 6750556 — constrains display_name at validation
• 8ba7fba — regenerates docs/CATALOG.md, classifies onboard-adapter as excluded infra setup in the cheatsheet generator
• 269200e — closes the Linear reclaim.sh stale-snapshot race three independent review passes converged on

Findings (posted inline)

2 new findings, both Important:

1. skills/onboard-adapter/reference/adapter-spec.md:54 — the display_name row still documents it as "Free text for comments and docs," but 6750556 constrained it to an anchored charset in the same PR, for the same reason every neighboring row's constraint is spelled out ("substituted literally... nothing downstream escapes it"). This row wasn't updated to match, so the documented handoff artifact between interview and generator is now wrong about its own most security-relevant field.
2. skills/onboard-adapter/scripts/generate-adapter.test.sh:228 — 6750556's commit message claims "Bob's Tracker", a $(...) payload, and an embedded newline were each manually verified as rejected, but none of those three cases has an automated assertion. This is the exact field a prior review round flagged as a CRITICAL injection vector; a future regression in the regex or quote_safe/QUOTED_CONTEXT_KEYS would have nothing here to catch it.

Verified correct (not flagged)

• 269200e's reclaim race fix — read the full mutated reclaim.sh and its new regression test (reclaim.test.sh:141-163). revalidate() now correctly checks for any live lease under a different handle (not just the original), closing the gap where a concurrent claimer's own new lease was invisible to the handle-keyed re-read. The assignee comparison is now re-fetched immediately before the unassign rather than reused from the pre-race snapshot, mirroring claim.sh:135. The seeded test (rival lease posted under a different handle mid-window) fails with the fix reverted and passes with it applied per the commit message — logic checks out by inspection.
• 764fc53's quote_safe()/QUOTED_CONTEXT_KEYS guard — single choke point at render(), keys derived by grepping actual template substitution sites rather than guessed; consistent with the file's stated design.
• 8ba7fba's catalog/cheatsheet changes — mechanical, consistent with the existing EXCLUDED_SKILLS pattern and generator consumption in scripts/generate-cheatsheet.mjs.

Not re-reported (already flagged in an earlier pass on this PR, still open, untouched by these 4 commits)

To avoid duplicate noise, these previously-posted findings weren't re-posted — they still stand at this head:

• adapters/gitea/README.md verb-coverage table still marks 4 implemented verbs as scaffolds
• adapters/gitea/create-item.sh label-pagination loop still has no upper bound
• conformance/bindings/linear.sh:14 scope doc-comment still describes GitHub's shape, not Linear's
• generate-adapter.sh basic-auth placeholder auth_user still has no override path (not reachable by the two shipped adapters)

Environment note

Could not execute the adapter/generator test suites directly in this run (test-runner Bash invocations were not approved) — findings above on reclaim.sh and the fixup commits are based on reading the code and the existing tests, not on independently running them.
· Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

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

---

Security review complete

• Load /review:security-review skill instructions
• Skip-gate check — PR open; one new commit (9e006df) landed since the last reviewed head (7b83a3b) → proceeded, scoped to the delta
• Diff since last security-reviewed head (7b83a3b) against current HEAD (9e006df)
• Review changed files for security issues
• Adversarially verify — confirmed the delta touches no production security surface
• Post final findings

This is a re-review at HEAD 9e006df (last reviewed head was 7b83a3b). Exactly one commit landed in between, touching three files:

plugins/skill-quality/scripts/check-skill.sh                                     | 12 ++++++++++--
.../onboard-adapter/scripts/templates/common.test.sh.tmpl                        | 15 +++++++++++++++
.../work-item-tracker/adapters/linear/common.test.sh                             | 15 +++++++++++++++

Not security-relevant (checked, no exploitable issue found)

• common.test.sh.tmpl / linear/common.test.sh — test-only additions (DOTLESS pin regression cases: suffix confusion, hyphenated lookalike, real subdomain, bare apex). These close the two "test-debt" findings raised in an earlier code-review pass on this PR — the host_under_pin() domain-boundary fix (7b83a3b, already security-reviewed and verified fixed) previously had regression coverage only in gitea/common.test.sh. No production logic changed here; these are pure assertions against already-fixed code in generate-adapter.sh template and linear/common.sh. Confirmed the assertions actually exercise the intended cases (matching the pattern already verified in gitea/common.test.sh).
• plugins/skill-quality/scripts/check-skill.sh — captures a sub-test's stdout/stderr into a variable and prints it on failure instead of discarding it (lines 565-583), so a failing *.test.sh is diagnosable from CI output. This is repo-internal tooling (a skill linter run against this repo's own skill fixtures), not part of any shipped adapter's credential/egress path — no secret material flows through it, and it doesn't touch the token/host/auth handling this PR's security-relevant surface consists of.

Still open — unchanged by this delta, previously reported on this PR

IMPORTANT — Linear reclaim.sh activity check only inspects the first page of comments (max 50). adapters/linear/reclaim.sh#L65 — re-verified at current HEAD, still comments(first: 50) with no after cursor / hasNextPage loop, unlike wit_linear_lease_comments in the same file which paginates exhaustively over the identical connection. On an issue with >50 comments, genuine post-renewed_at activity beyond position 50 is still silently missed, which can let reclaim release a lease that's still actively being worked. Already posted inline in an earlier pass on this PR; not re-posted here to avoid duplicate noise.

Previously reported, confirmed still fixed (spot-checked, not re-flagged)

• The display_name shell-injection guard and the host_under_pin() domain-boundary fix — both re-confirmed present and correct in the files touched by this delta while verifying the new tests actually exercise them.

Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

Claude finished @kyle-sexton's task in 2m 9s —— View job

---

Code review complete

• Load /review:code-review skill instructions
• Check skip gate — PR open, non-draft; one new commit (9e006df) landed since this lane's last pass at 7b83a3b → scoped review to that delta
• Review the delta commit for correctness & maintainability
• Verify it against the two findings it claims to fix
• Post summary (no new findings)

Per REVIEW.md, this lane scopes security findings out (this repo carries a claude-security-review workflow) and focuses on correctness/maintainability. This synchronize event added exactly one commit, 9e006df, on top of the head this lane last reviewed (7b83a3b).

What changed in this delta

9e006df is a direct fix for the two findings this lane left open after the previous pass, plus an unrelated diagnostic improvement:

1. skills/onboard-adapter/scripts/templates/common.test.sh.tmpl:170-183 — the four DOTLESS-pin regression assertions (suffix-confusion denial, hyphenated-lookalike denial, real-subdomain acceptance, apex acceptance) are now ported into the shared template test scaffold, matching what gitea/common.test.sh already had. Every future adapter generated by onboard-adapter now inherits test coverage for the wit_..._host_under_pin() domain-boundary fix, not just the fixed logic.
2. adapters/linear/common.test.sh:170-183 — the identical assertion block, adapted to Linear's own $GOOD fixture (api.mycompany.com in place of gitea's git.mycompany.com).
3. plugins/skill-quality/scripts/check-skill.sh:574-580 — Check 7 (scripts/*.test.sh) now captures each test's combined output instead of discarding it, and replays it on failure only. Unrelated to the two adapter fixes; targets a specific CI-only, unreproducible-locally failure the author called out in the commit message.

Verified correct (not flagged)

• Template port (chore: initialize marketplace scaffold #1) — read the full block; $GOOD is defined at line 131, well before the new block at 174, matching the pattern already established by the PINNED assertions immediately above it. Traced wit_..._host_under_pin()'s normalization logic (dot-led pin + separate apex check) and confirmed the expected pass/fail per assertion is correct.
• Linear port (fix: correct plugin-system inaccuracies and harden references #2) — same trace against adapters/linear/common.sh:100-104's wit_linear_host_under_pin(): pin mycompany.com normalizes to .mycompany.com, evilmycompany.com/attacker-mycompany.com correctly don't match (character before the pin substring isn't .), api.mycompany.com and the bare apex both correctly do. $GOOD here is defined at line 131 in this file too — same ordering, no forward-reference bug.
• check-skill.sh diagnostic change (feat: markdown-formatter plugin (pilot migration) #3) — this runs at top level (not inside a function), so the unprefixed test_out local variable is fine. if test_out="$(cmd)"; then correctly preserves cmd's exit status for the branch. The err()/printf ... >&2 sequence writes the FAIL: line followed by the captured output, both to stderr — consistent with the rest of the file's error-reporting convention. Pass/fail semantics for Check 7 are unchanged; only the failure path gained diagnostic output.
• No functional regressions, no new injection/logic surface, no test assumes something the underlying code doesn't actually do.

Not flagged

• check-skill.sh's own test suite (check-skill.test.sh) has no dedicated case asserting the new output-capture-on-failure behavior. Not raising this as a finding: it's a diagnostics-only change with no effect on pass/fail semantics, and the author's commit message already accounts for check-skill.test.sh's pre-existing (unrelated, ungated) redness rather than silently omitting the topic.

Still open — not touched by this delta, previously reported on this PR

To avoid duplicate noise, these aren't re-posted — they still stand at this head and weren't in scope of 9e006df:

• adapters/linear/reclaim.sh:65 — activity check only inspects the first page (50) of comments, no pagination.
• adapters/gitea/README.md verb-coverage table still marks 4 implemented verbs as scaffolds.
• adapters/gitea/create-item.sh label-pagination loop still has no upper bound.
• conformance/bindings/linear.sh:14 scope doc-comment still describes GitHub's shape, not Linear's.
• generate-adapter.sh basic-auth placeholder auth_user still has no override path (not reachable by the two shipped adapters).

Environment note

Read-only tooling only for this pass (git log/git show/Read/Grep) — no test execution. Findings above on the new assertions are based on static trace against the actual wit_..._host_under_pin() implementations, not an independent test run.
· Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

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

---

Code review complete

• Load /review:code-review skill instructions
• Check skip gate — PR open, non-draft; one new commit (aa6065f) landed since this lane's last pass at 9e006df → scoped review to that delta
• Review the delta commit for correctness & maintainability
• Post summary (no new findings)

Per REVIEW.md, this lane scopes security findings out (this repo carries a claude-security-review workflow) and focuses on correctness/maintainability. This synchronize event added exactly one commit, aa6065f, on top of the head this lane last reviewed (9e006df).

What changed in this delta

aa6065f is a CI-diagnosis fix with two independent parts:

1. skills/onboard-adapter/scripts/generate-adapter.test.sh:424-441 — the "generated shell is ShellCheck-clean" case previously ran shellcheck --rcfile=... whenever shellcheck was on PATH and asserted the exit code was 0. On the skill-quality-gate CI runner (which installs no extra tooling), the pre-installed ShellCheck predates 0.10.0 and doesn't support --rcfile, so it exits 3 ("bad syntax") — which the assertion read as a lint failure rather than a version mismatch. The fix probes the actual binary (shellcheck --rcfile="$SC_RC" /dev/null) before trusting it, and skips loudly (matching the file's existing "SKIP: tool not available" idiom) when the probe itself fails.
2. plugins/skill-quality/.claude-plugin/plugin.json / CHANGELOG.md — retroactive 0.17.0 → 0.17.1 version bump + changelog entry for 9e006df's check-skill.sh diagnostic-output change, which had landed without one (changelog-parity fix).
3. The remaining 20 files in the commit are exec-bit-only mode changes (100644 → 100755) on shell scripts/templates, correcting an earlier attempt (git update-index --chmod=+x) that a same-commit git add -A silently reverted. No content changed in any of these 20 files.

Verified correct (not flagged)

• The ShellCheck version-probe logic — traced by hand: .shellcheckrc sets shell=bash, so linting the empty /dev/null probe file doesn't hit the "unknown shell" error path on a modern ShellCheck (probe exits 0); an older binary that rejects --rcfile exits 3 regardless of target file, so the probe correctly discriminates "flag unsupported" from "flag supported, zero findings" without needing to parse a version string.
• Mode-only changes — confirmed via git diff --summary that all 20 mode-change entries are .sh/.tmpl shell files (test scripts and generator templates), consistent with what should be executable; no unrelated file had its mode touched, and no file content changed alongside a mode flip.
• Version bump — skill-quality plugin version/changelog now match; work-items (the plugin generate-adapter.test.sh actually lives in) was left unbumped, consistent with this being a fix to a test added earlier in this same PR's unreleased 0.37.0–0.39.0 cycle rather than a change to already-changelogged behavior.

No new correctness or maintainability findings in this delta.

Still open — not touched by this delta, previously reported on this PR

Not re-posted, to avoid duplicate noise — still stand at this head:

• adapters/linear/reclaim.sh:65 — activity check only inspects the first page (50) of comments, no pagination.
• adapters/gitea/README.md:65-74 — verb coverage table still marks 4 implemented verbs as scaffolds.
• adapters/gitea/create-item.sh:109-115 — label-pagination loop still has no upper bound.
• conformance/bindings/linear.sh:14 — scope doc-comment still describes GitHub's shape, not Linear's.
• generate-adapter.sh basic-auth placeholder auth_user still has no override path (not reachable by the two shipped adapters).

Environment note

Read-only tooling only for this pass (git log/git show/Read/Grep/git diff --summary) — no test execution. The ShellCheck probe logic was verified by reading .shellcheckrc and tracing the two code paths, not by running the test suite with multiple ShellCheck versions.
· Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

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

---

Security review complete

• Load /review:security-review skill instructions
• Skip-gate check — PR open; one new commit (aa6065f) landed since the last-reviewed head (9e006df) → proceeded, scoped to the delta
• Diff since last-reviewed head (9e006df) against current HEAD (aa6065f)
• Review changed files for security issues
• Adversarially verify — confirmed the delta touches no production security surface
• Post final findings

This is a re-review at HEAD aa6065f (last reviewed head was 9e006df). Exactly one commit landed in between, touching only:

plugins/skill-quality/.claude-plugin/plugin.json                        |  2 +-
plugins/skill-quality/CHANGELOG.md                                      | 15 +++++++++++++++
.../onboard-adapter/scripts/generate-adapter.test.sh                    | 20 ++++++++++++++++----
+ 20 files with a mode-only change (100644 → 100755, 0 insertions/deletions)

Not security-relevant (checked, no exploitable issue found)

• generate-adapter.test.sh#L424-L440 (blob) — replaces an unconditional shellcheck --rcfile=... invocation with a probe (shellcheck --rcfile="$SC_RC" /dev/null first) that skips the lint assertion when the local ShellCheck binary is too old to accept --rcfile (exit 3, "invoked with bad syntax", was previously being misread as a lint failure). This only changes which test cases run in CI based on tool version; it doesn't touch any adapter's credential, egress, or injection-relevant code path, and the probe command itself is a fixed literal (/dev/null) with no attacker-influenced input.
• plugin.json version bump + CHANGELOG.md entry — metadata/docs only, describing the fix above.
• 20 mode-only changes (generate-adapter.sh, several templates/*.tmpl, and a batch of adapter *.test.sh files) — confirmed via git diff --stat these are pure chmod +x (0 insertions, 0 deletions each), fixing a prior broken attempt to set the exec bit (an earlier git update-index --chmod=+x was clobbered by a subsequent git add -A re-reading on-disk 644 modes, per this commit's own message). No content change to verify.

Still open — unchanged by this delta, previously reported on this PR

IMPORTANT — Linear reclaim.sh activity check only inspects the first page of comments (max 50). adapters/linear/reclaim.sh#L65 — re-verified at current HEAD, still comments(first: 50) with no after cursor / hasNextPage loop, unlike wit_linear_lease_comments in the same file, which paginates exhaustively over the identical connection. On an issue with >50 comments, genuine post-renewed_at activity beyond position 50 is still silently missed, which can let reclaim release a lease that's still actively being worked. Already posted inline in an earlier pass on this PR; not re-posted here to avoid duplicate noise.

Previously reported, confirmed still fixed (spot-checked, not re-flagged)

The display_name shell-injection guard, the quote_safe()/QUOTED_CONTEXT_KEYS choke point, the host_under_pin() domain-boundary fix, and the reclaim.sh stale-snapshot race fix — none of the files implementing these were touched by this delta; all remain as verified in the prior pass.
· Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

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

---

Security review complete

• Load /review:security-review skill instructions
• Skip-gate check — PR open; one new commit (c8bf3ab) landed since the last-reviewed head (aa6065f) → proceeded, scoped to the delta
• Diff since last-reviewed head (aa6065f) against current HEAD (c8bf3ab)
• Review changed files for security issues
• Adversarially verify — traced the fix by hand against the pre-existing pagination pattern it mirrors
• Post final findings

This is a re-review at HEAD c8bf3ab (last reviewed head was aa6065f). Exactly one commit landed in between, c8bf3ab, doing two things:

plugins/work-items/skills/onboard-adapter/scripts/generate-adapter.sh          | 13 +++++++
plugins/work-items/skills/onboard-adapter/scripts/templates/capabilities.sh.tmpl        |  2 +-  (mode 755→644)
plugins/work-items/skills/onboard-adapter/scripts/templates/capabilities.test.sh.tmpl   |  2 +-  (mode 755→644)
plugins/work-items/skills/onboard-adapter/scripts/templates/common.test.sh.tmpl         |  2 +-  (mode 755→644)
plugins/work-items/skills/onboard-adapter/scripts/templates/verb.sh.tmpl                |  2 +-  (mode 755→644)
plugins/work-items/tools/work-item-tracker/adapters/linear/common.sh          | 45 +++++++
plugins/work-items/tools/work-item-tracker/adapters/linear/mock.sh            | 11 ++
plugins/work-items/tools/work-item-tracker/adapters/linear/reclaim.sh         | 21 +------
plugins/work-items/tools/work-item-tracker/adapters/linear/reclaim.test.sh    | 21 +++++

Fixed — verified by reading the code, not the commit message

IMPORTANT — Linear reclaim.sh activity check only inspected the first page of comments (max 50). This is the finding this lane (and the code-review lane) had repeatedly re-reported as still open since the PR's earliest reviewed head. Now closed:

• adapters/linear/common.sh#L690-L734 adds wit_linear_activity_since(), a paginating helper structured identically to the pre-existing wit_linear_lease_comments() two functions below it — same WIT_LINEAR_PAGE_SIZE-driven cursor loop, same pageInfo.hasNextPage/endCursor termination, same lease-marker skip (WIT_LINEAR_LEASE_PREFIX) so the adapter's own renewal heartbeat doesn't count as "activity."
• reclaim.sh#L61-L71 now calls this helper instead of the old unpaginated inline comments(first: 50) query — the exact code this lane's prior passes anchored the finding to is gone.
• Traced the pagination loop by hand: has_next starts "true", each iteration re-queries with the previous page's endCursor, breaks if hasNextPage is false or endCursor comes back empty (guards a malformed response from looping forever), and returns 0 (activity found) as soon as a non-lease comment postdating since is seen on any page — closing the "activity on the last page was invisible" gap.
• The new regression test (reclaim.test.sh#L165-L184) seeds the qualifying comment on page two via a new lin_comments_page mock helper (mock.sh#L121-L130) — this is a genuinely discriminating test (the old lin_comments mock hardcoded hasNextPage:false and couldn't express a multi-page response at all), so a regression that drops pagination again would be caught.

Not security-relevant (checked, no exploitable issue found)

• Template shebang → @@SHEBANG@@ substitution (generate-adapter.sh#L41-L51,731) — SHEBANG is a fixed literal ('#!/usr/bin/env bash'), not spec- or consumer-derived, so it's outside the injection surface the quote_safe()/QUOTED_CONTEXT_KEYS choke point exists to guard; correctly not added to that allowlist. The four templates dropped their literal shebang (mode reverted 755→644) purely to stop the repo's own script-discovery/ShellCheck tooling from misclassifying an un-rendered template as an executable script — a CI-hygiene fix, not a behavior change to any generated adapter's runtime security posture. Confirmed emit() (generate-adapter.sh#L766-L782) still writes the shebang and sets the exec bit on generated output exactly as before.
• lin_comments_page mock helper — test-only, reachable only from *.test.sh files, no production code path.

No new findings this pass

All previously-reported IMPORTANT/CRITICAL findings on this PR (display_name shell injection, host_suffix domain-boundary bypass, reclaim.sh stale-assignee race, reclaim.sh comment-pagination) are now fixed and verified across this review's history. Nothing new surfaced in this delta.

Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

Claude finished @kyle-sexton's task in 2m 8s —— View job

---

Security review complete

• Load /review:security-review skill instructions
• Skip-gate check — PR open; one new commit (22ad3c6) landed since the last-reviewed head (c8bf3ab) → proceeded, scoped to the delta
• Diff since last-reviewed head (c8bf3ab) against current HEAD (22ad3c6)
• Review changed files for security issues
• Adversarially verify — traced the new generator keys through render()'s quoting model by hand
• Post final findings

This is a re-review at HEAD 22ad3c6 (last reviewed head was c8bf3ab). Exactly one commit landed in between, 22ad3c6, closing the four remaining code-review findings on this PR plus a latent generator bug it surfaced while doing so:

plugins/work-items/skills/onboard-adapter/scripts/generate-adapter.sh                    | 30 +++++++++++++++++++++-
plugins/work-items/skills/onboard-adapter/scripts/templates/common.test.sh.tmpl          |  2 +-
plugins/work-items/skills/onboard-adapter/scripts/templates/conformance-binding.sh.tmpl  |  9 ++++---
plugins/work-items/tools/work-item-tracker/adapters/gitea/README.md                      |  8 +++---
plugins/work-items/tools/work-item-tracker/adapters/gitea/create-item.sh                 | 21 +++++++++++++--
plugins/work-items/tools/work-item-tracker/conformance/bindings/linear.sh                |  2 +-

No new security issues found

Checked each change against the injection surface this PR's history has repeatedly turned up (the render()/QUOTED_CONTEXT_KEYS/quote_safe() choke point in the generator):

• generate-adapter.sh#L433-L449 adds three new substitution keys for auth_scheme: basic (AUTH_CONFORMANCE_REQUIRE, AUTH_CONFORMANCE_JQARG, CONFORMANCE_AUTH_EXTRA), rendered into conformance-binding.sh.tmpl:36-41. None of the three carry spec- or attacker-influenced data — all three are fixed literal strings the generator assembles from $PROVIDER_UPPER (itself already constrained upstream), gated on the fixed basic branch of a case "$AUTH_SCHEME" — so they're safe by construction even though they aren't in QUOTED_CONTEXT_KEYS. The actual runtime value they wire up ($auth_user, sourced from the operator's own WIT_CONFORMANCE_<P>_AUTH_USER env var) reaches the generated jq program through --arg au "$auth_user", not string interpolation — correct jq variable binding, no injection path.
• New SAMPLE_ID_NUMBER/SAMPLE_ID keys (generate-adapter.sh#L362) land inside double-quoted contexts in common.test.sh.tmpl:62,64, where $(...) still expands — the same class of gap the display_name CRITICAL finding hit earlier in this PR's history. Traced it through: SAMPLE_ID is anchored-regex validated against WIT_ID_REGEX (^([a-z0-9][a-z0-9-]*):([A-Za-z0-9_.-]+)/([A-Za-z0-9_.-]+)#([0-9]+)$, lib/id.sh:8) before SAMPLE_ID_NUMBER is derived from it via ${SAMPLE_ID##*#} — that charset excludes $, backticks, quotes, and whitespace entirely, so neither value can carry a command substitution or break out of the double-quoted context. Not exploitable.
• create-item.sh label-pagination cap (create-item.sh#L107-L140) — closes an unbounded-loop path (a pathological/misbehaving server response could previously spin forever). All values still flow through jq --arg/--argjson, no new string interpolation. This is a availability/robustness fix, not itself something a malicious actor exploits, but worth noting it removes a resource-exhaustion vector against a misbehaving/compromised Gitea host.
• gitea/README.md, conformance/bindings/linear.sh — pure documentation corrections, no security surface.

Previously reported findings — final status, all fixed and verified across this PR's review history

• CRITICAL display_name shell injection — fixed (764fc53, 6750556), re-verified.
• IMPORTANT reclaim.sh stale pre-round-trip assignee snapshot — fixed (269200e), re-verified.
• IMPORTANT reclaim.sh activity check missing pagination — fixed (c8bf3ab), re-verified.
• IMPORTANT host_suffix domain-boundary bypass (gitea, linear, and the shared template) — fixed (7b83a3b), re-verified.

Nothing new surfaced in this delta.

Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

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

---

Security review complete

• Gather context (PR diff since last-reviewed head 22ad3c6, /review:security-review skill instructions)
• Skip-gate check — PR open; one new commit (44873a4) landed since the last-reviewed head → proceeded, scoped to the delta
• Diff since last-reviewed head (22ad3c6) against current HEAD (44873a4)
• Review changed files for security issues
• Adversarially verify — confirmed the delta introduces no logic change
• Post final findings

This is a re-review at HEAD 44873a4 (last reviewed head was 22ad3c6). Exactly one commit landed in between, 44873a4, touching only:

plugins/work-items/skills/onboard-adapter/scripts/generate-adapter.sh      | 8 ++++++++
plugins/work-items/skills/onboard-adapter/scripts/generate-adapter.test.sh | 5 +++++
2 files changed, 13 insertions(+)

No new security issues found

This commit is purely ShellCheck-suppression annotations plus one explicit no-op default case branch — no logic change. Verified by reading the actual diff, not the commit message:

• generate-adapter.sh#L749-L752 — adds *) ;; as quote_safe()'s explicit default branch. Confirmed this is behaviorally identical to the prior implicit fallthrough (a bash case with no matching pattern and no default already does nothing) — the QUOTED_CONTEXT_KEYS allowlist/quote_safe() choke point this PR's history repeatedly traced for the display_name/scope_pattern injection finding is unchanged in substance, just documented.
• generate-adapter.sh#L444-L445,452-453 — two # shellcheck disable=SC2016 annotations over pre-existing literal strings (CONFORMANCE_AUTH_EXTRA=',"auth_user":$au' and AUTH_CONFORMANCE_JQARG=' --arg au "$auth_user"'). Both are unchanged values from the prior commit already reviewed on this PR (22ad3c6 — reviewed and found safe: $au/$auth_user are jq variables bound via --arg in the generated conformance binding, expanded when that file runs, not shell-expanded here). The annotation only suppresses a linter warning about the deliberate non-expansion; no character reaching this code changed.
• generate-adapter.test.sh#L237-L239,245-247 — two # shellcheck disable=SC2016 annotations over the existing $(id)/backtick injection-payload test cases added in an earlier commit on this PR (already reviewed: these must stay unexpanded so the test exercises the generator's rejection of literal command-substitution syntax, not this shell's own expansion of it). Test-only, no production path.

Previously reported findings — all still fixed, unaffected by this delta

Nothing in this commit touches any of the files where this PR's history found and fixed issues (CRITICAL display_name shell injection, reclaim.sh stale-assignee race, reclaim.sh comment pagination, host_suffix domain-boundary bypass) — all remain fixed as verified in prior passes.

Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

Claude finished @kyle-sexton's task in 2m 27s —— View job

---

Security review complete

• Gather context (PR diff, prior review history, custom instructions)
• Invoke /review:security-review and follow its instructions
• Diff since last-reviewed head (44873a4) against current HEAD (ffc25be)
• Review changed files for security issues
• Adversarially verify candidate findings against source
• Post final findings

This is a re-review at HEAD ffc25be71c378e9a1ba764bec8eff4a4febdc80e (last reviewed head was 44873a4). Two commits landed in between:

1caa534  feat: close the three Lane C spin-offs rather than carry them as follow-ups
ffc25be  merge: bring origin/main into the adapter track, resolving five version collisions

Confirmed via git diff --stat 44873a4 ffc25be -- plugins/work-items/ that no file under plugins/work-items/ changed in this delta — the generator, both adapters, and the conformance bindings (the surface every prior finding on this PR lived in) are untouched since the last security pass.

Reviewed, no new security issues found

• plugins/claude-ops/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.sh (new, 1caa534) — read-only local-telemetry reporting tool. CALLER/CALLEE (from --pair) reach the jq program only via --arg, never string-interpolated into the filter text (lines 132–201) — no injection surface. STORE is used directly as a file path (<"$STORE"), sourced from --store or git rev-parse --show-toplevel, both operator-controlled, not attacker input. Malformed JSONL rows are dropped via fromjson? rather than fatal, matching the stated posture and closing the jq -s-fails-whole-file defect the commit message describes. No credential/token/egress surface at all — it only reads a local JSONL file.
• plugins/skill-quality/scripts/check-skill.sh — the merge brought in main's mawk-portability fix (ERE-interval rewrite in the markdown-fence/blockquote scanner) alongside this PR's own test-output-capture change from an earlier commit (9e006df). Diffed 44873a4:check-skill.sh against ffc25be:check-skill.sh and confirmed both changes coexist correctly — the merge did not clobber either side.
• Manifest version-collision resolutions (claude-ops, session-flow, skill-quality plugin.json) — per the merge commit's own message, three plugins were bumped to the same version on both branches; resolved by re-basing this branch's bump on top of main's released one, taking main's exact bytes and editing only the version field. Spot-checked claude-ops/.claude-plugin/plugin.json: the diff is exactly the version bump plus a —→— re-encoding fix, no content or logic change. Not a security surface (manifest metadata only).
• Everything else in the merge (ai-slop, disk-hygiene, planning, session-flow save-point/handoff docs, source-control, scripts/check-shell-portability.sh, scripts/check-skill-count-claims.sh, .claude/ai-slop.json) came in unmodified from origin/main (six commits landed there while this branch was open) — none of it touches this PR's own security-relevant surface (token/credential handling, egress control, generator templating, lease/concurrency logic in the adapters), and it isn't code authored by this PR.
• pre-pr.md / code-reviewer.md picked up a few more lines during the merge beyond 1caa534's own edit — both are process/criteria documentation, no security surface.

Previously reported findings — final status, all fixed and verified across this PR's review history, unaffected by this delta

• CRITICAL display_name shell injection in the generator — fixed (764fc53, 6750556)
• IMPORTANT reclaim.sh stale pre-round-trip assignee snapshot — fixed (269200e)
• IMPORTANT reclaim.sh activity check missing pagination — fixed (c8bf3ab)
• IMPORTANT host_suffix domain-boundary bypass (gitea, linear, shared template) — fixed (7b83a3b)

Nothing new surfaced in this delta. No findings to post.
· Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

Claude finished @kyle-sexton's task in 3m 44s —— View job

---

Security review complete

• Gather context (PR diff since last-reviewed head ffc25be, /review:security-review skill instructions)
• Skip-gate check — PR open; one new commit (ecc1acc) landed since the last-reviewed head → proceeded, scoped to the delta
• Diff since last-reviewed head (ffc25be) against current HEAD (ecc1acc)
• Review changed files for security issues
• Adversarially verify the candidate finding by tracing the exact race against the source
• Post final findings

This is a re-review at HEAD ecc1accb25ed385a18eccb4b03bfe80eccb9003a (last reviewed head was ffc25be). One commit landed in between, ecc1acc, touching only the Linear adapter's lease/claim protocol:

plugins/work-items/.claude-plugin/plugin.json                              |  2 +-
plugins/work-items/CHANGELOG.md                                            | 30 ++++++++++++++
plugins/work-items/tools/work-item-tracker/adapters/linear/claim.sh        | 36 +++++++++++++++--
plugins/work-items/tools/work-item-tracker/adapters/linear/claim.test.sh   | 33 ++++++++++++++
plugins/work-items/tools/work-item-tracker/adapters/linear/reclaim.sh      |  4 +-
plugins/work-items/tools/work-item-tracker/adapters/linear/renew-lease.sh  |  2 +-

Finding (posted inline)

1 new finding, IMPORTANT:

adapters/linear/claim.sh:107-110 — this commit adds an EXIT-trap rollback (_wit_linear_claim_rollback) to guard the assign→lease window, correctly fixing the "assigned with no lease" strand case its own test covers. But the rollback calls wit_linear_set_assignee "$ISSUE_UUID" "" unconditionally, with no check that the current assignee is still this session. It stays armed through the second wit_linear_update_comment write (line 128) and the AFTER arbitration read (line 133) — both of which the same commit made exit-on-failure. If a concurrent session legitimately wins the claim in that window (overwriting Linear's single-field assignee, exactly the hazard this file's own header describes as differentiating it from GitHub's list-based assignees) and then this session hits a transient failure, the rollback strips the winner's live assignment. The LOSER branch two sections below (lines 159-164) already applies the correct pattern — re-fetch and compare before clearing — and reclaim.sh was fixed earlier in this same PR (269200e) for the identical class of bug; the new rollback trap doesn't get the same treatment. Traced against claim.test.sh's new rollback case (lines 165-179): it only seeds a failure before any lease/assignee exists from a second party, so this gap is untested. Full race trace and fix direction posted inline.

Not security-relevant (checked, no exploitable issue found)

• reclaim.sh / renew-lease.sh (3 one-line changes) — each adds || exit "$?" after a wit_linear_lease_comments call inside a command substitution, propagating the helper's exit status out of the subshell instead of silently continuing with an empty result. This closes a real correctness gap (an exit inside $(...) only ends the subshell) but doesn't introduce new exposure — verified each site now correctly aborts rather than treating a failed read as "nothing claimed."
• claim.sh's other new || exit "$?" sites (lines 78, 120, 133) — same pattern, correctly propagate failure instead of silently arbitrating against an incomplete/empty read. The CRITICAL bug this commit's message describes (a failed commentCreate being swallowed by the subshell boundary, reporting a lease that doesn't exist) is genuinely fixed by the line-120 change.
• plugin.json / CHANGELOG.md — version bump and changelog entry only, no logic.

Previously reported findings — unaffected by this delta, still fixed as verified in prior passes

CRITICAL display_name shell injection, IMPORTANT reclaim.sh stale-assignee race (original), IMPORTANT reclaim.sh comment pagination, IMPORTANT host_suffix domain-boundary bypass — none of the files implementing these were touched by this delta.
· Branch: claude/pocock-shipping-breakdown-ukohlx

[claude]

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

---

Security review complete

• Gather context (PR diff, prior review history, custom instructions)
• Invoke /review:security-review and follow its instructions
• Diff since last-reviewed head (ecc1acc) against current HEAD (bd88fea)
• Review changed files for security issues
• Adversarially verify the fix against the exact race it claims to close
• Post final findings

This is a re-review at HEAD bd88fea (last reviewed head was ecc1acc). Exactly one commit landed in between, bd88fea, touching only:

plugins/work-items/.claude-plugin/plugin.json                         |  2 +-
plugins/work-items/CHANGELOG.md                                       | 20 +++++++++++++
plugins/work-items/tools/work-item-tracker/adapters/linear/claim.sh      | 23 ++++++++++++----
plugins/work-items/tools/work-item-tracker/adapters/linear/claim.test.sh | 22 +++++++++++++++

This is a direct fix for the IMPORTANT finding this lane posted at the previous head (ecc1acc): the claim.sh rollback trap cleared the assignee unconditionally, which could strip a concurrent winner's live assignment if this session hit a transient failure after losing the race legitimately.

Fixed — verified by reading the code, not the commit message

IMPORTANT — claim.sh rollback trap could strip a concurrent winner's live assignment (adapters/linear/claim.sh#L116-L123)

_wit_linear_claim_rollback() no longer calls wit_linear_set_assignee "$ISSUE_UUID" "" unconditionally. It now re-fetches the issue (wit_linear_fetch_issue) and clears the assignee only if it still reads back as $HOLDER — the identical compare-before-clear pattern the LOSER branch already applies at line 174-177, and the pattern reclaim.sh was fixed to use earlier in this PR's history for the same class of bug.

Traced the race this closes: the trap stays armed across the wit_linear_update_comment write (line 141) and the AFTER arbitration read (line 146), both exit-on-failure per the prior commit's || exit "$?" additions — so a concurrent session can legitimately win the claim inside that window (posting its own lease, overwriting Linear's single assignee field) and then this session can hit a transient failure. Previously the rollback would blindly clear the winner's assignment on the way out; now it re-observes the current assignee first and backs off if it no longer matches $HOLDER.

New regression test (claim.test.sh#L186-L206) is genuinely discriminating: it seeds commentUpdate to fail (triggering the rollback trap) and seeds the rollback's re-fetch to report a different assignee (other-session) — the concurrent winner. It asserts the resulting assigneeId:null write count is 0. With the prior unconditional-clear code this would have produced 1 (the blind clear firing regardless of who currently holds the assignment); with the fix it correctly stays 0. This is the right shape of test — it exercises the compare failing, not just the compare succeeding.

Also confirmed the fix doesn't reopen the case it was originally patched for (assigned-with-no-lease strand): lines 200-208 (pre-existing case, unmodified by this commit) still asserts a single assigneeId:null write when the assignee legitimately is still this session at rollback time.

Not security-relevant (checked)

• plugin.json (0.39.1 → 0.39.2) and CHANGELOG.md — version bump and changelog entry accurately describing the fix, no logic.

No new findings

This closes the last IMPORTANT finding open on this PR. Every previously-reported CRITICAL/IMPORTANT finding across this PR's full review history (display_name shell injection, reclaim.sh stale-assignee race, reclaim.sh comment pagination, host_suffix domain-boundary bypass, claim.sh no-rollback strand, claim.sh swallowed-exit-in-subshell double-claim, and now this unconditional-clear rollback race) is fixed and verified against the actual source at this head. Nothing new surfaced in this delta.

Branch: claude/pocock-shipping-breakdown-ukohlx