feat(01KVS12K): A skill that helped a story reach done is credited as useful even when no READY-FOR-MERGE verdict event was recorded

plugins/flow/mcp-server/dist/cli.js

@@ -28502,7 +28502,7 @@ var SkillEffectivenessResultSchema = external_exports.object({
   attribution: SkillEffectivenessAttribution
 }).strict();
 async function computeSkillEffectiveness(opts) {
-  const { targetRepoRoot, window: rawWindow, readTelemetryDirImpl, readFileImpl } = opts;
+  const { targetRepoRoot, window: rawWindow, readTelemetryDirImpl, readFileImpl, readDoneRefsImpl } = opts;
   const window2 = rawWindow ?? DEFAULT_SKILL_EFFECTIVENESS_WINDOW;
   if (!Number.isFinite(window2) || !Number.isInteger(window2) || window2 <= 0) {
     throw new SkillEffectivenessWindowInvalidError({
@@ -28577,6 +28577,24 @@ async function computeSkillEffectiveness(opts) {
     return a2.session_id < b.session_id ? -1 : a2.session_id > b.session_id ? 1 : 0;
   });
   const windowedInvokes = sortedInvokes.slice(0, window2);
+  const doneDir = path11.join(targetRepoRoot, ".flow", "state", "done");
+  let doneRefs;
+  try {
+    if (readDoneRefsImpl) {
+      doneRefs = await readDoneRefsImpl(doneDir);
+    } else {
+      const entries = await fs9.readdir(doneDir, { withFileTypes: true });
+      doneRefs = new Set(
+        entries.filter((e) => e.isFile() && e.name.endsWith(".yaml")).map((e) => e.name.slice(0, -".yaml".length))
+      );
+    }
+  } catch (err) {
+    if (err !== null && typeof err === "object" && err.code === "ENOENT") {
+      doneRefs = /* @__PURE__ */ new Set();
+    } else {
+      throw err;
+    }
+  }
   const usefulVerdictsBySession = /* @__PURE__ */ new Map();
   const usefulVerdictsByStory = /* @__PURE__ */ new Map();
   let usefulVerdictCount = 0;
@@ -28623,6 +28641,9 @@ async function computeSkillEffectiveness(opts) {
           return true;
         });
       }
+      if (!isUseful && inv.story_id !== void 0) {
+        isUseful = doneRefs.has(inv.story_id);
+      }
     }
     if (isUseful) {
       entry.useful++;
@@ -28638,7 +28659,7 @@ async function computeSkillEffectiveness(opts) {
       skill_tier: counts.tier
     };
   }
