Skip to content

fix(streams): name the closing and refusal states in producer write reasons - #1528

Merged
rickylabs merged 1 commit into
mainfrom
fix/1405-durable-producer-rejection-taxonomy
Aug 12, 2026
Merged

fix(streams): name the closing and refusal states in producer write reasons#1528
rickylabs merged 1 commit into
mainfrom
fix/1405-durable-producer-rejection-taxonomy

Conversation

@rickylabs

@rickylabs rickylabs commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Summary

Name the two durable-producer states truthfully without changing acceptance, retry, delivery, cancellation, or telemetry classification: graceful close-drain rejection now reports producer-stopping, and non-retryable transport failure reports the new published transport-refused reason.

Scope

Slices

  • S1 durable producer rejection taxonomy and deterministic negative tests — c491c6989

Validation

  • Scoped check — exit 0; 43 files, 0 failed batches.
  • Scoped lint — exit 0; 43 files, 0 occurrences.
  • Scoped format — exit 0; 43 files, 0 findings.
  • deno task quality:gate — exit 0; repository-configured quality/doctrine gates completed.
  • Explicit target quality scan — exit 0; findings=[], allowCount=0.
  • deno task doc:lint --root packages/plugin-streams-core --pretty — exit 0; 4 entrypoints, 0 errors.
  • Package task — exit 0; 33 passed, 0 failed.
  • Exact brief command deno test packages/plugin-streams-core — exit 1; 14 passed and 19 permission-only NotCapable failures because the command omits --allow-env. Preserved in the worklog; the checked-in package task grants its declared permissions and is green.
  • No e2e:cli run, as explicitly excluded by the slice brief.

Harness

  • Run dir: .llm/runs/release-0.0.6-features--orchestration/slices/1405/
  • Phase: impl; separate orchestrator-owned slice review and IMPL-EVAL remain pending.
  • PLAN-EVAL: N/A per the fully specified owner brief and orchestration record.
  • Do not merge or mark ready until the orchestrator completes review and evaluation.

Drift / Debt

  • No architecture debt accepted.
  • Recorded gate-invocation drift: the bare test command lacks env permission; root quality configuration omits this package, so an explicit target scan was added.
issue: 1405
entries:
  - box-index: 1
    evidence: "close drain negative test holds append/flush open and asserts producer-stopping, not producer-failed"
  - box-index: 2
    evidence: "first append refusal test asserts transport-refused, not retry-exhausted, and exactly one append call"
  - box-index: 3
    evidence: "retryable append failures test reaches maxAttempts and asserts retry-exhausted, not transport-refused"
  - box-index: 4
    evidence: "four explicit negative assertions prevent reason collapse; reconnect refusal separately covers the connect guard"
  - box-index: 5
    evidence: "existing telemetry classification guard passes 2/2; full configured package suite passes 33/33"

Definition of Done

  • Only the locked reason strings and observability are changed.
  • Each required negative path has deterministic regression coverage.
  • Scoped, package, documentation, quality, and JSR evidence is recorded truthfully.
  • Separate-session IMPL-EVAL passes.
  • Orchestrator pre-merge review completes.

