Skip to content
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
# ZCode 0.16.5 Permission Turn Lifecycle Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Preserve the active protocol turn across an admission-time legacy completion wake so later ZCode permission requests succeed, then release the local turn at the executor's authoritative cleanup boundary.

**Architecture:** Add a non-destructive completion observer alongside the existing destructive waiter in the protocol/client layer. Migrate only `executeJob` to that observer and explicitly release its local turn during unconditional teardown; retain compatibility fallbacks for injected test clients that expose only the historical interface.

**Tech Stack:** Node.js 22.13, ECMAScript modules, `node:test`, the existing JSON-RPC protocol client and job executor.

---

## File map

- Modify `scripts/lib/zcode-protocol.mjs`: share completion validation/wait registration while distinguishing destructive consumption from observation; add local turn release behavior.
- Modify `scripts/lib/zcode-client.mjs`: expose `observeCompletion()` and `releaseTurn()` without changing `waitForCompletion()`.
- Modify `scripts/lib/review.mjs`: use non-destructive observation for `legacyWake` and release the local turn during teardown.
- Modify `tests/process-zcode.test.mjs`: cover low-level observer, permission, timeout, release, and destructive-wait invariants.
- Modify `tests/job-control.test.mjs`: cover captured 0.16.5 executor ordering and success/error cleanup.

### Task 1: Add non-destructive completion observation

**Files:**
- Modify: `scripts/lib/zcode-protocol.mjs:95-145`
- Modify: `scripts/lib/zcode-client.mjs:125-135`
- Test: `tests/process-zcode.test.mjs`

- [ ] **Step 1: Write failing protocol tests**

Add tests that construct `ZCodeProtocolClient` with `PassThrough` streams, arm a turn, start `observeCompletion(sessionId)`, emit a matching `prompt_completed`, and assert:

```js
const completion = protocol.observeCompletion(sessionId);
protocol.handleLine(JSON.stringify({ method: 'state.updated', params: matchingCompletion }));
assert.equal((await completion).reason, 'prompt_completed');
assert.equal(protocol.turnState(sessionId), 'armed');
```

Then emit `interaction/requestPermission` for the same session, return an offered allow response from the handler, and verify the protocol writes the allow result instead of `ZCODE_PERMISSION_SESSION_INVALID`. Add companion assertions that `waitForCompletion()` still clears the turn, observer timeout leaves it armed, and `releaseTurn()` clears it and rejects any still-pending observer.

- [ ] **Step 2: Run the focused tests and verify RED**

Run:

```bash
node --test --test-name-pattern='non-destructive completion|completion observer|destructive completion' tests/process-zcode.test.mjs
```

Expected: FAIL because `observeCompletion` and `releaseTurn` do not exist.

- [ ] **Step 3: Implement the minimal protocol behavior**

Refactor the current waiter registration into one internal path with an explicit consumption mode. Preserve the public destructive path exactly, and add:

```js
observeCompletion(sessionId, timeoutMs) {
return this.waitForCompletionMode(sessionId, timeoutMs, false);
}

releaseTurn(sessionId) {
if (!nonEmpty(sessionId)) throw protocolInputError();
this.cancelTurn(sessionId, localTurnReleasedError(sessionId));
}
```

For observation mode, peek at an already queued completion rather than shifting it, do not call `abortTurn()` on resolution or timeout, and always unregister the observer. For destructive mode, keep the current shift, timeout cleanup, and `abortTurn()` behavior. Continue enforcing one waiter/observer per session with `waiterSessions`.

Expose the two operations from `ZCodeClient` with normal session-ID validation/documentation:

```js
observeCompletion(sessionId, timeoutMs) {
requireSessionId(sessionId);
return this.protocol.observeCompletion(sessionId, timeoutMs);
}

releaseTurn(sessionId) {
requireSessionId(sessionId);
this.protocol.releaseTurn(sessionId);
}
```

