fix(plugin): refuse consuming a non-existent directive; correct the tool-result comment

directive/consume now validates the directiveId names a real directive on the
ticket and refuses (404) otherwise instead of appending an orphan marker and
reporting success; consuming a real directive stays idempotent. The dart-mcp
toToolResult comment now accurately states it returns the hub's response
payload verbatim.

Author: olehsvyrydov

dart-mcp/src/server.js

@@ -47,7 +47,10 @@ function toZodShape(input, z) {
 
 // Shape a hub `{ code, payload }` result into an MCP tool result. The payload is returned
 // as quoted JSON data; an error/refusal/conflict is surfaced as text, never thrown into an
-// execution path. No tool argument is echoed back.
+// execution path. The text is the hub's response payload verbatim — for routes like
+// ticket/comment that means the persisted record, which echoes caller-provided fields
+// (e.g. the comment body) so the caller can confirm what was written. This is the hub's
+// authoritative response, not a secret leak: no credentials live in these payloads.
 function toToolResult({ code, payload }) {
   const isError = !(payload && payload.ok);
   return {

hub/lib/api.js

@@ -155,8 +155,16 @@ async function handle(route, data, project) {
       // marker), so this is idempotent: a second consume just appends another harmless
       // marker and the directive stays non-pending. Surfacing never clears a directive.
       const { id, directiveId, by, note } = data;
-      if (!findTicket(id)) return bad('unknown ticket');
+      const ticket = findTicket(id);
+      if (!ticket) return bad('unknown ticket');
       if (typeof directiveId !== 'string' || !directiveId) return bad('directiveId required');
+      // The directiveId must name a real directive ON THIS TICKET, derived from the SAME
+      // comment log pendingDirectives reads — a kind:"directive" record carrying that id.
+      // Existence (not pending-ness) is the check: an already-consumed directive still
+      // exists, so re-consuming it is the idempotent no-op above; an id matching NO
+      // directive (e.g. a typo) is refused so no orphan marker accrues.
+      const exists = (ticket.comments || []).some((c) => c && c.kind === 'directive' && c.id === directiveId);
+      if (!exists) return { code: 404, payload: { ok: false, error: 'unknown directive' } };
       const comment = w.appendComment(project, id, {
         author: by || 'hub',
         kind: 'directive-consumed',

hub/test/directives.test.js

@@ -291,6 +291,54 @@ test('N3: consuming the same directive twice is idempotent (still no pending)',
   }
 });
 
+test('N3: consuming a directiveId that matches NO directive on the ticket is refused (404, nothing appended)', async () => {
+  const dir = fixture({ 'T-1': { title: 'A', track: 'full', stage: 'code_review' } });
+  try {
+    // a real directive exists, but the caller references a different (typo) id
+    appendComment(dir, 'T-1', { author: '/rev', kind: 'directive', body: 'do X', target: ['/be'] });
+    const file = path.join(dir, '.aidevteam', 'comments', 'T-1.jsonl');
+    const before = fs.readFileSync(file, 'utf8');
+    const res = await api.handle('directive/consume', { id: 'T-1', directiveId: 'no-such-id', by: '/be' }, dir);
+    assert.equal(res.code, 404, 'an unknown directiveId is refused');
+    const after = fs.readFileSync(file, 'utf8');
+    assert.equal(after, before, 'nothing was appended — the JSONL is byte-identical');
+  } finally {
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('N3: consuming a real directive returns 200, appends the marker, and drops it from pending', async () => {
+  const dir = fixture({ 'T-1': { title: 'A', track: 'full', stage: 'code_review' } });
+  try {
+    const d = appendComment(dir, 'T-1', { author: '/rev', kind: 'directive', body: 'do X', target: ['/be'] });
+    const res = await api.handle('directive/consume', { id: 'T-1', directiveId: d.id, by: '/be' }, dir);
+    assert.equal(res.code, 200);
+    const marker = readComments(dir, 'T-1').find((c) => c.kind === 'directive-consumed' && c.ref === d.id);
+    assert.ok(marker, 'a typed consumed marker was appended for the real directive');
+    const t1 = buildState(dir).tickets.find((t) => t.id === 'T-1');
+    assert.equal(t1.pendingDirectives.length, 0, 'the consumed directive is no longer pending');
+  } finally {
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('N3: consuming an already-consumed real directive stays 200 (idempotent, no error)', async () => {
+  const dir = fixture({ 'T-1': { title: 'A', track: 'full', stage: 'code_review' } });
+  try {
+    const d = appendComment(dir, 'T-1', { author: '/rev', kind: 'directive', body: 'do X', target: ['/be'] });
+    const first = await api.handle('directive/consume', { id: 'T-1', directiveId: d.id, by: '/be' }, dir);
+    assert.equal(first.code, 200);
+    // the directive still EXISTS on the ticket, so a repeat consume is a harmless no-op,
+    // NOT refused — existence is what's validated, not pending-ness
+    const second = await api.handle('directive/consume', { id: 'T-1', directiveId: d.id, by: '/be' }, dir);
+    assert.equal(second.code, 200, 'a repeat consume of an existing directive is idempotent, not a 404');
+    const t1 = buildState(dir).tickets.find((t) => t.id === 'T-1');
+    assert.equal(t1.pendingDirectives.length, 0);
+  } finally {
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+});
+
 test('N3b: rendering the digest mutates nothing (no auto-clear on read)', () => {
   const dir = fixture({ 'T-1': { title: 'A', track: 'full', stage: 'code_review', assignee: '/rev' } });
   try {