Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions docs/agents/runs/cleanup-1-9-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
- Feature branches: one branch per approved ticket, created from the latest integrated `staging`
- Human owner: plebdev
- Started: 2026-07-18
- Current status: items 1–7 / issues #131–#137 merged into `staging`; item 8 / issue #138 is locally complete and awaiting its PR on `feature/public-behavior-test-seams`
- Current status: items 1–8 / issues #131–#138 merged into `staging`; item 9 / issue #139 is open as PR #148 into `staging`; one hosted Node 20 rerun pending after an isolated loaded-suite timeout
- Skill setup status: present and verified (`AGENTS.md`, GitHub issue tracker, triage labels, domain docs, ADRs, CI, CodeRabbit)

## Goal
Expand All @@ -27,7 +27,7 @@ Complete cleanup items 1–9 from the staging audit end to end, branch by branch
- Agent briefs: Grok 4.5 is the exclusive delegated sidecar; Cursor exposes the highest available tier as `cursor-grok-4.5-high`, which is used for all standards/spec passes
- Review packets: `issue-131-review-packet.md` through `issue-137-review-packet.md`; created per later ticket
- Local CodeRabbit report: `issue-131-coderabbit-local.md` through `issue-136-coderabbit-local.md`; created per later ticket
- PR URL: #140 merged for issue #131; #141 merged for issue #132; #142 merged for issue #133; #143 merged for issue #134; #144 merged for issue #135; #145 merged for issue #136; #146 merged for issue #137; issue #138 PR pending; always non-draft and targeting `staging`
- PR URL: #140 merged for issue #131; #141 merged for issue #132; #142 merged for issue #133; #143 merged for issue #134; #144 merged for issue #135; #145 merged for issue #136; #146 merged for issue #137; #147 merged for issue #138; #148 open for issue #139; always non-draft and targeting `staging`

## Commands

Expand All @@ -39,17 +39,17 @@ Complete cleanup items 1–9 from the staging audit end to end, branch by branch

## Ticket Ledger

| Issue | Type | Status | Branch | Review | Verified |
| --------------------------------- | ---- | --------------- | --------------------------------------- | -------------------------------------------------------- | ------------------------------------- |
| #131 NIP-46 diagnostic redaction | AFK | merged | `feature/nip46-diagnostic-redaction` | Grok approved; CodeRabbit local/hosted clean after fixes | Jest/Bun 1054/1054; hosted CI green |
| #132 published declaration purity | AFK | merged | `feature/public-type-test-purity` | Grok pass; CodeRabbit local/hosted clean | Jest/Bun 1055/1055; hosted CI green |
| #133 shared diagnostic seam | AFK | merged | `feature/shared-diagnostics-completion` | Grok standards/spec pass; local and hosted clean | Jest/Bun 1067/1067; hosted CI green |
| #134 NIP-47 service lifecycle | AFK | merged | `feature/nip47-service-lifecycle` | Grok pass; CodeRabbit local/hosted clean | Jest/Bun 1073/1073; hosted CI green |
| #135 NIP-57 consolidation | AFK | merged | `feature/nip57-client-consolidation` | Grok pass; CodeRabbit local/hosted clean after fixes | Jest/Bun 1082/1082; hosted CI green |
| #136 NIP-46 protocol core | AFK | merged | `feature/nip46-protocol-core` | Grok and CodeRabbit local/hosted clean after fixes | Jest/Bun 1096/1096; hosted CI green |
| #137 default test feedback loop | AFK | merged | `feature/fast-default-test-loop` | Grok pass; CodeRabbit hosted finding fixed and confirmed | routine 1063; slow 40; coverage 1103 |
| #138 public behavior test seams | AFK | PR pending | `feature/public-behavior-test-seams` | Grok approved; CodeRabbit local clean after fixes | Jest/Bun 1061; slow 35; coverage 1096 |
| #139 ephemeral Relay internals | AFK | blocked by #138 | `feature/ephemeral-relay-internals` | pending | pending |
| Issue | Type | Status | Branch | Review | Verified |
| --------------------------------- | ---- | ----------- | --------------------------------------- | -------------------------------------------------------- | ------------------------------------ |
| #131 NIP-46 diagnostic redaction | AFK | merged | `feature/nip46-diagnostic-redaction` | Grok approved; CodeRabbit local/hosted clean after fixes | Jest/Bun 1054/1054; hosted CI green |
| #132 published declaration purity | AFK | merged | `feature/public-type-test-purity` | Grok pass; CodeRabbit local/hosted clean | Jest/Bun 1055/1055; hosted CI green |
| #133 shared diagnostic seam | AFK | merged | `feature/shared-diagnostics-completion` | Grok standards/spec pass; local and hosted clean | Jest/Bun 1067/1067; hosted CI green |
| #134 NIP-47 service lifecycle | AFK | merged | `feature/nip47-service-lifecycle` | Grok pass; CodeRabbit local/hosted clean | Jest/Bun 1073/1073; hosted CI green |
| #135 NIP-57 consolidation | AFK | merged | `feature/nip57-client-consolidation` | Grok pass; CodeRabbit local/hosted clean after fixes | Jest/Bun 1082/1082; hosted CI green |
| #136 NIP-46 protocol core | AFK | merged | `feature/nip46-protocol-core` | Grok and CodeRabbit local/hosted clean after fixes | Jest/Bun 1096/1096; hosted CI green |
| #137 default test feedback loop | AFK | merged | `feature/fast-default-test-loop` | Grok pass; CodeRabbit hosted finding fixed and confirmed | routine 1063; slow 40; coverage 1103 |
| #138 public behavior test seams | AFK | merged | `feature/public-behavior-test-seams` | Grok approved; CodeRabbit local/hosted findings resolved | Jest/Bun 1061; hosted CI green |
| #139 ephemeral Relay internals | AFK | PR #148 open | `feature/ephemeral-relay-internals` | Grok standards/spec approved; CodeRabbit findings resolved/triaged | Jest/Bun 1,109; local gates green; hosted rerun pending |

