[kanfil]

Summary

resolveEvents() dropped canonical session_end for OpenCode (registry mapped it to null), so team-levelup never fired at session teardown. session_end now maps to the documented native session.deleted event, with fire-and-forget dispatcher side-effect runners in both plugin entrypoints:

• V1 generic event handler guards event.type === \"session.deleted\"
• V2 ctx.event.subscribe() loop guards the same native type in a dedicated subscription

session.deleted and file.edited share the event namespace but each block type-guards its own native type, so the two streams never mix. Session-end output has no context-injection target (the session is over), so dispatcher output is discarded — scripts only.

Tests

• Added 3 OpenCode session_end regression tests; watched 2 fail correctly before the fix (resolution dropped the event, no plugin emitted).
• node --test tests/events.test.mjs: 74/74 pass.
• Full suite node --test: 329/329 pass.

Docs

• docs/events-schema.md and README.md updated for the OpenCode session.deleted delivery and scripts-only semantics.

Scope note

This makes the session-end hook actually fire and run the skill script path. team-levelup's full interactive CDR extraction/review/publish flow still needs an interactive /team-levelup run; the teardown hook guarantees the deterministic script side-effect runs instead of being silently dropped.

[kanfil]

factory-review: review of 4cfb1d3: not merge-ready (1 Important, 3 Nit)

Reviewed revision: head 4cfb1d36fba9e44ea47236e25f9abcd5fb62e513 · base db2389610a712c979ffe67486f6e5c99e40f0287 · isolated detached checkout of exact head. CI on this head: 10/10 green.

🔴 Important

1. V1 server() object emits two "event" keys — the session.deleted handler is dead when file_edited is also configured (bugs: silently dropped handler)
buildOpenCodeHooks pushes one "event": async ... entry for sessionDeletedRuns (src/events.mjs:548) and a second for fileEditedRuns (:555), joined into a single object literal (:572, spliced into server() at :331). In JS, duplicate keys collapse — the second wins. Verified empirically on this head: a manifest with both session_end and file_edited generates keys ["event","event","experimental.chat.messages.transform"], and at runtime the first handler is lost. Since the session.deleted entry is pushed first, the file.edited handler overwrites it: the PR's flagship behavior (team-levelup at session end) silently never fires in exactly the configuration the README documents (per-agent delivery tables list both events). The new test session_end and file_edited share the V1 event handler namespace... only counts string occurrences of "event": async (passes 2/2) and cannot see the runtime overwrite.
Fix (preserve-only, one entry): emit a single "event" handler whose body dispatches on event.type — if (type === "session.deleted") {...} ; if (type === "file.edited") {...} — so both streams coexist. The existing per-block guards already have the right shape; they just need to live in one object entry.

💬 Nit (advisory)

2. Two concurrent V2 ctx.event.subscribe() loops; broadcast semantics unverified. The diff adds a second for await (const event of ctx.event.subscribe()) loop next to the file_edited one (src/events.mjs:422,431). If opencode's subscribe() is a broadcast, both loops see all events (fine); if it is a shared queue, the two loops split the stream and each misses the other's events. The pre-PR code had exactly one loop, so this question is new. Suggested: confirm against the plugin index.d.ts event.subscribe contract, or merge into one loop with a type-switch — mirroring the single-entry fix for finding 1.

3. runEvent is synchronous (execFileSync + per-skill timeout), so "fire-and-forget" blocks the event loop. The V2 session.deleted body calls runEvent(...) un-awaited inside for await, but runEvent (src/events.mjs:263) blocks up to timeout+5s per skill. On teardown the host may SIGKILL the plugin mid-run; a genuinely async spawn (or documenting the blocking bound) would match the comment. Cosmetic relative to finding 1 — no behavior change proposed beyond the comment or a spawn-based runner.

4. Scope note (informational, not a finding): the PR body correctly states team-levelup's interactive extraction still needs a /team-levelup run — the teardown path only guarantees the script side-effect. README's session_end envelope row ("no | yes") vs the new "scripts only, no context injection" delivery note is consistent with this. No action.

Checked

• security: skill/canonical names reach generated TS only via JSON.stringify (no injection); dispatcher invoked via execFileSync argv array; event payload travels on stdin; no payload is logged (session.deleted output discarded by design). Nothing found.
• spec-compliance: stop: null untouched (only session_end remapped); other agents' session_end mappings unchanged (SessionEnd/sessionEnd); docs (README.md, docs/events-schema.md) match the code's scripts-only semantics.
• test-integrity: 3 new regression tests assert resolution mapping + both V1/V2 handler presence + guard separation; tests/events.test.mjs 74/74 green locally on this head; full CI green. No weakened assertions.
• ci-integrity: no workflow files changed.

Not checked

• Whether session.deleted is the exact native type name in the installed opencode plugin API version (index.d.ts not vendored in this repo) — taken from the PR's claim; a plugin dist grep on the consumer side would close it.
• Windows/macOS runtime behavior of the second subscribe loop (static review only; CI is green).

This verdict is advisory only. It does not approve or merge; a human code-owner approves. Any push makes it stale.

[kanfil]

Self-heal status (run pr-66-selfheal-0, iter 0)

Review verdict (already on bus): important-found — F1 (V1 duplicate "event" key drops the session.deleted handler) + 3 advisory Nits, at head 4cfb1d3.

Lane disclosure: no subagent-dispatch tool in this harness — review/fix/converge all run inline (degraded from agent lane). No cross-runtime review either (cli:<other-runtime> unavailable → would have been agent anyway). Proceeding inline; converge re-verifies empirically.

Next: fix step — single "event" handler with type dispatch in buildOpenCodeHooks (F1 only; Nits advisory, untouched).

[kanfil]

Self-heal fix (run pr-66-selfheal-0, iter 0)

Fixed F1 (Important): V1 buildOpenCodeHooks emitted two "event" object keys when session_end+file_edited were both configured; duplicate-key semantics silently dropped the session.deleted handler. Merged into a single "event" entry with if (event.type === ...) dispatch for both blocks. Standalone messages.transform drainer preserved.

Validation: tests/events.test.mjs 74/74 green locally; negative control (new tests vs old code) fails 3 as expected. Updated 3 guard assertions to pin single-entry === semantics — test-only change.

Pushed: 4cfb1d3 → b9382cc (--force-with-lease on observed remote SHA). CI pending on new head.

[kanfil]

factory-review re-review of b9382cc: merge-ready (0 Important, 3 Nit)

Reviewed revision: head b9382cc3ddc171bb3486f6c0542b598083cc856a · base db2389610a712c979ffe67486f6e5c99e40f0287 · isolated detached checkout. Delta vs 4cfb1d3: F1 fix only (src/events.mjs + 3 test assertions) + nothing else. CI on this head: 10/10 green.

Previous findings disposition

• F1 (Important) RESOLVED: V1 emits exactly one "event" entry (grep -c = 1); both sessionDeletedRuns and fileEditedRuns dispatch inside it under event.type === guards. Verified at runtime: keys ["event","experimental.chat.messages.transform"], both guards + both skill runs present. Regression tests pin single-entry === semantics (RED-verified: fail 3 on old code, 74/74 on fixed).
• Nits 2–4 (advisory) carried forward unchanged: V2 dual subscribe() loops (Nit 2), sync runEvent under "fire-and-forget" label (Nit 3), team-levelup scope note (Nit 4).

Passes on new head

• bugs — pass. No duplicate keys; standalone transform drainer preserved for file_edited-without-session_start; handler ordering (session.deleted block first) irrelevant — both guarded.
• security — pass. No new exec/network/secret surface; guards are string literals.
• spec-compliance — pass. Docs unchanged and still accurate; mapping table untouched.
• test-integrity — pass. 3 assertion updates match the new guard text exactly; no assertions weakened (single-entry count went 2→1 to match the fix).
• ci-integrity — pass. No workflow changes.

This verdict is advisory only. It does not approve or merge; a human code-owner approves.

[kanfil]

Self-heal converge (run pr-66-selfheal-0, iter 0): DONE

• F1 independently reproduced against fixed head (single entry, both guards live at runtime). Fix Agent's RED/GREEN evidence confirmed.
• 74/74 events tests green on new head; full-matrix CI 10/10 green.
• No new Important findings; previous Nits advisory-only. Finding set shrank 1 → 0 (progress, no regression).
• 🟢 PASS — Ready for human review. Human code-owner approval + merge still required. validation label retained per skill (lifecycle stage, not verdict).