- [ ] **Step 4: Run focused and adjacent tests and verify GREEN**

Run:

```bash
node --test tests/process-zcode.test.mjs tests/zcode-client.test.mjs
```

Expected: PASS, including all existing destructive completion and permission tests.

- [ ] **Step 5: Self-review and commit**

Check that no existing call site changed and that observer cleanup cannot retain a timer, subscriber, or waiter-session entry. Then commit:

```bash
git add scripts/lib/zcode-protocol.mjs scripts/lib/zcode-client.mjs tests/process-zcode.test.mjs
git commit -m "fix: observe legacy completion without ending turn"
```

### Task 2: Migrate executor wake and own local cleanup

**Files:**
- Modify: `scripts/lib/review.mjs:220-360`
- Test: `tests/job-control.test.mjs`

- [ ] **Step 1: Write the captured-ordering regression test**

Extend the existing `0.16.5 foreground execution treats legacy completion as admission` fixture client with `observeCompletion`, `releaseTurn`, and a permission handler capture. Make `observeCompletion` publish the legacy wake first, then invoke the captured permission handler with a medium-risk Write request offering allow/deny. Assert the handler returns allow while the executor remains running, then publish the v4 authoritative terminal and assert success plus one local release:

```js
assert.deepEqual(permissionDecision, { decision: 'allow' });
assert.equal(releaseTurnCalls, 1);
assert.equal(releasedSessionId, sessionId);
```

Add an error-path test where observation or authoritative reconciliation fails after admission; assert `releaseTurn(sessionId)` still runs once before `close()`.

- [ ] **Step 2: Run the executor regressions and verify RED**

Run:

```bash
node --test --test-name-pattern='0.16.5 foreground execution|releases local turn' tests/job-control.test.mjs
```

Expected: FAIL because `executeJob` still calls destructive `waitForCompletion()` and never releases the local turn explicitly.

- [ ] **Step 3: Implement the executor migration**

Construct `legacyWake` from `client.observeCompletion(activeSessionId)` when available. Keep a fallback to `client.waitForCompletion(activeSessionId)` only for existing injected test doubles that predate the internal interface:

```js
const observeLegacyCompletion = typeof client.observeCompletion === 'function'
? client.observeCompletion.bind(client)
: client.waitForCompletion.bind(client);
const legacyWake = waitForCompletionOrAbort(observeLegacyCompletion(activeSessionId), input.signal);
```

In unconditional teardown, after all terminal/cancellation/error reconciliation and progress cleanup but before `client.close()`, release the known local session exactly once when supported:

```js
try {
if (sessionId && typeof client.releaseTurn === 'function') client.releaseTurn(sessionId);
} catch (cleanupError) {
if (!primaryError) primaryError = cleanupError;
}
```

Preserve the primary-error and cleanup-error conventions already used by the executor. Do not send an upstream stop from this release path and do not modify `decidePermission`.

- [ ] **Step 4: Run focused and executor-adjacent tests and verify GREEN**

Run:

```bash
node --test tests/job-control.test.mjs tests/integration/companion.test.mjs
```

Expected: PASS with the captured ordering, cleanup tests, and existing cancellation behavior unchanged.

- [ ] **Step 5: Self-review and commit**

Inspect the diff for exactly one production caller migration, one unconditional local cleanup, and no permission-policy changes. Then commit:

```bash
git add scripts/lib/review.mjs tests/job-control.test.mjs
git commit -m "fix: retain active turn through legacy wake"
```

### Task 3: Verify contracts and release readiness

**Files:**
- Modify if required by generated parity checks: checked-in marketplace mirrors only through the repository's existing builder
- Test: repository-wide verification

- [ ] **Step 1: Run permission-policy and protocol contract tests**

Run:

```bash
node --test --test-name-pattern='permission|completion' tests/process-zcode.test.mjs tests/zcode-client.test.mjs tests/job-control.test.mjs
```

