fix(execute): the Phase 8 payload goes on stdin; /trekreview findings closed
/trekreview of 5680591..84a0621 (verdict WARN: 0 BLOCKER, 3 MAJOR, 1 MINOR,
1 MAJOR suppressed as unverified for length only). All closed red -> green:
- Suppressed MAJOR 1d0171a7 (verified live): Phase 8 spliced {plan-path}
into a single-quoted --payload. A lone quote broke the shell parse (event
lost, no written:false to report); a quoted pair ran a command
substitution and still reported written:true. event-emit now reads
`--payload -` from stdin and the three Phase 8 blocks pass the payload
through a quoted heredoc. Checked under bash and zsh.
- 570ba19b: the SC5 set-step extractor read line by line and never saw the
wrapped Step 0 sentinel (`blocked`); it now matches across the break and
the test asserts `blocked` is read.
- c69a865a: the Phase 9 line_ref in jsonl-schemas.md was 9 lines off. Both
trekexecute rows are now pinned to the exact span by a test.
- 0004fa23 (MINOR): the gates_mode_flag_override block says what to do on
written:false (print the reason, continue).
- emit() docstring no longer says failures are swallowed silently.
- 7ffaede5: chosen to KEEP the prose-only legacy `passed` tolerance, because
a public-plugin run started under the 6.1.0 prose would otherwise re-run
every finished step on resume; recorded as an accepted deviation in the
(local-only) brief. No code change.
Suite 1400 (1398/0/2), was 1393.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
84a0621fcc
commit
cb68c4f125
7 changed files with 130 additions and 28 deletions
|
|
@ -1645,18 +1645,18 @@ and operator tooling can see that the pipeline reached its terminal
|
|||
boundary:
|
||||
|
||||
```bash
|
||||
node ${CLAUDE_PLUGIN_ROOT}/lib/stats/event-emit.mjs \
|
||||
--event main-merge-gate \
|
||||
--payload '{"plan":"{plan-path}","wave_count":{wave-count}}'
|
||||
node ${CLAUDE_PLUGIN_ROOT}/lib/stats/event-emit.mjs --event main-merge-gate --payload - <<'JSON'
|
||||
{"plan":"{plan-path}","wave_count":{wave-count}}
|
||||
JSON
|
||||
```
|
||||
|
||||
Fill in the placeholders yourself; the payload is single-quoted, so the shell
|
||||
expands nothing in it (shell state from an earlier Bash call does not survive,
|
||||
and an expanded variable is how this event used to print `payload-not-json`).
|
||||
`{plan-path}` is the repo-relative plan path — a Voyage plan path
|
||||
(`.claude/projects/<date>-<slug>/plan.md` or
|
||||
`.claude/plans/trekplan-<date>-<slug>.md`) holds no quote character.
|
||||
`{wave-count}` is the number of merged waves, `1` on the single-session path.
|
||||
Fill in the placeholders yourself. The payload goes on stdin through a quoted
|
||||
heredoc, so the shell expands nothing in it: not a variable (shell state from an
|
||||
earlier Bash call does not survive, and an expanded variable is how this event
|
||||
used to print `payload-not-json`), and not a quote character or `$(...)` in the
|
||||
plan path. `{plan-path}` is the repo-relative plan path written as a JSON
|
||||
string (escape a `"` or `\` in it). `{wave-count}` is the number of merged
|
||||
waves, `1` on the single-session path.
|
||||
|
||||
Run each event call as its own command, never chained with `&&`. The writer
|
||||
prints `{"written": true, ...}`. On `"written": false` (exit 1), print its
|
||||
|
|
@ -1679,9 +1679,9 @@ and the merge that publishes the integrated result.
|
|||
|
||||
On confirm:
|
||||
```bash
|
||||
node ${CLAUDE_PLUGIN_ROOT}/lib/stats/event-emit.mjs \
|
||||
--event main-merge-approved \
|
||||
--payload '{"plan":"{plan-path}"}'
|
||||
node ${CLAUDE_PLUGIN_ROOT}/lib/stats/event-emit.mjs --event main-merge-approved --payload - <<'JSON'
|
||||
{"plan":"{plan-path}"}
|
||||
JSON
|
||||
```
|
||||
|
||||
On the multi-session wave path (Phase 2.6), and only when `next-wave` said
|
||||
|
|
@ -1695,14 +1695,13 @@ cleared:
|
|||
|
||||
On decline:
|
||||
```bash
|
||||
node ${CLAUDE_PLUGIN_ROOT}/lib/stats/event-emit.mjs \
|
||||
--event main-merge-declined \
|
||||
--payload '{"plan":"{plan-path}","reason":"hold-for-review"}'
|
||||
node ${CLAUDE_PLUGIN_ROOT}/lib/stats/event-emit.mjs --event main-merge-declined --payload - <<'JSON'
|
||||
{"plan":"{plan-path}","reason":"hold-for-review"}
|
||||
JSON
|
||||
```
|
||||
|
||||
The reason is the literal label of the one decline option, never the
|
||||
operator's free text (free text in a shell argument breaks the JSON and can be
|
||||
expanded by the shell).
|
||||
operator's free text (free text breaks the JSON).
|
||||
|
||||
On decline, nothing is cleared: the merged-waves record is kept.
|
||||
|
||||
|
|
@ -2010,7 +2009,9 @@ node ${CLAUDE_PLUGIN_ROOT}/lib/stats/event-emit.mjs \
|
|||
|
||||
The record lands in `trekexecute-stats.jsonl` in the plugin data directory as
|
||||
`{ts, event, known_event, payload:{flag, signal_effort}}`. The two payload keys
|
||||
stay in the jsonl; the OTel export allowlist drops them by omission.
|
||||
stay in the jsonl; the OTel export allowlist drops them by omission. On
|
||||
`"written": false` (exit 1), print its `reason` in one line and continue — the
|
||||
override record never blocks the run.
|
||||
|
||||
Standard effort (or absent): use `gates_mode = 'adaptive'` (the
|
||||
default). Low effort: `gates_mode = 'open'` plus sequential-only
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@
|
|||
//
|
||||
// CLI shim:
|
||||
// node lib/stats/event-emit.mjs --event brief-approved --payload '{...}'
|
||||
// node lib/stats/event-emit.mjs --event main-merge-gate --payload - <<'JSON' (payload on stdin)
|
||||
// → prints {"written":true,"path":...} exit 0 | {"written":false,"reason":...} exit 1
|
||||
// (usage error, payload-not-json and a failed write alike). The command prose
|
||||
// prints the reason and continues.
|
||||
|
|
@ -29,7 +30,7 @@
|
|||
// Import direction: event-emit imports stats-append; stats-append must never
|
||||
// import event-emit (that would be a cycle).
|
||||
|
||||
import { appendFileSync, existsSync, mkdirSync, realpathSync } from 'node:fs';
|
||||
import { appendFileSync, existsSync, mkdirSync, readFileSync, realpathSync } from 'node:fs';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { resolveStatsDir } from './stats-append.mjs';
|
||||
|
|
@ -69,8 +70,8 @@ export function buildRecord(event, payload = {}, now = new Date()) {
|
|||
}
|
||||
|
||||
/**
|
||||
* Emit an event. Never throws — stat failures are swallowed silently
|
||||
* because lifecycle telemetry must not block the user's workflow.
|
||||
* Emit an event. Never throws — a failed write is returned as {written:false,
|
||||
* reason} because lifecycle telemetry must not block the user's workflow.
|
||||
*
|
||||
* @returns {{ written: boolean, path: string | null, reason?: string }}
|
||||
*/
|
||||
|
|
@ -128,7 +129,8 @@ if (isMain()) {
|
|||
let payload = {};
|
||||
let bad = false;
|
||||
if (args.payload) {
|
||||
try { payload = JSON.parse(args.payload); } catch { bad = true; }
|
||||
// `-` = read it from stdin: a quoted heredoc passes it with nothing expanded.
|
||||
try { payload = JSON.parse(args.payload === '-' ? readFileSync(0, 'utf8') : args.payload); } catch { bad = true; }
|
||||
}
|
||||
result = bad ? { written: false, path: null, reason: 'payload-not-json' } : emit(args.event, payload);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -74,7 +74,6 @@ function writtenStatuses(text) {
|
|||
if (fresh) for (const m of fresh[1].matchAll(/`([^`]+)`/g)) add(n, m[1], 'run', 'fresh-mode');
|
||||
for (const m of l.matchAll(/first step (?:with|where) status != `([^`]+)`/g)) add(n, m[1], 'step', 'resume');
|
||||
for (const m of l.matchAll(/steps with `status=([a-z_-]+)`/g)) add(n, m[1], 'step', 'phase-7.5');
|
||||
for (const m of l.matchAll(/Set step status = [`"]([^`"]+)[`"]/g)) add(n, m[1], 'step', 'set-step');
|
||||
for (const m of l.matchAll(/steps\.\{N\}\.status = "([^"]+)"/g)) add(n, m[1], 'step', 'steps.{N}.status');
|
||||
if (/update progress/i.test(l)) {
|
||||
for (const m of l.matchAll(/`status: "([^"]+)"`/g)) add(n, m[1], 'run', 'update-progress');
|
||||
|
|
@ -84,6 +83,8 @@ function writtenStatuses(text) {
|
|||
});
|
||||
// The escalation rule spans a line break: "steps with\n`status: "X"`".
|
||||
for (const m of text.matchAll(/steps with\s+`status: "([^"]+)"`/g)) add(lineOf(m.index), m[1], 'step', 'escalation');
|
||||
// So does the Step 0 sentinel: "Set step\nstatus = `blocked`".
|
||||
for (const m of text.matchAll(/Set step\s+status = [`"]([^`"]+)[`"]/g)) add(lineOf(m.index), m[1], 'step', 'set-step');
|
||||
return out;
|
||||
}
|
||||
|
||||
|
|
@ -133,6 +134,10 @@ test('SC5: every status the prose tells the executor to write is in the validato
|
|||
assert.ok(sites.has(site), `the ${site} extractor found nothing - teach it the new wording`);
|
||||
}
|
||||
assert.equal(entries.filter((e) => e.site === 'resume').length, 2, 'both resume rules');
|
||||
// The Step 0 sentinel's "Set step\nstatus = `blocked`" wraps a line; the guard
|
||||
// above was satisfied by another set-step line while this one went unread.
|
||||
const setStep = entries.filter((e) => e.site === 'set-step').map((e) => e.value);
|
||||
assert.ok(setStep.includes('blocked'), `the Step 0 sentinel must be read; set-step saw: ${setStep.join(', ')}`);
|
||||
const updateLines = text.split('\n').filter((l) => /update progress/i.test(l));
|
||||
assert.ok(updateLines.length >= 5, `expected >= 5 "Update progress" sites, found ${updateLines.length}`);
|
||||
const withValue = new Set(entries.filter((e) => e.site === 'update-progress' || e.site === 'steps.{N}.status').map((e) => e.line));
|
||||
|
|
|
|||
4
tests/fixtures/jsonl-schemas.md
vendored
4
tests/fixtures/jsonl-schemas.md
vendored
|
|
@ -22,8 +22,8 @@
|
|||
| trekbrief-stats | ts, task, slug, mode, interview_turns, review_iterations, brief_quality, research_topics, auto_research, auto_result, intent_approved, project_dir, order_id, order_claimed_at, order_reason | commands/trekbrief.md (orchestrator-emit Phase 7; order_* added by lib/stats/stats-append.mjs) | trekbrief.md:961-976 | profile, phase_models, profile_source | none |
|
||||
| trekresearch-stats | ts, question, mode, scope, engine, slug, project_dir, brief_path, dimensions, dimensions_baseline, dimensions_baseline_preserved, effort, conv_turns, empty_turns, unique_sources, agents_local, agents_external, gemini_used, confidence, contradictions, open_questions | commands/trekresearch.md (orchestrator-emit Stats tracking) | trekresearch.md:634-676 | profile, phase_models, parallel_agents, external_research_enabled, profile_source | none |
|
||||
| trekplan-stats | ts, task, mode, slug, brief_path, project_dir, codebase_size, codebase_files, agents_deployed, deep_dives, research_briefs_used, research_scout_used, critic_verdict, guardian_verdict, outcome | commands/trekplan.md (orchestrator-emit Phase 12) | trekplan.md:805-826 | profile, phase_models, parallel_agents, profile_source | none |
|
||||
| trekexecute-stats (Phase 9 record) | ts, plan, plan_type, mode, result, steps_total, steps_passed, steps_failed, steps_skipped, failed_at_step | lib/stats/stats-append.mjs (trekexecute.md Phase 9) | trekexecute.md:1882-1903 | profile, phase_models, profile_source | none |
|
||||
| trekexecute-stats (autonomy events) | ts, event, known_event, payload | lib/stats/event-emit.mjs `emit()` | event-emit.mjs:77-98 | payload.profile, payload.phase_models, payload.profile_source | none |
|
||||
| trekexecute-stats (Phase 9 record) | ts, plan, plan_type, mode, result, steps_total, steps_passed, steps_failed, steps_skipped, failed_at_step | lib/stats/stats-append.mjs (trekexecute.md Phase 9) | trekexecute.md:1890-1912 | profile, phase_models, profile_source | none |
|
||||
| trekexecute-stats (autonomy events) | ts, event, known_event, payload | lib/stats/event-emit.mjs `emit()` | event-emit.mjs:78-99 | payload.profile, payload.phase_models, payload.profile_source | none |
|
||||
| trekexecute-stats (PostToolUse Bash) | ts, session_id, command_excerpt, duration_ms, success | hooks/scripts/post-bash-stats.mjs (Bash PostToolUse) | post-bash-stats.mjs:42-54 | none (hook is plugin-level, not profile-aware) | command_excerpt (CWE-212) |
|
||||
| trekreview-stats | ts, slug, verdict, counts (BLOCKER/MAJOR/MINOR/SUGGESTION), reviewed_files_count, mode, duration_ms | commands/trekreview.md (orchestrator-emit Phase 8) | trekreview.md:255 | profile, phase_models, profile_source | none |
|
||||
| trekcontinue-stats | ts, project, next_session_label, status | commands/trekcontinue.md (orchestrator-emit Phase 5) | trekcontinue.md:289 | profile, profile_source | none |
|
||||
|
|
|
|||
|
|
@ -2067,6 +2067,38 @@ test('order fields: the trekbrief-stats row\'s line_ref still points at the Phas
|
|||
assert.ok(at >= Number(m[1]) && at <= Number(m[2]), `line_ref ${ref} does not contain the stats-append line (line ${at})`);
|
||||
});
|
||||
|
||||
/** The jsonl-schemas fixture row whose schema_id cell is `id`, split into cells. */
|
||||
function schemaRow(id) {
|
||||
const row = read('tests/fixtures/jsonl-schemas.md').split('\n').find((l) => l.startsWith(`| ${id} |`));
|
||||
assert.ok(row, `jsonl-schemas.md is missing the \`${id}\` row`);
|
||||
return row.split('|').map((c) => c.trim());
|
||||
}
|
||||
|
||||
// The Phase 9 line_ref was measured, then more prose landed above Phase 9 and
|
||||
// nothing re-measured it. Pinned to the span it names: heading to closing fence.
|
||||
test('the trekexecute Phase 9 row\'s line_ref spans the Phase 9 heading to the end of its stdin block', () => {
|
||||
const ref = schemaRow('trekexecute-stats (Phase 9 record)')[4];
|
||||
const m = ref.match(/^trekexecute\.md:(\d+)-(\d+)$/);
|
||||
assert.ok(m, `line_ref "${ref}" is not trekexecute.md:A-B`);
|
||||
const lines = read('commands/trekexecute.md').split('\n');
|
||||
const heading = lines.findIndex((l) => /^## Phase 9\b/.test(l)) + 1;
|
||||
const append = lines.findIndex((l) => /^\s*node \$\{CLAUDE_PLUGIN_ROOT\}\/lib\/stats\/stats-append\.mjs trekexecute\b/.test(l)) + 1;
|
||||
const fence = lines.findIndex((l, i) => i >= append && l.trim() === '```') + 1;
|
||||
assert.ok(heading > 0 && append > heading && fence > append, 'Phase 9 heading, stats-append line and closing fence');
|
||||
assert.equal(ref, `trekexecute.md:${heading}-${fence}`);
|
||||
});
|
||||
|
||||
test('the autonomy-events row\'s line_ref spans emit() in event-emit.mjs', () => {
|
||||
const ref = schemaRow('trekexecute-stats (autonomy events)')[4];
|
||||
const m = ref.match(/^event-emit\.mjs:(\d+)-(\d+)$/);
|
||||
assert.ok(m, `line_ref "${ref}" is not event-emit.mjs:A-B`);
|
||||
const lines = read('lib/stats/event-emit.mjs').split('\n');
|
||||
const start = lines.findIndex((l) => l.startsWith('export function emit(')) + 1;
|
||||
const end = lines.findIndex((l, i) => i >= start && l === '}') + 1;
|
||||
assert.ok(start > 0 && end > start, 'emit() and its closing brace');
|
||||
assert.equal(ref, `event-emit.mjs:${start}-${end}`);
|
||||
});
|
||||
|
||||
test('order fields: docs/architecture.md Stats names them and the order-id join', () => {
|
||||
const text = read('docs/architecture.md');
|
||||
const stats = text.slice(text.indexOf('\nStats:'), text.indexOf('\n## Terminology'));
|
||||
|
|
|
|||
|
|
@ -198,6 +198,42 @@ test('CLI shim with malformed --payload returns reason payload-not-json (exit 1)
|
|||
}
|
||||
});
|
||||
|
||||
// `--payload -` reads the payload from stdin, so the command prose can pass it
|
||||
// through a quoted heredoc: the shell expands nothing, and a plan path with a
|
||||
// quote character can no longer close a single-quoted argument.
|
||||
test('CLI shim with --payload - reads the payload from stdin, quote characters and all', () => {
|
||||
const home = tmp();
|
||||
try {
|
||||
const plan = ".claude/projects/it's a'$(touch PWNED)'b/plan.md";
|
||||
const r = spawnSync(process.execPath, [SHIM, '--event', 'main-merge-gate', '--payload', '-'],
|
||||
{ encoding: 'utf-8', env: isolatedEnv(home), input: JSON.stringify({ plan, wave_count: 1 }) });
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
const [rec] = records(join(home, DATA_REL, 'trekexecute-stats.jsonl'));
|
||||
assert.deepEqual(rec.payload, { plan, wave_count: 1 });
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('CLI shim with --payload - and a stdin that is not JSON returns payload-not-json (exit 1)', () => {
|
||||
const home = tmp();
|
||||
try {
|
||||
const r = spawnSync(process.execPath, [SHIM, '--event', 'main-merge-gate', '--payload', '-'],
|
||||
{ encoding: 'utf-8', env: isolatedEnv(home), input: 'not-json{{' });
|
||||
assert.equal(r.status, 1, r.stdout + r.stderr);
|
||||
assert.equal(JSON.parse(r.stdout.trim()).reason, 'payload-not-json');
|
||||
assert.ok(!existsSync(join(home, DATA_REL, 'trekexecute-stats.jsonl')));
|
||||
} finally {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// The emit() docstring said failures are "swallowed silently" after the contract
|
||||
// became {written:false, reason}.
|
||||
test('emit() is not documented as swallowing failures silently', () => {
|
||||
assert.doesNotMatch(readFileSync(SHIM, 'utf-8'), /swallowed silently/);
|
||||
});
|
||||
|
||||
test('CLI: with CLAUDE_PLUGIN_DATA empty the event lands under HOME', () => {
|
||||
const home = tmp();
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -357,12 +357,13 @@ function fencedBlocksWith(sec, needle) {
|
|||
}
|
||||
|
||||
/** Fill the prose placeholders an executor fills in, then run the block. */
|
||||
function runBlock(block, home) {
|
||||
function runBlock(block, home, { planPath = '.claude/projects/2026-10-05-x/plan.md', cwd } = {}) {
|
||||
const cmd = block.split('${CLAUDE_PLUGIN_ROOT}').join(ROOT)
|
||||
.split('{plan-path}').join('.claude/projects/2026-10-05-x/plan.md')
|
||||
.split('{plan-path}').join(planPath)
|
||||
.split('{wave-count}').join('1');
|
||||
return spawnSync('bash', ['-c', cmd], {
|
||||
encoding: 'utf8',
|
||||
cwd,
|
||||
env: { ...baseEnv(), CLAUDE_PLUGIN_DATA: '', HOME: home },
|
||||
});
|
||||
}
|
||||
|
|
@ -409,6 +410,31 @@ test('Phase 8: each event block, run as written, lands one known event', () => {
|
|||
}
|
||||
});
|
||||
|
||||
// The plan path is operator-chosen (positional [plan.md], --project <dir>). Spliced
|
||||
// into a single-quoted argument, a lone quote broke the shell parse (event lost,
|
||||
// no written:false to report) and a quoted pair let a command substitution run.
|
||||
test('Phase 8: a plan path with a quote character lands literally and the shell runs nothing in it', () => {
|
||||
const blocks = fencedBlocksWith(section(TE, 'Phase 8'), 'event-emit.mjs');
|
||||
assert.equal(blocks.length, 3, 'main-merge-gate, main-merge-approved, main-merge-declined');
|
||||
for (const plan of [".claude/projects/2026-10-05-it's/plan.md", ".claude/projects/a'$(touch PWNED)'b/plan.md"]) {
|
||||
for (const block of blocks) {
|
||||
const home = mkdtempSync(join(tmpdir(), 'stats-landing-'));
|
||||
try {
|
||||
const r = runBlock(block, home, { planPath: plan, cwd: home });
|
||||
assert.ok(!existsSync(join(home, 'PWNED')), `the shell ran a command from the plan path:\n${block}`);
|
||||
assert.equal(r.status, 0, `${block}\n${r.stdout}${r.stderr}`);
|
||||
const [rec] = records(join(home, DATA_REL, 'trekexecute-stats.jsonl'));
|
||||
assert.equal(rec.payload.plan, plan);
|
||||
} finally { rmSync(home, { recursive: true, force: true }); }
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('gates_mode_flag_override: a written:false is printed in one line and the run continues', () => {
|
||||
const flat = section(TE, 'Composition rule (v5.1)').replace(/\s+/g, ' ');
|
||||
assert.match(flat, /On `"written": false` \(exit 1\), print its `reason` in one line and continue/);
|
||||
});
|
||||
|
||||
test('Phase 8: a written:false is printed in one line and the run continues', () => {
|
||||
const sec = section(TE, 'Phase 8');
|
||||
const flat = sec.replace(/\s+/g, ' ');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue