Skip to content

test: move behavior suites off private shapes #147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AustinKelsay merged 11 commits into from
Jul 19, 2026
Merged

test: move behavior suites off private shapes #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bfd3314
test: define public behavior seam contract
AustinKelsay Jul 19, 2026
93cde85
test: replace Nostr and Relay private-shape adapters
AustinKelsay Jul 19, 2026
72c8d90
test: replace NIP-46 and NIP-47 private-shape casts
AustinKelsay Jul 19, 2026
9312c38
test: remove remaining Relay and Nostr private casts
AustinKelsay Jul 19, 2026
2074453
test: tighten behavior seams after review
AustinKelsay Jul 19, 2026
7fa8772
test: restore cleanup and replacement sensitivity
AustinKelsay Jul 19, 2026
81d3710
test: close final behavior coverage gaps
AustinKelsay Jul 19, 2026
241b4b5
test: address CodeRabbit cleanup findings
AustinKelsay Jul 19, 2026
7bdbcfa
test: keep NIP-04 permission fixture restrictive
AustinKelsay Jul 19, 2026
2424807
docs: record issue 138 verification
AustinKelsay Jul 19, 2026
0cfb1c6
test: harden behavior control cleanup
AustinKelsay Jul 19, 2026
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
45 changes: 23 additions & 22 deletions docs/agents/runs/cleanup-1-9-ledger.md
View file
Open in desktop
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–6 / issues #131–#136 merged into `staging`; item 7 / issue #137 is implemented with PR #146 pending on `feature/fast-default-test-loop`
- 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`
- 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; issue #137 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; issue #138 PR pending; 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 | review cooldown | `feature/fast-default-test-loop` | Grok pass; final CodeRabbit retry pending after cooldown | routine 1063; slow 40; coverage 1103 |
| #138 public behavior test seams | AFK | blocked by #137 | `feature/public-behavior-test-seams` | pending | pending |
| #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 | 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 |

## Parked HITL Slices

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

## Issue Session Ledger

| Issue | Fixed point | Implementation owner | Commit | Review result | Checks |
| ----- | ----------- | --------------------------------------------------- | --------------------------------------------------------------------------- | ----------------------------------------------------------------------- | --------------------------------------------------------------------------------------------- |
| #131 | `f4bda34` | current Codex orchestrator; Grok 4.5 High reviewers | `7ed8433`, `00104cc`, `21b4ad9`, `426c17b` | Grok approved after hosted fixes; CodeRabbit local code review clean | focused Jest/Bun 7/7; final Jest/Bun 1054/1054; policies, lint, types, builds, examples, pack |
| #132 | `cf705f0` | current Codex orchestrator; Grok 4.5 High reviewers | `0c111d7`, `20565a0`, `abfc836`, `dea7aa0` | Grok passed; CodeRabbit local/hosted clean after four local fixes | focused 8/8; Jest/Bun 1055/1055; all local gates and four hosted lanes green |
| #133 | `46d7289` | current Codex orchestrator; Grok 4.5 High reviewers | `b238461`, `6dd75c3`, `ae15ace` | Grok standards/spec passed; CodeRabbit local/hosted clean | focused 204/204; Jest/Bun 1067/1067; all local gates and four hosted lanes green |
| #134 | `2a3556d` | current Codex orchestrator; Grok 4.5 High reviewers | `569b266` plus review artifacts | Grok standards/spec passed; CodeRabbit local/hosted clean | focused 6/6; Jest/Bun 1073/1073; all local gates and four hosted lanes green |
| #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 | implementation `0de11d9`, `91ffafa`, `295d114`, `d8de419`; review records through HEAD | Grok standards/spec passed after Bun 1.3.9 fix; clean CodeRabbit retry pending | baseline 85/1096/58.793s; routine 84/1063/32.356s; slow 2/40/43.154s; coverage 86/1103 |
| Issue | Fixed point | Implementation owner | Commit | Review result | Checks |
| ----- | ----------- | --------------------------------------------------- | ------------------------------------------ | -------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- |
| #131 | `f4bda34` | current Codex orchestrator; Grok 4.5 High reviewers | `7ed8433`, `00104cc`, `21b4ad9`, `426c17b` | Grok approved after hosted fixes; CodeRabbit local code review clean | focused Jest/Bun 7/7; final Jest/Bun 1054/1054; policies, lint, types, builds, examples, pack |
| #132 | `cf705f0` | current Codex orchestrator; Grok 4.5 High reviewers | `0c111d7`, `20565a0`, `abfc836`, `dea7aa0` | Grok passed; CodeRabbit local/hosted clean after four local fixes | focused 8/8; Jest/Bun 1055/1055; all local gates and four hosted lanes green |
| #133 | `46d7289` | current Codex orchestrator; Grok 4.5 High reviewers | `b238461`, `6dd75c3`, `ae15ace` | Grok standards/spec passed; CodeRabbit local/hosted clean | focused 204/204; Jest/Bun 1067/1067; all local gates and four hosted lanes green |
| #134 | `2a3556d` | current Codex orchestrator; Grok 4.5 High reviewers | `569b266` plus review artifacts | Grok standards/spec passed; CodeRabbit local/hosted clean | focused 6/6; Jest/Bun 1073/1073; all local gates and four hosted lanes green |
| #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 |

## Alignment Decisions

Expand Down
54 changes: 54 additions & 0 deletions docs/agents/runs/issue-138-session.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Issue 138 Session: Public Behavior Test Seams

## Fixed Point

- Base branch: `staging`
- Fixed point: `b33f31f`
- Feature branch: `feature/public-behavior-test-seams`
- Issue: #138 — Move behavior tests off private shapes

## Scope

Remove the shared `Nostr` and `Relay` private-shape adapters, rewrite tests around public behavior where possible, and place only irreducible protocol injection or fault control behind narrow exports owned by `src/testing`.

## Test-First Plan

1. Add a source contract test that rejects the legacy adapters and named broad private-shape interfaces in the targeted Relay, Nostr, NIP-46, and NIP-47 clusters.
2. Replace Nostr state reads and key mutation with `addRelay`, `getRelay`, `removeRelay`, `setPrivateKey`, and `getPublicKey`; retain a narrow relay-replacement seam only where a deterministic mock must be injected.
3. Replace Relay field access with public getters/setters, observable callbacks, mock WebSocket behavior, and narrow protocol-message/fault seams in `src/testing`.
4. Replace NIP-46 and NIP-47 broad casts with public lifecycle behavior or focused testing-entrypoint controls.
5. Run focused Jest/Bun suites, the routine and slow lanes, complete coverage, build/package gates, Grok standards/spec review, and CodeRabbit local/hosted review.

## Guardrails

- Do not expose new production API solely to make an assertion convenient.
- A testing seam must name one behavior or fault; it must not return a shadow object containing arbitrary private state.
- Preserve failure sensitivity: cleanup tests must still prove cleanup and protocol tests must still traverse the production handler.

## Status

- Local implementation and review are complete; the non-draft PR to `staging` is pending.

## Implementation Result

- Removed the shared `NostrInternals`, `getNostrInternals`, `RelayTestAccess`, and `asTestRelay` private-shape adapters.
- Reworked the targeted Nostr, Relay, NIP-46, and NIP-47 suites around public lifecycle, callbacks, relay wire behavior, or focused controls exported by `src/testing`.
- Added a source contract that prevents the removed adapters and broad private-shape patterns from returning in the targeted suites.
- Extracted the NIP-46 replay guard into an internal module and exercised replay rejection, lifecycle reset, timer cleanup, relay reconnect policy, socket replacement, and replaceable-event ordering through behavior-sensitive tests.
- Kept NIP-04 permissions out of the shared core fixture and granted them only in the encryption compatibility test.

## Review Result

- Grok design, standards, and specification review: approved after the behavior-sensitivity fixes.
- CodeRabbit local review: clean, 0 findings on the final full diff after five earlier minor findings were fixed.
- No public replay-window option was added: exposing product configuration only to support a test would violate this issue's guardrail, while the internal replay guard remains directly testable.

## Verification Result

- Routine Jest: 86 suites, 1,061 tests passed.
- Slow Jest: 35 tests passed.
- Routine Bun: 1,061 tests passed.
- Slow Bun: 35 tests passed.
- Complete Jest coverage lane: 88 suites, 1,096 tests passed; 80.72% statements, 68.59% branches, 82.63% functions, and 81.20% lines.
- Commands policy, package-manager policy, lint, strict TypeScript, build, examples build, and package verification passed.
- Final NIP-04 permission adjustment passed its focused Jest and Bun test plus lint and strict TypeScript.
70 changes: 18 additions & 52 deletions src/nip46/bunker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
import { LogLevel } from "../utils/logger";
import { NIP46DiagnosticLogger } from "./utils/diagnostics";
import { NIP46BunkerEngine } from "./internal/bunker-engine";
import { NIP46ReplayGuard } from "./internal/replay-guard";

export class NostrRemoteSignerBunker {
private readonly engine: NIP46BunkerEngine;
Expand All @@ -49,7 +50,7 @@ export class NostrRemoteSignerBunker {
params: string[],
) => boolean | null)
| null = null;
private usedRequestIds: Map<string, number> = new Map(); // Request ID -> timestamp
private readonly replayGuard = new NIP46ReplayGuard();
private cleanupInterval: NodeJS.Timeout | null = null; // For cleanup interval management

constructor(options: NIP46BunkerOptions) {
Expand Down Expand Up @@ -125,10 +126,15 @@ export class NostrRemoteSignerBunker {
},
};
},
beforeRequest: (request) =>
this.isReplayAttack(request.id)
? { action: "drop" }
: { action: "continue" },
beforeRequest: (request) => {
const replay = this.replayGuard.isReplay(request.id);
if (replay) {
this.logger.warn("Replay attack detected", {
requestId: request.id,
});
}
return replay ? { action: "drop" } : { action: "continue" };
},
handlers: {
[NIP46Method.CONNECT]: (request, clientPubkey) =>
this.handleConnect(request, clientPubkey),
Expand Down Expand Up @@ -176,7 +182,7 @@ export class NostrRemoteSignerBunker {
}
this.connectedClients.clear();
this.pendingAuthChallenges.clear();
this.usedRequestIds.clear();
this.replayGuard.clear();
},
});

Expand Down Expand Up @@ -259,51 +265,6 @@ export class NostrRemoteSignerBunker {
this.logger.debug("Custom permission handler cleared");
}

/**
* Check if a request ID has been used before (replay attack prevention)
* @private
*/
private isReplayAttack(requestId: string): boolean {
const now = Date.now();
const requestTime = this.usedRequestIds.get(requestId);

if (requestTime !== undefined) {
// This request ID has been seen before
this.logger.warn("Replay attack detected", {
requestId,
originalTime: new Date(requestTime).toISOString(),
attemptTime: new Date(now).toISOString(),
});
return true;
}

// Store the request ID with current timestamp
this.usedRequestIds.set(requestId, now);

return false;
}

/**
* Clean up old request IDs to prevent memory leaks
* @private
*/
private cleanupOldRequestIds(): void {
const now = Date.now();
const maxAge = 120000; // 2 minutes - reduced from 1 hour for better security
let cleaned = 0;

for (const [requestId, timestamp] of this.usedRequestIds.entries()) {
if (now - timestamp > maxAge) {
this.usedRequestIds.delete(requestId);
cleaned++;
}
}

if (cleaned > 0) {
this.logger.debug("Cleaned up old request IDs", { count: cleaned });
}
}

/**
* Resolve an auth challenge by marking it as resolved
* @param pubkey The client pubkey that completed authentication
Expand Down Expand Up @@ -389,7 +350,12 @@ export class NostrRemoteSignerBunker {
* Periodic cleanup of expired data
*/
private cleanup(): void {
this.cleanupOldRequestIds();
const cleanedRequestIds = this.replayGuard.cleanup();
if (cleanedRequestIds > 0) {
this.logger.debug("Cleaned up old request IDs", {
count: cleanedRequestIds,
});
}

// Clean up expired auth challenges
const now = Date.now();
Expand Down
Loading
Loading