Expected: PASS; Rescue permission decisions remain unchanged and destructive completion callers retain their contract.

- [ ] **Step 2: Run the full repository check**

Run:

```bash
npm run check
```

Expected: PASS for line endings, lint, typecheck, all tests, qualification tests, and marketplace parity/build checks.

- [ ] **Step 3: Inspect final scope**

Run:

```bash
git diff --check origin/main...HEAD
git diff --stat origin/main...HEAD
git diff origin/main...HEAD -- scripts/lib/review.mjs scripts/lib/zcode-client.mjs scripts/lib/zcode-protocol.mjs
```

Expected: no whitespace errors; changes remain limited to the completion lifecycle, tests, and approved docs.

- [ ] **Step 4: Commit any verification-only generated parity update**

If and only if the repository's official check regenerates tracked marketplace parity files, review and commit those exact generated changes:

```bash
git add marketplace
git commit -m "build: refresh marketplace snapshot"
```

If there are no generated tracked changes, skip this commit.
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# ZCode 0.16.5 Permission Turn Lifecycle Design

## Context

The executor already treats ZCode 0.16.5 `state.updated` notifications with reason `prompt_completed` as legacy liveness signals. True completion is established by the v4 conversation observer or a coherent current-turn snapshot. However, `executeJob` obtains the liveness signal through `client.waitForCompletion()`, whose public contract consumes completion and deletes the protocol client's active-turn state.

ZCode 0.16.5 can emit that legacy notification immediately after admission, then request tool permission while the real runtime turn is still active. Once the legacy waiter deletes the turn, the permission request fails the exact-active-turn check and is returned as JSON-RPC `-32000` (`ZCODE_PERMISSION_SESSION_INVALID`). This is independent of the configured Codex permission mode: the incident job already carried `bypassPermissions`.

## Goals

- Keep the exact turn armed after an admission-time legacy completion wake so later permission requests can be evaluated normally.
- Preserve the current destructive behavior of `waitForCompletion()` for every existing caller.
- Clear local turn state when `executeJob` reaches its real terminal or cleanup boundary.
- Cover the captured 0.16.5 event ordering with deterministic regression tests.

## Non-goals

- No change to `decidePermission`, permission snapshots, risk mapping, or offered-response validation.
- No new appserver flag, permission field, broker authorization rule, or persisted schema.
- No reinterpretation of legacy completion as authoritative success or failure.
- No broad refactor of the protocol or executor lifecycle.

## Design

### Non-destructive legacy observation

Add a narrowly named protocol/client operation for observing the next validated completion notification without consuming the active turn. It must apply the same session, timeout, active-turn, duplicate-waiter, and `isCompletionFor` validation used by `waitForCompletion()`, but resolution must not call `abortTurn()` and must not consume turn ownership.

An observer timeout removes only that observer; it does not silently acquire authority to end the turn. Executor teardown remains responsible for local release. Add an idempotent client-level local release operation that cancels outstanding local completion observation and clears the protocol turn without sending an upstream stop request.

The existing `waitForCompletion()` remains unchanged in observable behavior: queued or live completion resolution consumes the turn, while timeout and cancellation retain their present destructive cleanup semantics.

Only `executeJob` switches its `legacyWake` construction to the non-destructive observer. The coordinator continues to use the wake solely to trigger authoritative v4/snapshot reconciliation.

### Explicit executor cleanup

Because the wake no longer consumes local state, `executeJob` must explicitly release the protocol turn after the authoritative lifecycle has finished. Cleanup belongs in the executor's existing unconditional teardown, after any terminal/cancellation reconciliation that may still need permission and turn identity, and before client close completes.

The cleanup operation is local and idempotent. It must not send `session/stop`, alter durable job state, or replace the existing cancellation paths. Successful terminal, provider failure, remote interruption, local abort, timeout, and error cleanup all converge on the same local turn release when a session was created or resumed.