@rickylabs rickylabs added area:plugins plugins/* and plugin-core packages status:impl type:fix labels Aug 12, 2026 — with ChatGPT Codex Connector
@rickylabs rickylabs added this to the 0.0.6 milestone Aug 12, 2026 — with ChatGPT Codex Connector

Copy link
Copy Markdown
Owner Author

[PHASE: IMPL]

Landed the locked #1405 reason-only slice at c491c6989628b1e7f9bca88f1c266a19b5be0af9.

Scope

Gate output

scoped check exit 0
{"selection":{"filesSelected":43,"batches":1,"failedBatches":0},"summary":{"totalOccurrences":0}}

scoped lint exit 0
{"selection":{"filesSelected":43,"batches":1},"summary":{"totalOccurrences":0}}

scoped format exit 0
{"summary":{"filesSelected":43,"batches":1,"failedBatches":0,"findings":0,"ignoredFindings":0}}

quality:gate exit 0
Task quality:gate deno task quality:scan && deno task arch:check
{"ok":true,"mode":"repository","scanned":["packages/cli/src","plugins"],"findings":[],"allowCount":7}

explicit target quality scan exit 0
{"ok":true,"mode":"repository","scanned":["packages/plugin-streams-core/src"],"findings":[],"allowCount":0}

doc:lint exit 0
"entrypoints": ["./mod.ts","./src/sse/mod.ts","./src/telemetry/mod.ts","./src/testing/mod.ts"]
"totalErrors": 0
"totalMissingJSDoc": 0

focused contract tests exit 0
ok | 8 passed | 0 failed

telemetry behavior guard exit 0
ok | 2 passed | 0 failed

package-configured full suite exit 0
ok | 33 passed | 0 failed

brief's exact bare test command exit 1
FAILED | 14 passed | 19 failed
NotCapable: Requires env access to "DURABLE_STREAMS_URL", run again with the --allow-env flag

The exact bare command's red is permission-only and is recorded in worklog.md/drift.md; the checked-in package task (deno test --allow-all tests/) is green 33/33. The configured root quality gate omits this package, so the explicit target scan was run and is clean.

Next

  • Orchestrator-owned substantive slice review.
  • Separate-session IMPL-EVAL.
  • Keep draft; do not mark ready or merge.

@rickylabs

Copy link
Copy Markdown
Owner Author

[PHASE: REVIEW]

Tier-A slice review by the orchestrator (review_codex_light, Opus 5 high paired to the Sol·low implementation lane). Claims re-verified independently in the worktree rather than read off the slice report.

Verified independently

  • Package suite — ran deno task --cwd packages/plugin-streams-core test myself: 33 passed, 0 failed. Matches the slice's claim.
  • Negative case demonstrated (the check that matters). The gate-integrity rule in milestone-run.md is that a guard enters only with its negative case proven — 0.0.4 shipped two guards whose predicate could never fire. So I reverted both fixes in durable-stream-producer-supervisor.ts (refusal branch back to the conflated retry-exhausted, selector back to default: producer-failed) and re-ran: 29 passed, 5 failed. Restored, re-ran: 33/33, clean tree. These tests genuinely fire.
  • Diff matches the locked decisions. transport-refused is the only public-surface addition (producer-contract-v1.ts:84-89). Close-drain reuses the existing producer-stopping — no producer-closing member. #failActive now branches on isRetryable (:427-435), and the conflated guard at the connect call site is split into two, so a non-retryable connect failure on the final attempt reports refusal rather than exhaustion.
  • The duplicate-selector hazard is closed properly. stateRejection() was deleted from the façade and both write entry points now delegate to supervisor.writeRejectionReason(). That was the preferred shape in the brief: the two selectors can no longer drift apart, which is how this defect existed in the first place.

Findings

  1. A1 (advisory, not blocking) — reverted-state failure is a cascade, not four clean diagnostics. With the fix reverted, the first test throws an uncaught error and the other three report cancelled rather than failing individually. Acceptance box 4 is still satisfied — a collapse of the reasons does turn the suite red — but a future maintainer sees one uncaught error instead of four precise assertion failures. Not worth churning the slice for; recorded so the next reader is not surprised.
  2. A2 (advisory) — doctrine WARN carried forward. The slice reports arch:check at FAIL=0 WARN=1: the supervisor file is 515 lines against a 500-line advisory cap, which this change pushed past. Not a merge blocker (advisory, zero FAILs) and not worth an unrelated refactor inside a reasons-only slice.

Defect in my own brief, correctly reported

The brief's gate list named deno test packages/plugin-streams-core, which exits 1 with 19 NotCapable permission failures because it omits --allow-env. The slice ran it, reported the red with its cause, and used the package-declared task instead. That is the behaviour the brief asked for — a red reported is useful, a red sat on is the failure mode. The brief was wrong, not the slice. Corrected for the #1398 brief.

Verdict

Slice review passes. No blocking findings. This is not a sign-off: separate-session IMPL-EVAL and the pre-merge gate are still required, and merge authority stays with the orchestrator.

Next

  • IMPL-EVAL dispatched to a separate session on the small-impl evaluator lane.
  • Orchestrator runs the milestone-run.md pre-merge gate before merge. Do not flip to ready.

@rickylabs

Copy link
Copy Markdown
Owner Author

[PHASE: IMPL-EVAL] [VERDICT: PASS]

Separate-session IMPL-EVAL on the small-impl evaluator lane (DeepSeek V4 Flash 0731 max, fresh session in its own worktree at c491c6989, 642,836 ms, 5,226 stream events, is_error: False). Verbatim verdict: .llm/runs/release-0.0.6-features--orchestration/slices/evaluate-1405.md.

The check that makes acceptance box 4 real

My own review had reverted both fixes together and observed 5 failures — that proves the tests fire, but not that each guards its own defect. The evaluator was asked to revert them one at a time, and did:

  • Revert only the #closing / #writeRejectionReason change → only test 1 (close-drain) fails; tests 2, 3, 4 pass.
  • Revert only the #failActive reason selection → only tests 2 and 4 fail; test 1 and the retry-exhaustion guard pass.
  • Tree restored, git status porcelain empty.

That is the difference between "the suite goes red" and "each reason is independently pinned". Acceptance box 4 asks that a future refactor collapsing the reasons fails — this demonstrates it per reason, not in aggregate.

It also independently confirmed behaviour preservation: the only functional deltas are reason strings plus a behaviour-neutral split of the connect guard, with no change to any settlement status, admission decision, or retry count.

Findings — three advisory, none blocking

  1. The ?? 'producer-failed' fallback at create-durable-stream.ts:132,160 is unreachable. Correct: writeRejectionReason() returns undefined only when #accepted === true, and both call sites sit behind if (this.closed). I am keeping it, and the evaluator's suggested cleanup does not typecheck. The method returns StreamWriteRejectionReasonV1 | undefined, so dropping the ?? requires either a non-null assertion — which this slice's brief forbids — or widening the refactor. Keeping a total, type-safe expression is the better trade than either. Recorded rather than actioned.
  2. quality:gate's configured roots omit packages/plugin-streams-core. Confirmed by the evaluator against deno.json:156, matching what the slice reported. Compensating target-scoped scans were run and are clean (findings=[], allowCount=0). This is a gate-coverage gap in the repo, not a defect in this change — the package-quality verdict here rests on the explicit target scan, and saying so is the honest version.
  3. research-1405.md appeared absent to the evaluator. That is an artifact of my worktree layout, not a missing file: the evaluator worktree is detached at c491c6989, which is based on origin/main, while the run-dir research lives on the orchestration control branch (chore(harness): orchestrate the 0.0.6 runtime and public-surface lane #1525). The file exists; it was simply not reachable from where the evaluator stood. My setup, my correction.

Status

IMPL-EVAL PASS. Merge is not yet clear: every CI check on this PR currently reports skipping because it is a draft, and the pre-merge gate treats "nothing ran" as unproven rather than clean. Flipping to ready and getting real SUCCESS on the named gates plus a green close-gate is the remaining work before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:plugins plugins/* and plugin-core packages canary:0.0.6-canary.2 Published NetScript prerelease 0.0.6-canary.2 status:shipped type:fix

Projects

None yet

1 participant