mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-29 04:55:12 +02:00
fix(plan-canvas): deliver browser chat to the agent every time (#2739)
Feedback sent from the canvas only reached an agent through a live /api/await long poll. When a turn ended with no await parked, queueFeedback wrote the message to sessions.json and nothing ever consumed it, so sending appeared to do nothing at all. The presence pill made it worse: workingKeys had no expiry and the feedback handler never broadcast presence, so it froze on "agent working" while nobody was listening. Delivery: - Add the stop:plan-canvas-pending hook. It drains undelivered feedback and blocks the Stop, handing the messages to the agent, so a canvas message lands even when no await is running. Scoped to sessions under cwd so parallel agents cannot swallow each other's feedback; set ECC_PLAN_CANVAS_STOP_SCOPE=all to widen. Honors stop_hook_active and fails open on every error path. - run-with-flags.js did not await a hook's run(), so any async hook silently degraded to pass-through. Fixed; plan-canvas-pending is the only async hook today. Presence and indicators: - Presence is now ended/typing/thinking/listening/queued/waiting. thinking and typing self-expire (90s/30s) and a 5s sweep pushes the decay to an idle browser, so the pill can no longer stick. - Broadcast presence when feedback is queued, and clear the activity state when an agent reply lands. - Add POST /api/session/:key/typing so agents can drive the indicator. - Chat shows an animated dots bubble for thinking and typing, plus an explicit note when a message is queued with nobody listening. Respects prefers-reduced-motion. - Send status reports what actually happened instead of always claiming the agent will pick it up. CLI and skill: - Add `ecc-plan-canvas pending` and `typing <file> --state ...`. - SKILL.md documents background await as the primary pattern and makes replying in the canvas mandatory. Tests: 6 new server cases covering queued presence, the typing endpoint, state expiry and the sweep, plus a new hook suite covering delivery, drain-once, stop_hook_active, cwd scoping and fail-open. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,194 @@
|
||||
/**
|
||||
* Integration tests for scripts/hooks/plan-canvas-pending.js (Stop)
|
||||
*
|
||||
* The hook is the delivery guarantee for canvas chat: without it, feedback the
|
||||
* human sends while no `await` is parked simply never reaches the agent.
|
||||
*
|
||||
* Run with: node tests/hooks/plan-canvas-pending-hook.test.js
|
||||
*/
|
||||
|
||||
const assert = require('assert');
|
||||
const fs = require('fs');
|
||||
const os = require('os');
|
||||
const path = require('path');
|
||||
|
||||
const HOOK = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'plan-canvas-pending.js');
|
||||
|
||||
async function test(name, fn) {
|
||||
try {
|
||||
await fn();
|
||||
console.log(` ✓ ${name}`);
|
||||
return true;
|
||||
} catch (err) {
|
||||
console.log(` ✗ ${name}`);
|
||||
console.log(` Error: ${err.message}`);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function freshStateDir() {
|
||||
return fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-pending-'));
|
||||
}
|
||||
|
||||
function writeState(stateDir, sessions) {
|
||||
fs.mkdirSync(stateDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(stateDir, 'sessions.json'), JSON.stringify({ sessions, feedbackCounter: 0 }, null, 2));
|
||||
}
|
||||
|
||||
function sessionRecord(key, file, pendingFeedback, overrides = {}) {
|
||||
const at = '2026-01-01T00:00:00.000Z';
|
||||
return {
|
||||
key,
|
||||
file,
|
||||
status: pendingFeedback.length ? 'feedback' : 'open',
|
||||
chat: [],
|
||||
pendingFeedback,
|
||||
createdAt: at,
|
||||
updatedAt: at,
|
||||
...overrides
|
||||
};
|
||||
}
|
||||
|
||||
function readPending(stateDir, key) {
|
||||
const state = JSON.parse(fs.readFileSync(path.join(stateDir, 'sessions.json'), 'utf8'));
|
||||
return state.sessions[key].pendingFeedback;
|
||||
}
|
||||
|
||||
// The hook resolves the state dir at call time, so the env var has to be set
|
||||
// before each invocation; a fresh require keeps the cases independent.
|
||||
function loadHook(stateDir) {
|
||||
delete require.cache[require.resolve(HOOK)];
|
||||
process.env.ECC_PLAN_CANVAS_STATE_DIR = stateDir;
|
||||
return require(HOOK);
|
||||
}
|
||||
|
||||
async function runTests() {
|
||||
console.log('\n=== Testing plan-canvas-pending Stop hook ===\n');
|
||||
let passed = 0;
|
||||
let failed = 0;
|
||||
const originalStateDir = process.env.ECC_PLAN_CANVAS_STATE_DIR;
|
||||
|
||||
if (await test('blocks the stop and hands over undelivered feedback', async () => {
|
||||
const stateDir = freshStateDir();
|
||||
const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-project-'));
|
||||
const artifact = path.join(projectDir, 'feature.plan.md');
|
||||
writeState(stateDir, {
|
||||
aaaaaaaaaaaa: sessionRecord('aaaaaaaaaaaa', artifact, [
|
||||
{ id: 'fb-1', kind: 'chat', text: 'move phase 2 up', at: '2026-01-01T00:00:00.000Z' }
|
||||
])
|
||||
});
|
||||
const hook = loadHook(stateDir);
|
||||
const result = await hook.run(JSON.stringify({ cwd: projectDir, stop_hook_active: false }));
|
||||
const decision = JSON.parse(result.stdout);
|
||||
assert.strictEqual(decision.decision, 'block');
|
||||
assert.ok(decision.reason.includes('move phase 2 up'), 'reason carries the message text');
|
||||
assert.ok(decision.reason.includes('--reply'), 'reason tells the agent to answer in the canvas');
|
||||
// Drained, so the next Stop does not block on the same message.
|
||||
assert.deepStrictEqual(readPending(stateDir, 'aaaaaaaaaaaa'), []);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('a drained queue does not block a second time', async () => {
|
||||
const stateDir = freshStateDir();
|
||||
const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-project-'));
|
||||
const artifact = path.join(projectDir, 'feature.plan.md');
|
||||
writeState(stateDir, {
|
||||
aaaaaaaaaaaa: sessionRecord('aaaaaaaaaaaa', artifact, [
|
||||
{ id: 'fb-1', kind: 'chat', text: 'first', at: '2026-01-01T00:00:00.000Z' }
|
||||
])
|
||||
});
|
||||
const hook = loadHook(stateDir);
|
||||
const input = JSON.stringify({ cwd: projectDir });
|
||||
const first = await hook.run(input);
|
||||
assert.strictEqual(JSON.parse(first.stdout).decision, 'block');
|
||||
const second = await hook.run(input);
|
||||
assert.strictEqual(second.stdout, input, 'second stop passes stdin through');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('never blocks twice in a row via stop_hook_active', async () => {
|
||||
const stateDir = freshStateDir();
|
||||
const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-project-'));
|
||||
writeState(stateDir, {
|
||||
aaaaaaaaaaaa: sessionRecord('aaaaaaaaaaaa', path.join(projectDir, 'a.plan.md'), [
|
||||
{ id: 'fb-1', kind: 'chat', text: 'hello', at: '2026-01-01T00:00:00.000Z' }
|
||||
])
|
||||
});
|
||||
const hook = loadHook(stateDir);
|
||||
const input = JSON.stringify({ cwd: projectDir, stop_hook_active: true });
|
||||
const result = await hook.run(input);
|
||||
assert.strictEqual(result.stdout, input);
|
||||
assert.strictEqual(readPending(stateDir, 'aaaaaaaaaaaa').length, 1, 'nothing drained');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('ignores sessions outside the project, unless scope=all', async () => {
|
||||
const stateDir = freshStateDir();
|
||||
const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-project-'));
|
||||
const otherDir = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-other-'));
|
||||
const state = {
|
||||
bbbbbbbbbbbb: sessionRecord('bbbbbbbbbbbb', path.join(otherDir, 'other.plan.md'), [
|
||||
{ id: 'fb-1', kind: 'chat', text: 'not yours', at: '2026-01-01T00:00:00.000Z' }
|
||||
])
|
||||
};
|
||||
writeState(stateDir, state);
|
||||
const hook = loadHook(stateDir);
|
||||
assert.strictEqual(hook.pendingSessions({ sessions: state }, projectDir, {}).length, 0);
|
||||
assert.strictEqual(
|
||||
hook.pendingSessions({ sessions: state }, projectDir, { ECC_PLAN_CANVAS_STOP_SCOPE: 'all' }).length,
|
||||
1
|
||||
);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('ended sessions and empty queues are left alone', async () => {
|
||||
const stateDir = freshStateDir();
|
||||
const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-project-'));
|
||||
const state = {
|
||||
cccccccccccc: sessionRecord(
|
||||
'cccccccccccc',
|
||||
path.join(projectDir, 'ended.plan.md'),
|
||||
[{ id: 'fb-1', kind: 'chat', text: 'stale', at: '2026-01-01T00:00:00.000Z' }],
|
||||
{ status: 'ended', endedBy: 'user' }
|
||||
),
|
||||
dddddddddddd: sessionRecord('dddddddddddd', path.join(projectDir, 'quiet.plan.md'), [])
|
||||
};
|
||||
writeState(stateDir, state);
|
||||
const hook = loadHook(stateDir);
|
||||
assert.strictEqual(hook.pendingSessions({ sessions: state }, projectDir, {}).length, 0);
|
||||
const input = JSON.stringify({ cwd: projectDir });
|
||||
assert.strictEqual((await hook.run(input)).stdout, input);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('renders annotations and verdicts readably', async () => {
|
||||
const hook = loadHook(freshStateDir());
|
||||
assert.strictEqual(
|
||||
hook.describeItem({ kind: 'annotation', text: 'split this', anchor: { snippet: 'Phase 2' } }),
|
||||
'on "Phase 2": split this'
|
||||
);
|
||||
assert.strictEqual(hook.describeItem({ kind: 'verdict', verdict: 'approve' }), 'APPROVED the plan');
|
||||
assert.strictEqual(
|
||||
hook.describeItem({ kind: 'verdict', verdict: 'request-changes', text: 'too vague' }),
|
||||
'REQUESTED CHANGES: too vague'
|
||||
);
|
||||
assert.strictEqual(hook.describeItem({ kind: 'chat', text: '' }), null);
|
||||
assert.strictEqual(hook.describeItem(null), null);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('malformed stdin and a missing state dir fail open', async () => {
|
||||
const hook = loadHook(path.join(os.tmpdir(), 'plan-canvas-does-not-exist-xyz'));
|
||||
assert.strictEqual((await hook.run('not json')).stdout, 'not json');
|
||||
assert.strictEqual((await hook.run('{}')).exitCode, 0);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (originalStateDir === undefined) delete process.env.ECC_PLAN_CANVAS_STATE_DIR;
|
||||
else process.env.ECC_PLAN_CANVAS_STATE_DIR = originalStateDir;
|
||||
|
||||
console.log('\n========================================');
|
||||
console.log(`Passed: ${passed}`);
|
||||
console.log(`Failed: ${failed}`);
|
||||
console.log('========================================\n');
|
||||
return failed === 0;
|
||||
}
|
||||
|
||||
if (require.main === module) {
|
||||
runTests().then(ok => process.exit(ok ? 0 : 1));
|
||||
}
|
||||
|
||||
module.exports = { runTests };
|
||||
@@ -253,11 +253,120 @@ async function main() {
|
||||
assert.strictEqual(result.items[0].anchor.selector, 'h2:nth-of-type(1)');
|
||||
assert.strictEqual(result.items[1].verdict, 'request-changes');
|
||||
|
||||
await waitFor(() => sse.received.some(e => e.event === 'presence' && e.data.state === 'working'));
|
||||
await waitFor(() => sse.received.some(e => e.event === 'presence' && e.data.state === 'thinking'));
|
||||
await waitFor(() => sse.received.some(e => e.event === 'chat-sync' && e.data.chat.length === 2));
|
||||
sse.close();
|
||||
})) passed++; else failed++;
|
||||
|
||||
// Regression: feedback sent with nobody parked on `await` used to leave the
|
||||
// pill claiming "agent working" while the message sat undelivered forever.
|
||||
if (await test('feedback with no listener reports queued, not working', async () => {
|
||||
const queuedArtifact = path.join(tmp, 'queued.plan.md');
|
||||
fs.writeFileSync(queuedArtifact, '# Plan: Queued\n');
|
||||
const opened = jsonBody(await request(port, 'POST', '/api/sessions', { body: { file: queuedArtifact } }));
|
||||
const sse = openSse(port, opened.key);
|
||||
await sse.ready;
|
||||
await waitFor(() => sse.received.some(e => e.event === 'presence' && e.data.state === 'waiting'));
|
||||
|
||||
const post = await request(port, 'POST', `/api/session/${opened.key}/feedback`, {
|
||||
body: { items: [{ kind: 'chat', text: 'anyone there?' }] }
|
||||
});
|
||||
assert.strictEqual(jsonBody(post).presence, 'queued');
|
||||
assert.strictEqual(canvas.presenceFor(opened.key), 'queued');
|
||||
await waitFor(() => sse.received.some(e => e.event === 'presence' && e.data.state === 'queued'));
|
||||
|
||||
// Draining it hands the batch over and flips the indicator to thinking.
|
||||
const drained = jsonBody(await request(port, 'GET', `/api/await?key=${opened.key}&timeoutMs=0`));
|
||||
assert.strictEqual(drained.status, 'feedback');
|
||||
assert.strictEqual(canvas.presenceFor(opened.key), 'thinking');
|
||||
sse.close();
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('typing endpoint drives the indicator and reply clears it', async () => {
|
||||
const typingArtifact = path.join(tmp, 'typing.plan.md');
|
||||
fs.writeFileSync(typingArtifact, '# Plan: Typing\n');
|
||||
const opened = jsonBody(await request(port, 'POST', '/api/sessions', { body: { file: typingArtifact } }));
|
||||
const sse = openSse(port, opened.key);
|
||||
await sse.ready;
|
||||
|
||||
const typing = await request(port, 'POST', `/api/session/${opened.key}/typing`, { body: { state: 'typing' } });
|
||||
assert.strictEqual(jsonBody(typing).presence, 'typing');
|
||||
await waitFor(() => sse.received.some(e => e.event === 'presence' && e.data.state === 'typing'));
|
||||
|
||||
const thinking = await request(port, 'POST', `/api/session/${opened.key}/typing`, { body: { state: 'thinking' } });
|
||||
assert.strictEqual(jsonBody(thinking).presence, 'thinking');
|
||||
|
||||
const bad = await request(port, 'POST', `/api/session/${opened.key}/typing`, { body: { state: 'dancing' } });
|
||||
assert.strictEqual(bad.statusCode, 400);
|
||||
|
||||
// A landed reply must take the bubble down, not leave it spinning.
|
||||
await request(port, 'POST', `/api/session/${opened.key}/reply`, { body: { text: 'done' } });
|
||||
assert.strictEqual(canvas.presenceFor(opened.key), 'waiting');
|
||||
await waitFor(() => sse.received.some(e => e.event === 'presence' && e.data.state === 'waiting'));
|
||||
sse.close();
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('thinking and typing states expire instead of sticking', async () => {
|
||||
const staleArtifact = path.join(tmp, 'stale.plan.md');
|
||||
fs.writeFileSync(staleArtifact, '# Plan: Stale\n');
|
||||
const staleStore = createSessionStore({ stateDir: path.join(tmp, 'stale-state') });
|
||||
const staleCanvas = createPlanCanvasServer({
|
||||
store: staleStore,
|
||||
version: '9.9.9-test',
|
||||
idleTimeoutMs: 0,
|
||||
thinkingStaleMs: 40,
|
||||
typingExpiryMs: 20,
|
||||
presenceSweepMs: 0
|
||||
});
|
||||
const bound = await staleCanvas.listen(0);
|
||||
const opened = jsonBody(await request(bound.port, 'POST', '/api/sessions', { body: { file: staleArtifact } }));
|
||||
|
||||
await request(bound.port, 'POST', `/api/session/${opened.key}/typing`, { body: { state: 'typing' } });
|
||||
assert.strictEqual(staleCanvas.presenceFor(opened.key), 'typing');
|
||||
await new Promise(resolve => setTimeout(resolve, 60));
|
||||
assert.strictEqual(staleCanvas.presenceFor(opened.key), 'waiting');
|
||||
|
||||
// An abandoned agent decays to queued so the human is never told a
|
||||
// stalled session is still being worked on.
|
||||
await request(bound.port, 'POST', `/api/session/${opened.key}/typing`, { body: { state: 'thinking' } });
|
||||
await request(bound.port, 'POST', `/api/session/${opened.key}/feedback`, {
|
||||
body: { items: [{ kind: 'chat', text: 'still there?' }] }
|
||||
});
|
||||
assert.strictEqual(staleCanvas.presenceFor(opened.key), 'thinking');
|
||||
await new Promise(resolve => setTimeout(resolve, 60));
|
||||
assert.strictEqual(staleCanvas.presenceFor(opened.key), 'queued');
|
||||
await staleCanvas.close();
|
||||
})) passed++; else failed++;
|
||||
|
||||
// The stuck pill only self-heals if the decay is pushed to an idle browser
|
||||
// that is not making any requests of its own.
|
||||
if (await test('presence sweep pushes the decayed state to an idle browser', async () => {
|
||||
const sweepArtifact = path.join(tmp, 'sweep.plan.md');
|
||||
fs.writeFileSync(sweepArtifact, '# Plan: Sweep\n');
|
||||
const sweepStore = createSessionStore({ stateDir: path.join(tmp, 'sweep-state') });
|
||||
const sweepCanvas = createPlanCanvasServer({
|
||||
store: sweepStore,
|
||||
version: '9.9.9-test',
|
||||
idleTimeoutMs: 0,
|
||||
thinkingStaleMs: 50,
|
||||
presenceSweepMs: 20
|
||||
});
|
||||
const bound = await sweepCanvas.listen(0);
|
||||
const opened = jsonBody(await request(bound.port, 'POST', '/api/sessions', { body: { file: sweepArtifact } }));
|
||||
const sse = openSse(bound.port, opened.key);
|
||||
await sse.ready;
|
||||
|
||||
await request(bound.port, 'POST', `/api/session/${opened.key}/typing`, { body: { state: 'thinking' } });
|
||||
await waitFor(() => sse.received.some(e => e.event === 'presence' && e.data.state === 'thinking'));
|
||||
|
||||
const before = sse.received.length;
|
||||
await waitFor(() =>
|
||||
sse.received.slice(before).some(e => e.event === 'presence' && e.data.state === 'waiting')
|
||||
);
|
||||
sse.close();
|
||||
await sweepCanvas.close();
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('long-poll heartbeat whitespace arrives before the payload', async () => {
|
||||
const chunks = [];
|
||||
const done = new Promise((resolve, reject) => {
|
||||
|
||||
Reference in New Issue
Block a user