### Safety invariants

- A validated early legacy completion leaves `turnState(sessionId) === 'armed'`.
- A later permission request for that session reaches the configured handler and returns one offered response.
- Authoritative executor teardown leaves `turnState(sessionId) === null`.
- Ordinary `waitForCompletion()` still leaves `turnState(sessionId) === null` immediately after resolution.
- Permission policy and durable job outcomes are unchanged.

## Testing

1. Add a protocol/client regression proving non-destructive observation preserves the armed turn and permits a subsequent permission request.
2. Retain or strengthen the destructive waiter assertion so its compatibility contract is explicit.
3. Add an executor regression with captured 0.16.5 ordering: admission, early legacy wake, later permission request, authoritative terminal, successful result, and final local turn cleanup.
4. Exercise cleanup on a non-success path so an observer cannot leave an armed turn behind.
5. Run focused tests, then `npm run check` before review and PR creation.

## Rollout and compatibility

This is an internal additive API and a one-call-site migration. No user configuration or data migration is required. Older ZCode versions continue to produce the same wake signal, and all callers outside `executeJob` retain existing semantics.
18 changes: 9 additions & 9 deletions marketplace/.agents/plugins/provenance.json
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"packageVersion": "0.1.0",
"pluginVersion": "0.1.0",
"sourceRef": "e7c986bcc8e743f6ba2ae35004cb51ad00a995ac",
"sourceSha": "e7c986bcc8e743f6ba2ae35004cb51ad00a995ac",
"sourceRef": "ee1fa5c9778cf6dd02e094cf2bfe7f2835c02dab",
"sourceSha": "ee1fa5c9778cf6dd02e094cf2bfe7f2835c02dab",
"dependencyLock": {
"file": "npm-shrinkwrap.json",
"sha256": "fa927194e6ca0b25c1d3f428859b2ab4798b8eabb4d31bc87188d73c630f9938"
},
"content": {
"algorithm": "sha256",
"sha256": "02b5640cd5cf5ad4c1dbae7dd9e5028710d8ac3f12077ea08ce2f670c4dd2543",
"sha256": "40d1a65be6976f5f070b2784a35635f5aba4597c4846ec798b86d3f30dd77af7",
"files": [
{
"path": ".agents/plugins/marketplace.json",
Expand Down Expand Up @@ -658,8 +658,8 @@
},
{
"path": "plugins/zcode/scripts/lib/review.mjs",
"size": 43083,
"sha256": "5495fe99558e3d9595f9c016bdef9ed9aca4940b7f89438e544c5fd280c0f25f"
"size": 43572,
"sha256": "0d945d223ce618ee28e2e81831ce855df1b18cc975f45dcc5538b8cc6e934d11"
},
{
"path": "plugins/zcode/scripts/lib/session-progress.mjs",
Expand Down Expand Up @@ -703,8 +703,8 @@
},
{
"path": "plugins/zcode/scripts/lib/zcode-client.mjs",
"size": 52052,
"sha256": "2f38c2c6f0b1abbe094fbc7e67f359e6256ebfc0f05a602138f3c3a1e3559d77"
"size": 52495,
"sha256": "0d4c854d4045260cc86d14d30e647db22efb08bd4f9ad30a2f795c3b5e37f901"
},
{
"path": "plugins/zcode/scripts/lib/zcode-discovery.mjs",
Expand All @@ -713,8 +713,8 @@
},
{
"path": "plugins/zcode/scripts/lib/zcode-protocol.mjs",
"size": 38990,
"sha256": "a6531d8d112b7c605a26d8eee8c0af5f3e01f8e14ef34bfe3226df60a31f9db7"
"size": 42018,
"sha256": "68e98ef8be8b670648e3add7966fb1619b3836be0ce6b2a29e678899f68f9426"
},
{
"path": "plugins/zcode/scripts/lib/zcode-runtime-config.mjs",
Expand Down
15 changes: 13 additions & 2 deletions marketplace/plugins/zcode/scripts/lib/review.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,10 @@ export async function executeJob(input) {
reporter.activateAcceptedBoundary({ readSnapshot: () => client.readSession(activeSessionId), describer: sessionDescriber });
} catch { reporter.activateAcceptedBoundary({}); }
reporter.activate({ method: 'state.updated', params: { scope: 'session', sessionId: activeSessionId, reason: 'prompt_started' } });
const legacyWake = waitForCompletionOrAbort(client.waitForCompletion(activeSessionId), input.signal);
const observeLegacyCompletion = typeof client.observeCompletion === 'function'
? client.observeCompletion.bind(client)
: client.waitForCompletion.bind(client);
const legacyWake = waitForCompletionOrAbort(observeLegacyCompletion(activeSessionId), input.signal);
const terminal = await awaitCurrentTurnTerminal({
legacyWake, conversationObserver, readSnapshot: () => client.readSession(activeSessionId), turnBoundary, signal: input.signal,
});
Expand All @@ -252,7 +255,7 @@ export async function executeJob(input) {
}
} catch (error) {
primaryError = error instanceof SuccessfulResultFinalizationError ? error.cause : error;
let current = initialBoundStopGuardComplete ? await input.store.readJob(workspace, job.id).catch(() => running) : null;
try { let current = initialBoundStopGuardComplete ? await input.store.readJob(workspace, job.id).catch(() => running) : null;
let resumeFailureSettlementRejected = false;
if (input.resumeSessionId && current?.status === 'queued' && input.onResumeFailure) {
try {
Expand Down Expand Up @@ -333,9 +336,17 @@ export async function executeJob(input) {
}
if (canFail) try { await input.store.finishJob(workspace, job.id, [current.status], 'failed', { error: safeError(error), exitCode: 1 }); } catch (finalizeError) { primaryError = finalizeError; }
}
} catch (reconciliationError) {
primaryError = reconciliationError;
}
}
// Cleanup order is part of the progress lifecycle contract.
await cleanupProgress();
try {
if (sessionId && typeof client.releaseTurn === 'function') client.releaseTurn(sessionId);
} catch (cleanupError) {
if (!primaryError && output?.job?.status !== 'succeeded') primaryError = cleanupError;
}
await client.close().catch(() => {});
if (!primaryError && appliedFinalization && output?.job?.status === 'succeeded' && typeof output.result === 'string') {
await jobLog?.appendBlock('Final output', output.result, Date.now() + OPTIONAL_PROGRESS_FENCE_MS);
Expand Down
2 changes: 2 additions & 0 deletions marketplace/plugins/zcode/scripts/lib/zcode-client.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ export class ZCodeClient {
}

/** Wait for a validated terminal notification; no deadline applies unless configured on the client or supplied here. @param {string} sessionId @param {number} [timeoutMs] */ waitForCompletion(sessionId, timeoutMs) { return this.protocol.waitForCompletion(sessionId, timeoutMs); }
/** Observe a validated terminal notification without consuming the active turn. @param {string} sessionId @param {number} [timeoutMs] */ observeCompletion(sessionId, timeoutMs) { return this.protocol.observeCompletion(sessionId, timeoutMs); }
/** Locally release an active turn without sending an upstream request. @param {string} sessionId */ releaseTurn(sessionId) { requireSessionId(sessionId); this.protocol.releaseTurn(sessionId); }
/** Exact local protocol invariant used to prove whether this client owns an active turn. @param {string} sessionId */ turnState(sessionId) { requireSessionId(sessionId); return this.protocol.turnState(sessionId); }
/** @param {string} sessionId @param {{connectionId:string,clientMode:'desktop-continuous'|'web-remote-replayable'}} options */
async subscribeConversation(sessionId, options) {
Expand Down
Loading
Loading