## Parked HITL Slices

Expand All @@ -68,7 +68,8 @@ Complete cleanup items 1–9 from the staging audit end to end, branch by branch
| #135 | `25e055d` | current Codex orchestrator; Grok 4.5 High reviewers | `0909227`, `1b12872`, `b66d483` | Grok passes; CodeRabbit local/hosted clean after fixes | focused 23/23; NIP-57 41/41; Jest/Bun 1082/1082; all local gates and four hosted lanes green |
| #136 | `ed9fa4a` | current Codex orchestrator; Grok 4.5 High reviewers | PR #145 through merge `8b970e4` | Grok and CodeRabbit local/hosted clean after fixes | NIP-46 185/185; Jest/Bun 1096/1096; all local and hosted gates green |
| #137 | `8b970e4` | current Codex orchestrator; Grok 4.5 High reviewers | PR #146 through merge `b33f31f` | Grok standards/spec passed after Bun 1.3.9 fix; CodeRabbit hosted finding fixed and explicitly confirmed | baseline 85/1096/58.793s; routine 84/1063/32.356s; slow 2/40/43.154s; coverage 86/1103; hosted CI green |
| #138 | `b33f31f` | current Codex orchestrator; Grok 4.5 High reviewers | `bfd3314` through `7bdbcfa` | Grok approved after fixes; final CodeRabbit local full-diff review clean | routine Jest/Bun 1061; slow Jest/Bun 35; coverage 1096; policies, lint, types, builds, examples, pack |
| #138 | `b33f31f` | current Codex orchestrator; Grok 4.5 High reviewers | PR #147 through merge `3c1e905` | Grok approved; CodeRabbit local clean; two hosted nits fixed and false positive withdrawn | routine Jest/Bun 1061; slow Jest/Bun 35; coverage 1096; all local gates and four hosted lanes green |
| #139 | `3c1e905` | current Codex orchestrator; Grok 4.5 High reviewers | PR #148 through current head | Grok standards/spec approved with 0 findings; CodeRabbit in-scope findings fixed, scope changes rejected | focused 50/50; Jest/Bun 1,109/1,109; policies, lint, types, builds, examples, web, pack green |

## Alignment Decisions

Expand Down
31 changes: 31 additions & 0 deletions docs/agents/runs/issue-139-coderabbit-local.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# CodeRabbit Local Review: Issue #139

## Scope

- Branch: `feature/ephemeral-relay-internals`
- Base: `staging` / fixed point `3c1e905`
- Full-diff command: `coderabbit review --agent --type all --base staging`
- Incremental command: `coderabbit review --agent --type uncommitted --base staging`

## Findings Addressed