-  const isAttributed = usefulVerdictCount > 0 || anyNonExecutionInvoke;
+  const isAttributed = usefulVerdictCount > 0 || anyNonExecutionInvoke || doneRefs.size > 0;
   return {
     per_skill,
     window_size: window2,

plugins/flow/mcp-server/dist/index.js

@@ -49487,7 +49487,7 @@ var SkillEffectivenessResultSchema = external_exports.object({
   attribution: SkillEffectivenessAttribution
 }).strict();
 async function computeSkillEffectiveness(opts) {
-  const { targetRepoRoot, window: rawWindow, readTelemetryDirImpl, readFileImpl } = opts;
+  const { targetRepoRoot, window: rawWindow, readTelemetryDirImpl, readFileImpl, readDoneRefsImpl } = opts;
   const window2 = rawWindow ?? DEFAULT_SKILL_EFFECTIVENESS_WINDOW;
   if (!Number.isFinite(window2) || !Number.isInteger(window2) || window2 <= 0) {
     throw new SkillEffectivenessWindowInvalidError({
@@ -49562,6 +49562,24 @@ async function computeSkillEffectiveness(opts) {
     return a2.session_id < b.session_id ? -1 : a2.session_id > b.session_id ? 1 : 0;
   });
   const windowedInvokes = sortedInvokes.slice(0, window2);
+  const doneDir = path39.join(targetRepoRoot, ".flow", "state", "done");
+  let doneRefs;
+  try {
+    if (readDoneRefsImpl) {
+      doneRefs = await readDoneRefsImpl(doneDir);
+    } else {
+      const entries = await fs32.readdir(doneDir, { withFileTypes: true });
+      doneRefs = new Set(
+        entries.filter((e) => e.isFile() && e.name.endsWith(".yaml")).map((e) => e.name.slice(0, -".yaml".length))
+      );
+    }
+  } catch (err) {
+    if (err !== null && typeof err === "object" && err.code === "ENOENT") {
+      doneRefs = /* @__PURE__ */ new Set();
+    } else {
+      throw err;
+    }
+  }
   const usefulVerdictsBySession = /* @__PURE__ */ new Map();
   const usefulVerdictsByStory = /* @__PURE__ */ new Map();
   let usefulVerdictCount = 0;
@@ -49608,6 +49626,9 @@ async function computeSkillEffectiveness(opts) {
           return true;
         });
       }
+      if (!isUseful && inv.story_id !== void 0) {
+        isUseful = doneRefs.has(inv.story_id);
+      }
     }
     if (isUseful) {
       entry.useful++;
@@ -49623,7 +49644,7 @@ async function computeSkillEffectiveness(opts) {
       skill_tier: counts.tier
     };
   }
-  const isAttributed = usefulVerdictCount > 0 || anyNonExecutionInvoke;
+  const isAttributed = usefulVerdictCount > 0 || anyNonExecutionInvoke || doneRefs.size > 0;
   return {
     per_skill,
     window_size: window2,

plugins/flow/mcp-server/src/tools/__tests__/compute-skill-effectiveness.test.ts

@@ -442,6 +442,120 @@ describe("computeSkillEffectiveness — AC3 edges", () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// Done/-manifest attribution — Story native:01KVS12K AC1 + AC2
+// ---------------------------------------------------------------------------
+
+describe("computeSkillEffectiveness — done/-manifest attribution (Story native:01KVS12K)", () => {
+  it("AC1: credits a done-but-no-verdict-event invoke as a useful fire via done/-manifest read", async () => {
+    // A skill.invoke whose story_id reached done/ but has NO joined READY FOR
+    // MERGE reviewer.verdict in telemetry — the done/-manifest is the only signal.
+    // Before this fix: useful_fire_count 0 → retro wrongly proposes retire/revise.
+    // After this fix: useful_fire_count 1, effectiveness_ratio 1.
+    const doneStoryRef = "native:01DONE0NOVERDICT00000001";
+    const events = [
+      makeInvoke({ ts: makeTs(1000), session_id: "s-done-1", skill_name: "flow:run", story_id: doneStoryRef }),
+      // No READY FOR MERGE verdict for this story — only the done/ manifest signals completion.
+    ].map(assertValid);
+
+    const result = await computeSkillEffectiveness({
+      targetRepoRoot: ROOT,
+      ...seams({ "2026-06.jsonl": events }),
+      readDoneRefsImpl: async () => new Set([doneStoryRef]),
+    });
+
+    expect(SkillEffectivenessResultSchema.safeParse(result).success).toBe(true);
+    expect(result.per_skill["flow:run"]).toEqual({
+      invoke_count: 1,
+      useful_fire_count: 1,
+      effectiveness_ratio: 1,
+      skill_tier: "execution",
+    });
+    // Done refs present → signal is "attributed", not "no-completed-flows".
+    expect(result.attribution).toBe("attributed");
+  });
+
+  it("AC2a: a not-done, no-verdict invoke is NOT credited (genuinely unhelpful skill stays 0)", async () => {
+    // A skill.invoke whose story is neither in done/ nor has a joined READY FOR
+    // MERGE verdict must remain uncredited — the done/ path must not over-credit.
+    const notDoneStoryRef = "native:01NOTDONE0000000000001";
+    const events = [
+      makeInvoke({ ts: makeTs(1000), session_id: "s-notdone-1", skill_name: "flow:run", story_id: notDoneStoryRef }),
+      // No verdict and story NOT in done/.
+    ].map(assertValid);
+
+    const result = await computeSkillEffectiveness({
+      targetRepoRoot: ROOT,
+      ...seams({ "2026-06.jsonl": events }),
+      readDoneRefsImpl: async () => new Set(), // empty done set
+    });
+
+    expect(SkillEffectivenessResultSchema.safeParse(result).success).toBe(true);
+    expect(result.per_skill["flow:run"]).toEqual({
+      invoke_count: 1,
+      useful_fire_count: 0,
+      effectiveness_ratio: 0,
+      skill_tier: "execution",
+    });
+    // No done refs, no verdicts, no planning/cockpit → "no-completed-flows".
+    expect(result.attribution).toBe("no-completed-flows");
+  });
+
+  it("AC2b: a skill.invoke that already joins a READY-FOR-MERGE verdict still counts as a useful fire exactly as today", async () => {
+    // The existing verdict-join path must be unaffected by the done/-manifest
+    // augmentation. An invoke followed by a READY FOR MERGE verdict (regardless of
+    // whether the story also has a done/ manifest) must still be credited.
+    const storyRef = "native:01VERDICTANDDO00000001";
+    const events = [
+      makeInvoke({ ts: makeTs(1000), session_id: "s-v-d", skill_name: "flow:run", story_id: storyRef }),
+      makeVerdict({ ts: makeTs(2000), session_id: "run-ulid-v-d", pr_number: 42, verdict: "READY FOR MERGE", story_id: storyRef }),
+    ].map(assertValid);
+
+    // Provide both: verdict in telemetry AND story in done/ — must still credit once.
+    const result = await computeSkillEffectiveness({
+      targetRepoRoot: ROOT,
+      ...seams({ "2026-06.jsonl": events }),
+      readDoneRefsImpl: async () => new Set([storyRef]),
+    });
+
+    expect(SkillEffectivenessResultSchema.safeParse(result).success).toBe(true);
+    expect(result.per_skill["flow:run"]).toEqual({
+      invoke_count: 1,
+      useful_fire_count: 1,     // credited exactly once (verdict join wins; done/ is redundant but harmless)
+      effectiveness_ratio: 1,
+      skill_tier: "execution",
+    });
+    expect(result.attribution).toBe("attributed");
+  });
+
+  it("done/-manifest ENOENT is treated as empty set (no done refs), not an error", async () => {
+    // Repos without a done/ dir yet should behave as if there are no done refs —
+    // consistent with the empty-telemetry-dir posture.
+    const events = [
+      makeInvoke({ ts: makeTs(1000), session_id: "s-enoent", skill_name: "flow:run", story_id: "native:01SOMEREF000" }),
+    ].map(assertValid);
+
+    // Simulate ENOENT on the done/ dir via the injected seam.
+    const result = await computeSkillEffectiveness({
+      targetRepoRoot: ROOT,
+      ...seams({ "2026-06.jsonl": events }),
+      readDoneRefsImpl: async () => {
+        const err = new Error("no done dir") as NodeJS.ErrnoException;
+        err.code = "ENOENT";
+        throw err;
+      },
+    });
+
+    // Should not throw; the invoke is uncredited (no verdict, no done manifest).
+    expect(result.per_skill["flow:run"]).toEqual({
+      invoke_count: 1,
+      useful_fire_count: 0,
+      effectiveness_ratio: 0,
+      skill_tier: "execution",
+    });
+  });
+});
+
 // ---------------------------------------------------------------------------
 // Tier-aware scoring — Story native:01KVEYY1 AC1 + AC2 + AC3
 // ---------------------------------------------------------------------------

plugins/flow/mcp-server/src/tools/__tests__/skill-effectiveness-attribution.int.test.ts

@@ -144,6 +144,103 @@ afterEach(() => {
   rmSync(tmpRoot, { recursive: true, force: true });
 });
 
+describe("skill-effectiveness done/-manifest attribution (Story native:01KVS12K)", () => {
+  // This describe block uses its OWN isolated temp dir (doneTestRoot) — completely
+  // separate from the shared `tmpRoot` set up by the outer beforeEach. The shared
+  // `tmpRoot` always contains an in-progress manifest for STORY_REF, so using it
+  // would leave two in-progress manifests at once, making resolveActiveStoryRef
+  // return undefined (ambiguous) and breaking story_id attribution.
+  let doneTestRoot: string;
+
+  beforeEach(() => {
+    doneTestRoot = mkdtempSync(path.join(os.tmpdir(), "skill-attr-done-"));
+  });
+
+  afterEach(() => {
+    rmSync(doneTestRoot, { recursive: true, force: true });
+  });
+
+  it("AC1: credits a skill.invoke as a useful fire when its story reached done/ but NO READY-FOR-MERGE verdict event was recorded", async () => {
+    // Scenario: /flow:run fires for a story, the story reaches done/ (manifest
+    // lands in .flow/state/done/) but the run path produced NO reviewer.verdict
+    // READY FOR MERGE event in telemetry — e.g. auto-merge gate or operator merge.
+    //
+    // Before this fix, computeSkillEffectiveness would score useful_fire_count: 0
+    // and the retro might draft a false retire/revise proposal.
+    // After this fix, the done/-manifest read credits the invoke as useful.
+
+    const harnessSession = "01HZDONE0MANIFEST000000TEST";
+    const storyRef = "native:01HZDONE0STORYREF000000001";
+
+    // Minimal .flow/config.yaml so the capture seam's cwd-derived root is valid.
+    await fs.mkdir(path.join(doneTestRoot, ".flow"), { recursive: true });
+    await atomicWriteFile(
+      path.join(doneTestRoot, ".flow", "config.yaml"),
+      "adapter: native\nadapter_config: {}\n",
+    );
+
+    // Write a single in-progress manifest for storyRef so resolveActiveStoryRef
+    // returns storyRef unambiguously (exactly ONE in-progress manifest → attribute).
+    const inProgressDir = path.join(doneTestRoot, ".flow", "state", "in-progress");
+    await fs.mkdir(inProgressDir, { recursive: true });
+    // Use the raw ref as the filename — matching moveBetweenStates convention.
+    await atomicWriteFile(
+      path.join(inProgressDir, storyRef + ".yaml"),
+      `ref: "${storyRef}"\nstatus: in-progress\nclaimed_by: "${harnessSession}"\n`,
+    );
+
+    // Write a real skill.invoke event via the capture seam. Since there is exactly
+    // ONE in-progress manifest, the capture seam resolves storyRef and stamps it as
+    // story_id on the skill.invoke telemetry event.
+    const captured = await captureSkillInvoke(
+      {
+        hook_event_name: "PreToolUse",
+        tool_name: "Skill",
+        tool_input: { skill: "flow:run", args: "" },
+        session_id: harnessSession,
+        cwd: doneTestRoot,
+      },
+      {
+        recordImpl: (opts) =>
+          recordSkillInvoke({
+            ...opts,
+            now: () => new Date("2020-02-01T00:00:00.000Z"),
+          }),
+        pluginRoot: undefined,
+      },
+    );
+    expect(captured).toEqual({ recorded: true });
+
+    // Write a done/ manifest for the story — this is the only signal that the
+    // story completed. There is NO reviewer.verdict READY FOR MERGE event.
+    // The done-manifest file name is the raw story ref + ".yaml" — matching the
+    // real convention used by moveBetweenStates / completeStory (which writes
+    // `<stateRoot>/<state>/<ref>.yaml` using the ref as-is, NOT sanitised).
+    // See manifest-state-machine.ts lines 99-100.
+    const doneDir = path.join(doneTestRoot, ".flow", "state", "done");
+    await fs.mkdir(doneDir, { recursive: true });
+    await atomicWriteFile(
+      path.join(doneDir, storyRef + ".yaml"),
+      `ref: "${storyRef}"\nstatus: done\n`,
+    );
+
+    // Run the scorer — it should credit the invoke via the done/ manifest even
+    // though NO READY FOR MERGE verdict event exists in telemetry.
+    const result = await computeSkillEffectiveness({ targetRepoRoot: doneTestRoot });
+
+    // The invoke carries storyRef as story_id; the done/ manifest for storyRef is
+    // present; therefore useful_fire_count must be 1 and ratio > 0.
+    expect(result.per_skill["flow:run"]).toEqual({
+      invoke_count: 1,
+      useful_fire_count: 1,
+      effectiveness_ratio: 1,
+      skill_tier: "execution",
+    });
+    // Attribution must be "attributed" — done/ signals completion.
+    expect(result.attribution).toBe("attributed");
+  });
+});
+
 describe("skill-effectiveness attribution end-to-end (issue #390)", () => {
   it("credits a useful fire across the divergent harness/run session namespaces", async () => {
     // 1) REAL capture seam: a programmatic skill invocation whose session id is