Probe repo-local skill discovery and startup ordering across harnesses - #975
alexeyzimarev wants to merge 42 commits into
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ed (#961) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
#961) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Summary by QodoAdd cross-harness repo-local skill discovery probes
AI Description
Diagram
High-Level Assessment
Files changed (36)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| /usr/bin/git -C <worktree> push https://github.com/kurrent-io/kcap-cli.git alexeyzimarev/ai-2829-verify-repo-local-skill-discovery-and-startup-ordering | ||
| ``` | ||
|
|
||
| Open the PR with `gh pr create` following `.github/PULL_REQUEST_TEMPLATE.md`. Title: `Probe repo-local skill discovery and startup ordering across harnesses`. The reference line carries `Refs #961` (not a closing keyword: pass 2 closes it) and `AI-2829`. End the description with `🤖 Generated with [Claude Code](https://claude.com/claude-code)`. |
There was a problem hiding this comment.
5. The pull request leaves its issue open 📘 Rule violation § Compliance
The PR reference line uses Refs #961 — AI-2829 rather than one of the required GitHub closing keywords. GitHub will therefore treat #961 only as a reference when this branch merges, even though both issue identifiers appear together.
Agent Prompt
## Issue description
The PR description begins with `Refs #961 — AI-2829`, but the compliance requirement calls for one GitHub closing keyword and the Linear identifier on the same reference line.
## Fix Focus Areas
- docs/superpowers/plans/2026-09-16-ai2829-skills-discovery-probes.md[3620-3620]
## Recommended Fix
Change the planned and actual PR reference line to use a supported closing keyword, such as `Closes #961 AI-2829`, while keeping both identifiers on that single line.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| def parse_reply(reply_text: str, raw: str, name: str | None = None) -> Reply: | ||
| # The reply decides; the raw event log is consulted only when no reply text was extracted, | ||
| # because a tool-call frame that echoes SKILL.md would otherwise count as a loaded skill. | ||
| found = TOKEN_RE.findall(reply_text) if reply_text.strip() else TOKEN_RE.findall(raw) | ||
| # A reply that lists some other probe skill has not named this one. | ||
| named = (name in reply_text) if name else NAME_RE.search(reply_text) is not None | ||
| return Reply( | ||
| tokens=frozenset(found), | ||
| skill_named=named, | ||
| no_skill=NO_SKILL in reply_text, |
There was a problem hiding this comment.
4. Tool output can fake discovery 🐞 Bug ≡ Correctness
parse_reply scans the complete raw event stream whenever reply extraction is empty, so a skill token echoed by a file-read or tool-result frame becomes a reported token. A failed or incomplete Codex reply after reading SKILL.md can consequently be marked visible, and the same raw token can abort S0 as a prompt-design failure.
Agent Prompt
Issue description
The fallback token parser treats any raw vendor event as model output, including tool results that contain the skill body.
Fix Focus Areas
- docs/probes/2026-09-16-skills-discovery/lib/probe_skill.py[86-95]
- docs/probes/2026-09-16-skills-discovery/harness/codex.py[53-69]
Recommended Fix
Remove the unrestricted raw-stream token fallback, or pass a separately extracted agent-message fallback into `parse_reply`. Never scan raw tool-call or tool-result payloads for probe tokens; if no agent reply can be extracted, record the run as untested or not visible according to the driver failure state.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| def run_scenario(self, mode: str, scenario: str, arms: list[str] | None = None) -> list[RunRecord]: | ||
| gated = scenario not in ("S0", "S1") and not self.s1_ok.get(mode, True) | ||
| native = self.adapter.native_root | ||
| out: list[RunRecord] = [] | ||
| if scenario == "S0": | ||
| out += self.run_arm(lambda: self.arm_s0(mode), mode, "S0", "S0/none", None, "none") | ||
| elif scenario == "S1": | ||
| recs = self.run_arm(lambda: self.arm_s1(mode), mode, "S1", "S1/native", native, "none") | ||
| self.s1_ok[mode] = combine([r.verdict for r in recs])[0] == "visible_first_turn" |
There was a problem hiding this comment.
10. Selected scenarios bypass the baseline 🐞 Bug ≡ Correctness
run_scenario treats an absent S1 result as a successful native-root control through self.s1_ok.get(mode, True). Invoking the advertised --scenario S2, S3, or S4 option without S1, including in a later invocation with existing S1 records, spends turns and emits conclusions despite never establishing that the harness reads its native root.
Agent Prompt
Issue description
Targeted later scenarios default to passing the required S1 native-root control when S1 was not run in this `Runner` instance.
Fix Focus Areas
- docs/probes/2026-09-16-skills-discovery/probe.py[160-169]
- docs/probes/2026-09-16-skills-discovery/probe.py[339-381]
- docs/probes/2026-09-16-skills-discovery/probe.py[436-478]
Recommended Fix
Before S2, S3, or S4, load and evaluate settled S1 records for the requested mode, or run S1 automatically when no valid control exists. Block later scenarios when that control is absent, untested, or not visible.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| def wait_notification(self, method: str, timeout: float) -> dict | None: | ||
| deadline = time.time() + timeout | ||
| while time.time() < deadline: | ||
| msg = self.child.recv(max(0.1, deadline - time.time())) | ||
| if msg is None: | ||
| return None | ||
| self._absorb(msg) | ||
| if msg.get("method") == method: | ||
| return msg | ||
| return None |
There was a problem hiding this comment.
11. Completed daemon turns wait unnecessarily 🐞 Bug ☼ Reliability
_Rpc.request absorbs notifications received before the matching turn/start response, but wait_notification never checks the already absorbed notification list. When turn/completed arrives before that response, the app-server driver waits the full turn timeout and records turn=None although completion and reply events were already received.
Agent Prompt
Issue description
The app-server driver loses completion notifications for synchronization when they were buffered while waiting for a JSON-RPC response.
Fix Focus Areas
- docs/probes/2026-09-16-skills-discovery/lib/appserver_driver.py[17-46]
- docs/probes/2026-09-16-skills-discovery/lib/appserver_driver.py[97-117]
Recommended Fix
Make `wait_notification` search buffered notifications before reading new frames, consuming or marking the matched notification as appropriate. Continue to collect item notifications for reply extraction without waiting for a completion that was already received.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2618e435fa
ℹ️ 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".
| # Backgrounded commands get /dev/null on fd 0 unless explicitly redirected, so the real | ||
| # stdin is saved to fd 3 first and handed to the background reader from there. A vendor | ||
| # that hands the hook no stdin at all must not abort the script under set -e. | ||
| "{ exec 3<&0; } 2>/dev/null || exec 3</dev/null\n" |
There was a problem hiding this comment.
When a vendor launches the hook with fd 0 closed, /bin/sh treats the failed redirection on the exec special builtin as fatal and exits with status 2 before the || fallback runs. This makes test_script_tolerates_a_closed_stdin fail and can cause vendors to reject an otherwise completed startup hook; duplicate stdin to fd 3 using a construct that cannot terminate the current shell on redirection failure.
Useful? React with 👍 / 👎.
| plugin = sb.root / "plugin" | ||
| (plugin / ".cursor-plugin").mkdir(parents=True, exist_ok=True) | ||
| (plugin / ".cursor-plugin" / "plugin.json").write_text(json.dumps({"name": "kcap-probe", "version": "1.0.0"})) | ||
| target = plugin / "skills" / skill_file.parent.name / "SKILL.md" |
There was a problem hiding this comment.
For the Cursor registration arm, skill_file points into the repository's native root, but this replaces it with a plugin-local target and never creates the original path. After the turn, Runner.arm_s2 checks target.exists() for that original skill_file and forcibly changes the verdict to untested, so Cursor registration can never be measured successfully even if pluginPaths loads the generated skill.
Useful? React with 👍 / 👎.
| if r["scenario"] == "S4" and r["verdict"] in VISIBLE and r["root"]: | ||
| consumed_by[r["root"]].add(r["entry"]) |
There was a problem hiding this comment.
When an entry loads a root that it does not document, S4 records the verdict as leaked, but this index counts only the two visible verdicts. Consequently, if entry X legitimately consumes a root and entry Y leaks that same root, the report still labels it a vendor-isolated destination for X, producing an incorrect capability matrix precisely in the cross-vendor collision case that S4 is intended to detect.
Useful? React with 👍 / 👎.
| except Exception as ex: # noqa: BLE001 | ||
| aborted += 1 | ||
| print(f"{name}: aborted: {ex!r}") | ||
| return 1 if aborted and not completed else 0 |
There was a problem hiding this comment.
In a multi-harness sweep, any successful entry increments completed, causing the command to return 0 even when another entry raises an exception or fails its negative control. Automation therefore treats a partial, aborted experiment as successful and may emit incomplete results; the exit status should remain nonzero whenever aborted is nonzero.
Useful? React with 👍 / 👎.
| def list_catalogue(self, sb: Sandbox) -> str | None: | ||
| out = subprocess.run([self.binary_path() or self.binary, "skill", "list", "--json"], cwd=str(sb.repo), | ||
| env=sb.env, capture_output=True, text=True, timeout=120) | ||
| return out.stdout |
There was a problem hiding this comment.
This definition is indented inside classify_copilot_tools after its unconditional return, rather than being a method of CopilotAdapter. As a result, CopilotAdapter inherits the base no-op implementation and the free phase never runs copilot skill list --json, leaving the claimed catalogue corroboration absent from newly generated evidence.
Useful? React with 👍 / 👎.
…961) Their keyring logins are not visible from a private HOME, and a signed-out agy starts a browser sign-in on every launch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The json and text formats print nothing once the stream error ends a tool-using turn. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…turn (#961) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ols (#961) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A scenario an entry never ran renders as not run rather than none, so a blocked harness cannot read as a measured negative. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
Refs #961 — AI-2829 (pass 1 of 2; the pass 2 PR closes the issue)
What & why
Repeatable probes under
docs/probes/2026-09-16-skills-discovery/measure, per harness and launch mode, whether a repo-local skill written by a startup hook reaches the first model request, which Git exclusion keeps it loadable, and which vendor directories each harness reads. The verdict is the starting session's own reply: a token that exists only in the skill body.matrix.json,capability-matrix.mdandfindings.mdare the deliverables #778 and #962 consume; no production code changes.Where to look
findings.md"Consequences": a hook file drop reaches the first request only on Kiro (agentSpawn runs at session creation) and on OpenCode 1.x print into an existing root; Pi'sresources_discoverextension route works; Cursor's fire-and-forget hook races indexing; Claude, Codex, Copilot and agy index before hooks and offer no headless reload. Both.gitignoreandinfo/excludekeep skills loadable everywhere. Claude and Codex have no vendor-isolated destination. Gemini is untested on this account (OAuth tier refused) and OpenCode 2.0.4 loads no local plugin.Verification
Nine harnesses measured on macOS 26.6.2 arm64 (Claude 2.1.273, Codex 0.154.0, Pi 0.85.1, Copilot 1.0.85, Kiro 2.21.4, OpenCode 1.18.31 and 2.0.4, cursor-agent 2026.09.15, agy 1.2.4; Gemini 0.60.0 blocked on credentials), print and daemon modes: every verdict row carries two runs, hook stamps and the vendor's event stream.
🤖 Generated with Claude Code