| Severity | Finding | Resolution |
| --- | --- | --- |
| Minor | CLOSE coverage did not prove server-side subscription removal | Wait for `NostrRelay.subs` to empty, publish again, and assert no stale delivery |
| Major | NIP-46 routing re-derived the subscription ID from the map key | Route with the subscription owner's typed `subscriptionId` |
| Major | Rejected session shutdown could skip transport cleanup | Put timeout, listener, and owned-state cleanup under `finally` while preserving the original rejection |
| Major | Native server lost its only `error` listener after startup | Retain a runtime error handler for native and in-memory servers |
| Major | Timed-out test polling could continue scheduling callbacks | Make polling cancellation explicit and clear all timeout handles |

## Findings Not Addressed

| Severity | Finding | Reason |
| --- | --- | --- |
| Minor | Send an event only once when multiple filters in one subscription match | Pre-existing observable wire behavior; changing it is outside this compatibility-preserving structural ticket |
| Minor | Add a maximum connection count in transport acceptance | New product/security policy with no specified limit or existing shared policy; outside issue #139 |

## Result

- Final incremental review of the last polling fix: 0 findings.
- All production findings from the full-diff reviews were fixed except the two explicitly rejected scope changes above.
- Focused Jest/Bun regression checks and the complete project gate matrix passed after production fixes.
36 changes: 36 additions & 0 deletions docs/agents/runs/issue-139-review-packet.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Review Packet: Issue #139 Split Ephemeral Relay Internals

## Fixed Point and Spec

- Fixed point: `3c1e905`
- Diff: `git diff 3c1e905...HEAD`
- Spec: GitHub issue #139, child of cleanup run #130
- Standards: `AGENTS.md`, `CONTRIBUTING.md`, and the code-review smell baseline

## Delivered Shape

- `src/utils/ephemeral-relay.ts` remains the stable public facade and composition root.
- `transport.ts` owns native/in-memory selection, connection acceptance, fallback, runtime errors, and ordered shutdown.
- `client-session.ts` owns per-client wire protocol, subscriptions, NIP-46 routing, and socket cleanup through a narrow host interface.
- `filter-match.ts` owns pure Subscription Filter matching.
- Cache/replacement behavior stays in the facade; no new package entrypoint or production root export was added.

## Review Decisions

- Added missing JSDoc and camelCase names for the new private interfaces.
- Reused the shared diagnostics owner and moved server-client traversal behind transport.
- Added happy/error session messaging, filter semantics, transport-failure cleanup, late-error, lifecycle, and package-surface coverage.
- Retained the private transport factory: it keeps the implementation class hidden and is not accidental middle-man surface.
- Retained the cohesive client-session owner despite its size: splitting NIP-46 or wire-protocol behavior again is outside this ticket.
- Rejected a new connection cap and a legacy multi-filter delivery change because neither belongs to the compatibility-preserving structural scope.

## Verification

- Focused public/internal contracts: Jest and Bun 50/50 after CodeRabbit production fixes; final polling-only check 2/2 in both.
- Routine lanes: Jest/Bun 1,074 tests each.
- Slow lanes: Jest/Bun 35 tests each.
- Complete Jest coverage inventory: 1,109 tests.
- Green: command policy, package-manager policy, ESLint, strict TypeScript, CJS/ESM build, web entry checks, examples, and packed-consumer verification.
- Pack result: 19 referenced targets, 376 packed files, 55 web modules, 3 guarded Node fallbacks.
- CodeRabbit: all actionable in-scope findings fixed; final incremental review clean.
- Grok 4.5 final review: Standards approved with 0 hard / 0 actionable judgement findings; Spec approved with 0 code/spec findings.
57 changes: 57 additions & 0 deletions docs/agents/runs/issue-139-session.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Issue Session: #139 Split Ephemeral Relay Internals

## Issue

