[pablof7z]

Summary

Fixes the e2e probe scenarios added in #116 (file modification tracking) and #117 (project hooks). All meaningful verdicts now pass.

• file-modification-tracking: pm agent must be generalist category (not orchestrator) — orchestrators have workspace_access_restricted = true so fs_write is never registered for them
• file-modification-tracking: cassette reordered so the more-specific second-run entry (containing fileModificationSecondRequest in history) is checked before the first-run entry, preventing false matches on shared conversation history
• file-modification-tracking + hooks-pre-tool: scenario drivers switch from waitForObservedEvent (relay push subscription, blocked by relay ACL deferral for non-whitelisted pubkeys) to waitForStoredMessage (conversation DB filesystem poll, unaffected by relay ACL)
• verdicts: requestDebug is a Rust {:?} format string — quotes inside are backslash-escaped; verdict checks updated to match type=\"file-modifications\" instead of type="file-modifications"

Test plan

Both scenarios run green:

• bun scripts/tenex-runtime-probe.ts file-modification-tracking → 4 ok, 0 bad
• bun scripts/tenex-runtime-probe.ts hooks-pre-tool → 4 ok, 0 bad

🤖 Generated with Claude Code

[claude]

Code Review

Overview

This PR fixes two failing e2e probe scenarios by addressing four distinct root causes: agent category mismatch, cassette false-match ordering, relay ACL blocking event delivery, and Rust {:?} debug-format escaping in verdict checks. The changes are focused and the PR description accurately explains each fix.

---

Correctness

generalist vs orchestrator category fix (tenex-runtime-probe.ts:315)
The core fix is sound — if workspace_access_restricted = true prevents fs_write from registering on orchestrators, then a scenario that tests file writes must use a generalist. The write-access skill addition is consistent with this.

Cassette reordering (tenex-runtime-probe-scenarios.ts)
Moving the second-run entry before the first-run entry is the right fix. The second-run match now includes fileModificationSecondRequest in containsAll, making it properly more specific. Without this ordering, a cassette matcher reading a cumulative history could match the first-run entry when it should have matched the second — a genuine false-positive risk.

waitForStoredMessage switch (both scenario files)
Switching from relay-subscription polling to conversation DB polling is a principled fix. The relay ACL deferral is a real external dependency that test infrastructure shouldn't need to work around; the DB poll is an internal observable that directly reflects agent state.

Rust {:?} escape fix (tenex-runtime-probe-verdicts.ts:432)
Correct. {:?} formats string fields with backslash-escaped inner quotes. The old check for type="file-modifications" would never match.

---

Issues

Unexplained removal of contains: "Successfully wrote" (tenex-runtime-probe-scenarios.ts, turn 2 entry)
The first-run turn-2 cassette entry previously matched on contains: "Successfully wrote". That check is dropped without explanation. This reduces the specificity of that cassette entry — any PM response at turn 2 will now match. If this was causing a false negative (the agent wasn't saying "Successfully wrote"), the PR description should explain why. If it was just cleanup, the specificity loss is worth noting.

Growing ternary chain in tenex-runtime-probe.ts
The skills assignment now has four arms:

: scenarioName === "project-membership-reload"
    ...
: scenarioName === "sign-as-user-nip46"
    ? { model: llmModelName, skills: ["signer"] }
: scenarioName === "file-modification-tracking"
    ? { model: llmModelName, skills: ["write-access"] }
: { model: llmModelName }

This is pre-existing but the PR extends it. Not a blocker, but a Record<ScenarioName, AgentConfig> map would be more readable and safer as new scenarios are added. At minimum, the category selection two lines above has the same multi-arm ternary problem and should be consistent.

Second-run waitForStoredMessage uses firstEvent.id as root (tenex-runtime-probe-scenarios.ts:1445)
This is correct if both events share the same conversation root, but that invariant is implicit. A brief comment explaining that the second event is a reply within the same thread (not a new conversation root) would prevent future confusion — this is genuinely a non-obvious protocol detail.

---

Comments

Several new inline comments explain what the code does rather than why:

• // First run turn 2: acknowledge the write. — the surrounding code already makes this clear
• // Second run belongs to the same conversation (root = firstEvent.id). — worth keeping; the relay vs DB root distinction is non-obvious

The // requestDebug is Rust {:?} format — quotes are backslash-escaped. comment in verdicts.ts is exactly the kind of non-obvious invariant that warrants a comment. Keep it.

---

Summary

The fixes are correct, targeted, and address real failure modes. The main actionable feedback is:

1. Explain the removal of contains: "Successfully wrote" — either in a comment or the PR description.
2. Consider whether the ternary chain for agent config has grown past the point of readability and whether a map structure is warranted.
3. The firstEvent.id-as-root usage in the second-run wait deserves a short explanatory comment.

No blocking issues. The scenario logic is sound and the relay→DB polling change improves test reliability.