- Issue: #139 — Split ephemeral Relay internals
- Fixed point before session: `3c1e905` (`staging` merge of PR #147)
- Worker session: current Codex orchestrator; Grok 4.5 High is the exclusive delegated read-only reviewer
- Commits: `d6504f6`, `7a48586`, `f8e9aac`, `96dd79f` plus review artifacts
- Status: PR #148 open into `staging`; hosted rerun pending after one loaded Node 20 NIP-46 timeout

## Inputs

- Spec issue: #130 — Complete the high-impact cleanup chain
- Ticket: #139
- Relevant glossary terms: Relay, Nostr Event, Subscription Filter
- Relevant ADRs: ADR 0001 centralizes Nostr Event validation and keeps Relay behavior at the public interface
- Prototype answer and source branch, if any: none

## Implementation

- Public interface used: `NostrRelay` through `src/testing`, plus the compatible `snstr/testing` and `snstr/utils/ephemeral-relay` package subpaths
- Behaviors covered: native and in-memory connection lifecycle, session wire messages and cleanup, Subscription Filter matching, restart state reset, exact-URL disconnect observation, and package export compatibility
- `tdd` used: yes; tests stay at public `NostrRelay`/wire seams, with a compact pure matcher contract at its internal module interface
- Commands run during implementation: focused Jest/Bun public-behavior baseline 25/25; final focused owner/lifecycle/session/filter set 50/50 after production review fixes; final polling-only confirmation 2/2 in both runtimes
- Full suite command: `npm test`, `npm run test:slow`, `npm run test:bun`, `npm run test:bun:slow`, and `npm run test:coverage:all` — green at 1,074 routine + 35 slow = 1,109 tests

## Review

- Review fixed point: `3c1e905`
- Standards findings: initial Grok pass found missing JSDoc, new snake_case ownership, duplicated diagnostic coercion, and transport client-set traversal; all were fixed. Final Grok re-review approved with 0 hard and 0 actionable judgement findings.
- Spec findings: initial Grok pass found final gates/PR pending and session coverage error-only; gates are green and focused session coverage now proves REQ, EOSE, EVENT, CLOSE, malformed messages, and stale-route removal. Final Grok re-review approved with 0 code/spec findings and named the PR as the expected next action.
- Worthy fixes applied: shared diagnostic coercion/protection, transport-owned broadcast, camelCase subscription state, post-start server error handling, guaranteed shutdown cleanup, direct NIP-46 subscription routing, and cancellable test polling.
- Findings ignored with reasons: CodeRabbit's connection cap requires a new unspecified product policy; changing duplicate delivery for overlapping filters changes pre-existing observable behavior. Both are outside the structural compatibility scope.
- CodeRabbit: two full-diff passes completed; five in-scope findings fixed; final incremental review 0 findings. See `issue-139-coderabbit-local.md`.

## Design Result

- Keep `src/utils/ephemeral-relay.ts` as the stable public facade and composition root.
- Extract private connection/transport lifecycle, client-session protocol state, and pure Subscription Filter matching owners under `src/utils/ephemeral-relay/`.
- Keep cache and replaceable-event storage in the facade; do not add a shallow fourth store module.
- Do not expose internal modules through the package root or testing entrypoint.
- Give the session owner a narrow host interface for cache, subscriptions, storage, connection count, and broadcast behavior; do not leak `WebSocketServer` across that seam.

## Risks

- Close ordering is load-bearing: late connections must be rejected, sessions must drain, and the public `Relay` disconnect must be observable before `NostrRelay.close()` resolves.
- Native and in-memory transports have intentionally different shutdown mechanics that must retain identical observable behavior.
- NIP-46 kind `24133` routing currently has special `#p` behavior and must not be silently unified with general Subscription Filter matching in this structural slice.
- `cache`, `subs`, `conn`, `wss`, `store`, both package subpaths, and the legacy numeric purge option remain compatible public behavior.

## Verification Result

- Routine Jest/Bun: 1,074/1,074 each.
- Slow Jest/Bun: 35/35 each.
- Complete Jest coverage inventory: 1,109/1,109.
- Policy, lint, strict types, CJS/ESM and examples builds, web exports, and pack verification: green.
- Pack verification: 19 referenced targets, 376 packed files, 55 web modules, 3 guarded Node fallbacks.
16 changes: 16 additions & 0 deletions src/utils/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,22 @@ import {
type DiagnosticMethod = keyof DiagnosticLogger;
const SAFE_ERROR_NAME = /^(?:Error|[A-Z][A-Za-z0-9]{0,58}Error)$/;

/** Convert an unknown value into the structured diagnostic contract. */
export function asDiagnosticArgument(value: unknown): DiagnosticLogArgument {
if (
value === null ||
value === undefined ||
typeof value === "string" ||
typeof value === "number" ||
typeof value === "boolean" ||
typeof value === "object"
) {
return value;
}

return String(value);
}

/** Emit an observational diagnostic without allowing the sink to alter behavior. */
export function reportDiagnostic(
logger: DiagnosticLogger,
Expand Down
Loading
Loading