Skip to content

Add openWarp timeline facade#728

Merged
flyingrobots merged 11 commits into
mainfrom
v19-openwarp-timeline-facade
Jul 8, 2026
Merged

Add openWarp timeline facade#728
flyingrobots merged 11 commits into
mainfrom
v19-openwarp-timeline-facade

Conversation

@flyingrobots

Copy link
Copy Markdown
Member

Summary

  • Add the v19 openWarp({ storage, writer }) product opener.
  • Add public Warp and Timeline facade handles to root and browser entrypoints.
  • Keep legacy worldline runtime details behind an internal timeline runtime bridge.
  • Add unit and consumer typecheck coverage for the new facade.
  • Regenerate the source-backed public reference.

Fixes #722

Validation

  • npx vitest run test/unit/domain/WarpFacade.test.ts
  • npm run typecheck:src
  • npm run typecheck:test
  • npm run typecheck:consumer
  • npm run typecheck:surface
  • npm run lint -- --quiet
  • npm run lint:source-backed-reference
  • npm run lint:docs-topology
  • npm run test:local
  • pre-push IRONCLAD M9 gates passed

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds the v19 openWarp()/Warp/Timeline facade API, with shared identity validation, runtime timeline association, root exports, reference docs, changelog notes, and compile-time/unit coverage.

Changes

Warp/Timeline facade

Layer / File(s) Summary
Identity validation
src/domain/api/assertIdentity.ts, src/domain/api/OpenWarpIdentityFailure.ts
Adds the shared identity assertion helper and the open-warp failure descriptor used by facade validation.
Timeline class and runtime association
src/domain/api/Timeline.ts, src/domain/api/TimelineRuntime.ts
Adds the frozen Timeline facade with validated name/writer, plus createTimeline/requireTimelineRuntime mapping timelines to WarpWorldline runtimes via WeakMap.
Warp class and openWarp factory
src/domain/api/Warp.ts, src/domain/api/openWarp.ts
Adds the frozen Warp facade, validates opener options, and opens worldlines via openWarpWorldline before wrapping results as Timeline.
Root exports, docs, and changelog
index.ts, docs/topics/reference.md, CHANGELOG.md
Re-exports openWarp, Warp, Timeline, OpenWarpOptions, and WarpStorage from the root; regenerates reference export inventories; documents the v19 facade and boundary changes.
Type-check and unit tests
test/type-check/tsconfig.json, test/type-check/v19-consumer.ts, test/unit/domain/WarpFacade.test.ts
Adds a compile-only consumer and unit tests covering facade behavior, export exclusions, hidden internals, and validation errors.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant openWarp
  participant Warp
  participant openWarpWorldline
  participant createTimeline
  participant Timeline
  Caller->>openWarp: openWarp({ storage, writer })
  openWarp->>Warp: new Warp(writer, openTimeline)
  Caller->>Warp: warp.timeline('events')
  Warp->>openWarpWorldline: open(storage, writer, 'events')
  openWarpWorldline-->>Warp: WarpWorldline
  Warp->>createTimeline: createTimeline(runtime)
  createTimeline->>Timeline: new Timeline(worldlineName, writerId)
  createTimeline-->>Caller: Timeline
Loading

Possibly related PRs

  • git-stunts/git-warp#679: Updates the reference-doc generation path that reflects the new root exports in docs/topics/reference.md.
  • git-stunts/git-warp#726: Also changes the package-root export surface and boundary checks around v19 public API exposure.

Poem

A warp opens wide, then names a stream,
A rabbit hops by with a tidy dream. 🐇
Warp and Timeline gleam at the gate,
With roots and tests all neatly in state.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Concise and specific; it clearly describes the main change: adding the openWarp timeline facade.
Description check ✅ Passed Covers summary, issue reference, and validation; only the ADR checklist section is missing.
Linked Issues check ✅ Passed Implements openWarp/Warp/Timeline, keeps openWarpWorldline off the root, and hides internals behind the facade.
Out of Scope Changes check ✅ Passed Changes stay focused on the v19 facade, tests, docs, and generated reference; no unrelated edits stand out.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/domain/api/openWarp.ts`:
- Around line 15-29: openWarp() currently validates options before creating the
promise, so invalid input throws synchronously instead of returning a rejected
Promise<Warp>. Update openWarp() to run assertOpenWarpOptions() inside the
async/promise path—either make openWarp() async or wrap the validation and Warp
construction in the promise chain—so bad options are rejected consistently. Use
the openWarp() and assertOpenWarpOptions() symbols to keep the fix localized.

In `@src/domain/api/Timeline.ts`:
- Around line 36-44: `assertTimelineIdentity` duplicates the same empty-string
validation already implemented by `assertNonEmpty` in `Warp.ts`; consolidate
both into one shared identity validator so the logic stays in sync. Extract the
shared check into a reusable helper (for example, a parameterized
`assertIdentity`/similar function), then update `Timeline.ts` and `Warp.ts` to
call it with their respective message/code values while preserving the existing
`WarpError` context shape.

In `@src/domain/api/Warp.ts`:
- Around line 38-46: The shared non-empty identity check currently lives in the
Warp.ts module alongside the Warp class, but it should be extracted into a
dedicated validator module and reused by both Warp.ts and Timeline.ts. Move
assertNonEmpty into a shared helper (for example, assertIdentity.ts), update
openWarp.ts and the Timeline.ts identity validation to import and use that
helper, and remove the duplicate private assertTimelineIdentity logic so the
identity validation is centralized.

In `@test/unit/domain/WarpFacade.test.ts`:
- Around line 1-73: Add a regression test to verify the v19 root export surface
does not include openWarpWorldline. In WarpFacade.test.ts, extend the existing
facade/export coverage by importing the root module via index.ts and asserting
that openWarpWorldline is not present on that module object, alongside the
current openWarp, Warp, and Timeline checks. This keeps the public facade
contract locked down and protects the explicit root-export restriction.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 354b0504-97b1-44a6-8263-d579f91e3261

📥 Commits

Reviewing files that changed from the base of the PR and between 7e68bdf and 9a8bd50.

📒 Files selected for processing (11)
  • CHANGELOG.md
  • browser.ts
  • docs/topics/reference.md
  • index.ts
  • src/domain/api/Timeline.ts
  • src/domain/api/TimelineRuntime.ts
  • src/domain/api/Warp.ts
  • src/domain/api/openWarp.ts
  • test/type-check/tsconfig.json
  • test/type-check/v19-consumer.ts
  • test/unit/domain/WarpFacade.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: coverage-threshold
  • GitHub Check: test-node (22)
  • GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx,js,jsx}: Do not introduce any, as any, as unknown as, unknown (outside adapters), Record<string, unknown> (outside adapters), *Like placeholder types, JSON.parse/JSON.stringify (outside adapters), fetch (outside adapters), process.env (outside adapters), @ts-ignore, or z.any() in core code; use validated boundary models and ports instead.
Use constructor-injected ports for external capabilities; do not rely on ambient dependencies for I/O, clocks, persistence, or entropy.
Do not create utils.ts, helpers.ts, misc.ts, or common.ts; name files after the actual concept they model.
Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.
Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.
No enum usage; prefer runtime-backed domain forms and unions.
Do not use boolean trap parameters; prefer named option objects or separate methods.
Avoid magic strings or numbers when a named constant should exist.
Keep domain bytes as Uint8Array; Buffer belongs in infrastructure adapters.

Files:

  • src/domain/api/Timeline.ts
  • src/domain/api/Warp.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/TimelineRuntime.ts
src/domain/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/domain/**/*.{ts,tsx,js,jsx}: In src/domain/**, do not use Date.now(), new Date(), Date(), performance.now(), Math.random(), crypto.randomUUID(), crypto.getRandomValues(), setTimeout, setInterval, raw new Error(...)/new TypeError(...), or direct imports from Node built-ins; time, entropy, and external capabilities must enter through ports or parameters, and domain errors should extend WarpError.
Construct domain objects only in core when doing so establishes validated runtime truth; do not build infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core.
Prefer discriminated unions and explicit result types instead of boolean-flag bags, and model expected failures as return values rather than exceptions.
src/domain/ must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.
Domain code must not use the wall clock directly; time must enter through a port or parameter.

Files:

  • src/domain/api/Timeline.ts
  • src/domain/api/Warp.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/TimelineRuntime.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use explicit domain concepts with validated constructors, Object.freeze, and instanceof dispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.

Files:

  • src/domain/api/Timeline.ts
  • src/domain/api/Warp.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/TimelineRuntime.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Do not use direct imports from src/infrastructure/** in src/domain/** or src/ports/**; depend on a port instead.
Do not use direct Node built-ins in src/domain/** or src/ports/**; use a port instead.

Files:

  • src/domain/api/Timeline.ts
  • index.ts
  • browser.ts
  • test/type-check/v19-consumer.ts
  • src/domain/api/Warp.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/TimelineRuntime.ts
  • test/unit/domain/WarpFacade.test.ts
🧠 Learnings (1)
📚 Learning: 2026-03-08T19:50:17.519Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 65
File: CHANGELOG.md:88-88
Timestamp: 2026-03-08T19:50:17.519Z
Learning: Follow the Keep a Changelog convention for CHANGELOG.md. Allow duplicate subheadings across versions (e.g., '### Added', '### Fixed'). Configure markdownlint MD024 with {"siblings_only": true} to avoid cross-version false positives.

Applied to files:

  • CHANGELOG.md
🔇 Additional comments (10)
index.ts (1)

18-22: LGTM!

browser.ts (1)

25-29: LGTM!

docs/topics/reference.md (1)

33-118: LGTM!

CHANGELOG.md (1)

12-13: LGTM!

test/type-check/tsconfig.json (1)

11-11: LGTM!

test/type-check/v19-consumer.ts (1)

1-40: LGTM!

src/domain/api/Timeline.ts (1)

15-34: LGTM!

src/domain/api/TimelineRuntime.ts (1)

1-22: LGTM!

src/domain/api/Warp.ts (1)

17-36: LGTM!

src/domain/api/openWarp.ts (1)

31-39: LGTM!

Comment thread src/domain/api/openWarp.ts
Comment thread src/domain/api/Timeline.ts Outdated
Comment thread src/domain/api/Warp.ts Outdated
Comment thread test/unit/domain/WarpFacade.test.ts
@flyingrobots

Copy link
Copy Markdown
Member Author

@codex second opinion requested.

Phase I self-audit found one additional issue beyond the existing CodeRabbit threads:

Severity Source File Lines Issue Required action
P2 Self src/domain/api/Warp.ts, src/domain/api/Timeline.ts constructors Public facade constructors dereference nullable options before establishing runtime truth, so JavaScript callers can get raw host TypeError instead of WarpError. Add deterministic constructor-option guards and regression coverage.

@flyingrobots

Copy link
Copy Markdown
Member Author

@codex second opinion requested.

Follow-up self-audit during validation found one additional patch-local issue:

Severity Source File Lines Issue Required action
P2 Self src/domain/api/openWarp.ts function body The first async-rejection fix used an async function only to normalize thrown validation, tripping @typescript-eslint/require-await. Keep validation inside the promise path using an explicit promise boundary instead of a no-await async function.

@flyingrobots

Copy link
Copy Markdown
Member Author

Activity Summary

# Severity Source File Commit Outcome
1 P2 PR src/domain/api/openWarp.ts c498cbe0 + 79157325 openWarp() validation now runs inside the promise path without violating require-await; invalid options reject instead of throwing synchronously.
2 P2 PR src/domain/api/Timeline.ts, src/domain/api/Warp.ts, src/domain/api/assertIdentity.ts d5ebbedb Identity validation is centralized in assertIdentity.ts; duplicate Timeline/Warp validators removed.
3 P2 Self src/domain/api/Warp.ts, src/domain/api/Timeline.ts 6e191c70 Public facade constructors now reject invalid construction options with WarpError instead of raw host TypeError.
4 P5 PR test/unit/domain/WarpFacade.test.ts 0f66c4ee Added regression proving openWarpWorldline stays off the package root export surface.
5 P2 Self src/domain/api/openWarp.ts 79157325 Replaced no-await async normalization with an explicit promise boundary to satisfy lint while preserving rejection semantics.

Validation:

  • npm exec -- vitest run test/unit/domain/WarpFacade.test.ts
  • npm run typecheck
  • npm run typecheck:consumer
  • npm run typecheck:surface
  • npm run lint -- --quiet
  • npm run lint:source-backed-reference
  • VITEST_MAX_WORKERS=1 npm run test:local
  • pre-push IRONCLAD M9 gates passed, including stable unit shards

Resolved review threads: 4/4.

Console SitRep: patch-local findings are closed and pushed; CI/CodeRabbit should now rerun on 79157325.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/domain/api/Warp.ts`:
- Around line 24-27: The identity failure payload is duplicated across Warp
validation calls, so extract the repeated message/code pair into a shared
constant and reuse it everywhere. Update the `assertIdentity` calls in `Warp`
(constructor and `timeline`) and in `assertOpenWarpOptions` in `openWarp.ts` to
reference that constant instead of inline literals, keeping the failure text and
code consistent in one place.

In `@test/unit/domain/WarpFacade.test.ts`:
- Around line 11-13: The current WarpFacade test relies on readRepoSource and
raw toContain/not.toContain string matching, which is brittle and tied to file
text rather than the centralized-validation invariant. Replace those source-text
assertions with a behavior- or structure-based check in WarpFacade.test.ts,
using the relevant Warp and Timeline module imports or a lint/AST-based
assertion to verify no duplicate validator is exported instead of grepping
repository source.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 861df14b-cb37-45e8-8057-431a9c626492

📥 Commits

Reviewing files that changed from the base of the PR and between 9a8bd50 and 7915732.

📒 Files selected for processing (5)
  • src/domain/api/Timeline.ts
  • src/domain/api/Warp.ts
  • src/domain/api/assertIdentity.ts
  • src/domain/api/openWarp.ts
  • test/unit/domain/WarpFacade.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: coverage-threshold
  • GitHub Check: type-firewall-lint
  • GitHub Check: test-node (22)
  • GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx,js,jsx}: Do not introduce any, as any, as unknown as, unknown (outside adapters), Record<string, unknown> (outside adapters), *Like placeholder types, JSON.parse/JSON.stringify (outside adapters), fetch (outside adapters), process.env (outside adapters), @ts-ignore, or z.any() in core code; use validated boundary models and ports instead.
Use constructor-injected ports for external capabilities; do not rely on ambient dependencies for I/O, clocks, persistence, or entropy.
Do not create utils.ts, helpers.ts, misc.ts, or common.ts; name files after the actual concept they model.
Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.
Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.
No enum usage; prefer runtime-backed domain forms and unions.
Do not use boolean trap parameters; prefer named option objects or separate methods.
Avoid magic strings or numbers when a named constant should exist.
Keep domain bytes as Uint8Array; Buffer belongs in infrastructure adapters.

Files:

  • src/domain/api/assertIdentity.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/Timeline.ts
  • src/domain/api/Warp.ts
src/domain/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/domain/**/*.{ts,tsx,js,jsx}: In src/domain/**, do not use Date.now(), new Date(), Date(), performance.now(), Math.random(), crypto.randomUUID(), crypto.getRandomValues(), setTimeout, setInterval, raw new Error(...)/new TypeError(...), or direct imports from Node built-ins; time, entropy, and external capabilities must enter through ports or parameters, and domain errors should extend WarpError.
Construct domain objects only in core when doing so establishes validated runtime truth; do not build infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core.
Prefer discriminated unions and explicit result types instead of boolean-flag bags, and model expected failures as return values rather than exceptions.
src/domain/ must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.
Domain code must not use the wall clock directly; time must enter through a port or parameter.

Files:

  • src/domain/api/assertIdentity.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/Timeline.ts
  • src/domain/api/Warp.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use explicit domain concepts with validated constructors, Object.freeze, and instanceof dispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.

Files:

  • src/domain/api/assertIdentity.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/Timeline.ts
  • src/domain/api/Warp.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Do not use direct imports from src/infrastructure/** in src/domain/** or src/ports/**; depend on a port instead.
Do not use direct Node built-ins in src/domain/** or src/ports/**; use a port instead.

Files:

  • src/domain/api/assertIdentity.ts
  • test/unit/domain/WarpFacade.test.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/Timeline.ts
  • src/domain/api/Warp.ts
🔇 Additional comments (10)
test/unit/domain/WarpFacade.test.ts (2)

59-63: Regression test added as requested.

This addresses the previously requested coverage confirming openWarpWorldline is not a root export.


1-14: LGTM!

Also applies to: 15-58, 65-83, 95-110

src/domain/api/openWarp.ts (3)

34-45: Validation logic is correct. Note: the E_OPEN_WARP_IDENTITY failure literal here duplicates the same pair used twice in Warp.ts — see the consolidation suggestion raised there rather than repeating it here.


1-14: LGTM!


16-32: LGTM! This resolves the previously flagged issue where openWarp() threw synchronously instead of rejecting the returned promise — validation now runs inside the Promise.resolve().then() callback.

src/domain/api/assertIdentity.ts (1)

1-16: LGTM!

src/domain/api/Timeline.ts (2)

16-42: LGTM! The prior duplicate identity-validation logic flagged against assertTimelineIdentity is now resolved via the shared assertIdentity helper.


44-51: LGTM!

src/domain/api/Warp.ts (2)

22-31: LGTM overall — the prior "extract shared validator" concern is resolved via assertIdentity. One residual DRY gap remains: see refactor suggestion below.

Also applies to: 37-43


46-53: LGTM!

Comment thread src/domain/api/Warp.ts Outdated
Comment thread test/unit/domain/WarpFacade.test.ts Outdated
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@flyingrobots

Copy link
Copy Markdown
Member Author

@codex please confirm this self-review finding.

Severity File Lines Type Finding Recommended mitigation
P2 test/unit/domain/WarpFacade.test.ts 33-38 Test isolation / global runtime state The new browser export test imports ../../../browser.ts at runtime. That module calls installDefaultRuntimeHostBrowserPorts() at top level (browser.ts lines 22-24), which mutates the module-level resolver slots in src/domain/warp/RuntimeHostPortResolvers.ts lines 14-36. This test file has already imported ../../../index.ts (test/unit/domain/WarpFacade.test.ts lines 4-8), so Node defaults are installed first (index.ts lines 14-16), then the browser import overwrites codec/crypto defaults without any restore. Later tests in the same Vitest worker can inherit browser defaults, and this assertion can also be masked by the earlier Node-root side effect. Replace the side-effectful browser-root import with source-backed export-surface inspection, or add a scoped restore/reset path for runtime host defaults and restore Node defaults in finally. Prefer avoiding the runtime import in this unit test because the assertion only needs export identity, not browser boot side effects.

Agent prompt: Verify that the browser-root export assertion does not mutate shared RuntimeHostPortResolvers state. Replace await import('../../../browser.ts') in test/unit/domain/WarpFacade.test.ts with a deterministic export-surface check, or introduce a scoped resolver reset/restore API and add regression coverage proving Node defaults remain intact after the browser-root assertion.

@flyingrobots

Copy link
Copy Markdown
Member Author

Resolved the self-review finding from #728 (comment).

Severity File Commit Outcome
P2 browser.ts, test/unit/domain/WarpFacade.test.ts, CHANGELOG.md 96cf7a5c Removed the branch-added v19 facade exports from the browser root, replaced the side-effectful browser.ts runtime import test with a source-backed negative export assertion, and corrected the changelog to state that the v19 facade is package-root only.

Verification:

Gate Result
RED regression WARP_TEST_MIN_FREE_MB=64 npm run test:local -- --run test/unit/domain/WarpFacade.test.ts failed before the fix because browser.ts still exported openWarp.
Targeted suite WARP_TEST_MIN_FREE_MB=64 npm run test:local -- --run test/unit/domain/WarpFacade.test.ts passed after the fix.
Full local tests WARP_TEST_MIN_FREE_MB=64 npm run test:local passed.
Lint npm run lint passed.
Typecheck npm run typecheck passed.
Markdown lint npm run lint:md -- --ignore node_modules passed.
Source-backed reference npm run lint:source-backed-reference passed.
Pre-push IRONCLAD M9 passed during git push.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
test/unit/domain/WarpFacade.test.ts (2)

82-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Brittle source-string assertions for centralized validation, previously flagged.

Same concern as raised earlier in this review thread: relying on readRepoSource + toContain/not.toContain on raw file text to prove "no duplicate validator" will break on harmless refactors (reformatting, renaming, moving functions) without verifying the actual invariant. Prefer a behavior-based check (e.g., import Warp/Timeline modules and assert they don't export a validator function) or an AST/lint-based rule.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/unit/domain/WarpFacade.test.ts` around lines 82 - 90, The test in
WarpFacade.test.ts is using brittle raw source-string checks via readRepoSource
and toContain/not.toContain to enforce validator placement, so refactors can
break it without changing behavior. Replace this with a behavior-based assertion
by importing the Warp and Timeline modules directly and checking their exports,
or use an AST/lint-style check, while still verifying that assertIdentity
remains exposed only from assertIdentity and not from Warp or Timeline.

33-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Same source-grep brittleness as previously flagged, now also here.

Asserting the browser-root boundary via raw toContain/not.toContain string checks on browser.ts's source ties the test to file formatting rather than the actual export surface. A harmless reformat or reordering of the export statement (e.g., splitting across lines, changing quote style) would silently pass despite verifying nothing real, or could false-fail on a valid refactor.

Since the dynamic-import approach was intentionally dropped to avoid mutating shared runtime state, consider asserting against a parsed AST of browser.ts (e.g., via TypeScript's compiler API) instead of raw substring matching, to keep the check both safe and resilient to formatting changes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/unit/domain/WarpFacade.test.ts` around lines 33 - 39, The browser-root
boundary test in WarpFacade.test.ts is relying on brittle raw source substring
checks against browser.ts instead of the actual export surface. Update the test
to inspect a parsed AST for browser.ts, using a TypeScript compiler API-based
helper or equivalent, and assert that the relevant exports (openWarp, Warp,
Timeline, OpenWarpOptions, WarpStorage) are absent from the browser entrypoint
regardless of formatting or line wrapping. Keep the existing
readRepoSource/browser.ts lookup but replace the toContain/not.toContain
assertions with symbol-aware export detection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@test/unit/domain/WarpFacade.test.ts`:
- Around line 82-90: The test in WarpFacade.test.ts is using brittle raw
source-string checks via readRepoSource and toContain/not.toContain to enforce
validator placement, so refactors can break it without changing behavior.
Replace this with a behavior-based assertion by importing the Warp and Timeline
modules directly and checking their exports, or use an AST/lint-style check,
while still verifying that assertIdentity remains exposed only from
assertIdentity and not from Warp or Timeline.
- Around line 33-39: The browser-root boundary test in WarpFacade.test.ts is
relying on brittle raw source substring checks against browser.ts instead of the
actual export surface. Update the test to inspect a parsed AST for browser.ts,
using a TypeScript compiler API-based helper or equivalent, and assert that the
relevant exports (openWarp, Warp, Timeline, OpenWarpOptions, WarpStorage) are
absent from the browser entrypoint regardless of formatting or line wrapping.
Keep the existing readRepoSource/browser.ts lookup but replace the
toContain/not.toContain assertions with symbol-aware export detection.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3c67b533-5a8d-47b2-abbd-70b677c0c001

📥 Commits

Reviewing files that changed from the base of the PR and between 7915732 and 96cf7a5.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • test/unit/domain/WarpFacade.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: preflight
  • GitHub Check: test-node (22)
  • GitHub Check: coverage-threshold
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Do not use direct imports from src/infrastructure/** in src/domain/** or src/ports/**; depend on a port instead.
Do not use direct Node built-ins in src/domain/** or src/ports/**; use a port instead.

Files:

  • test/unit/domain/WarpFacade.test.ts
🧠 Learnings (1)
📚 Learning: 2026-03-08T19:50:17.519Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 65
File: CHANGELOG.md:88-88
Timestamp: 2026-03-08T19:50:17.519Z
Learning: Follow the Keep a Changelog convention for CHANGELOG.md. Allow duplicate subheadings across versions (e.g., '### Added', '### Fixed'). Configure markdownlint MD024 with {"siblings_only": true} to avoid cross-version false positives.

Applied to files:

  • CHANGELOG.md
🔇 Additional comments (6)
test/unit/domain/WarpFacade.test.ts (5)

56-60: Regression test for openWarpWorldline absence from root exports, as previously requested, is now present and correctly implemented.


1-31: LGTM!


42-54: LGTM!


62-80: LGTM!


92-107: LGTM!

CHANGELOG.md (1)

12-18: LGTM!

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@flyingrobots

Copy link
Copy Markdown
Member Author

Code Lawyer Activity Summary

# Source Severity File Commit Outcome
1 PR P2 src/domain/api/Warp.ts, src/domain/api/openWarp.ts, src/domain/api/OpenWarpIdentityFailure.ts, test/unit/domain/WarpFacade.test.ts 82c665a7 Centralized the repeated openWarp identity failure payload into OPEN_WARP_IDENTITY_FAILURE and reused it across Warp and openWarp validation.
2 PR P4 test/unit/domain/WarpFacade.test.ts c882cee5 Replaced raw source-string toContain checks with TypeScript AST export inspection so facade tests assert structure instead of grepping source text.

Resolved review threads by GraphQL:

Thread Result
PRRT_kwDOQ8bKSs6PIRyZ resolved
PRRT_kwDOQ8bKSs6PIRyf resolved

Verification:

Gate Result
RED #1 WARP_TEST_MIN_FREE_MB=64 npm run test:local -- --run test/unit/domain/WarpFacade.test.ts failed before the constant existed.
GREEN #1 Targeted facade suite passed after 82c665a7.
RED #2 Targeted facade suite failed on missing exportedNamesFor() after removing raw source-grep call sites.
GREEN #2 Targeted facade suite passed after c882cee5.
Typecheck npm run typecheck passed.
Lint npm run lint passed.
Markdown lint npm run lint:md -- --ignore node_modules passed.
Source-backed reference npm run lint:source-backed-reference passed.
Whitespace git diff --check origin/main...HEAD passed.
Full local tests WARP_TEST_MIN_FREE_MB=64 npm run test:local passed.
Pre-push IRONCLAD M9 passed during git push.

Self-audit: no additional self-discovered issues beyond the two unresolved PR review threads.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/unit/domain/WarpFacade.test.ts`:
- Around line 32-72: The current check only collects exported names, so it can
miss reintroduced private duplicates like assertTimelineIdentity or
assertNonEmpty. Update the AST scanning in
exportedNamesFor/collectExportedDeclarationName to also inspect all top-level
declarations via the broader allDeclaredNamesFor-style walk, and include
non-exported function/variable declarations when matching the banned
identifiers. Keep the existing export-based logic, but add a parallel pass that
flags those private names even when they are not exported.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 10a0e0d5-d373-412d-b036-a364b65a4c1f

📥 Commits

Reviewing files that changed from the base of the PR and between 96cf7a5 and c882cee.

📒 Files selected for processing (4)
  • src/domain/api/OpenWarpIdentityFailure.ts
  • src/domain/api/Warp.ts
  • src/domain/api/openWarp.ts
  • test/unit/domain/WarpFacade.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: coverage-threshold
  • GitHub Check: test-node (22)
  • GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx,js,jsx}: Do not introduce any, as any, as unknown as, unknown (outside adapters), Record<string, unknown> (outside adapters), *Like placeholder types, JSON.parse/JSON.stringify (outside adapters), fetch (outside adapters), process.env (outside adapters), @ts-ignore, or z.any() in core code; use validated boundary models and ports instead.
Use constructor-injected ports for external capabilities; do not rely on ambient dependencies for I/O, clocks, persistence, or entropy.
Do not create utils.ts, helpers.ts, misc.ts, or common.ts; name files after the actual concept they model.
Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.
Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.
No enum usage; prefer runtime-backed domain forms and unions.
Do not use boolean trap parameters; prefer named option objects or separate methods.
Avoid magic strings or numbers when a named constant should exist.
Keep domain bytes as Uint8Array; Buffer belongs in infrastructure adapters.

Files:

  • src/domain/api/OpenWarpIdentityFailure.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/Warp.ts
src/domain/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/domain/**/*.{ts,tsx,js,jsx}: In src/domain/**, do not use Date.now(), new Date(), Date(), performance.now(), Math.random(), crypto.randomUUID(), crypto.getRandomValues(), setTimeout, setInterval, raw new Error(...)/new TypeError(...), or direct imports from Node built-ins; time, entropy, and external capabilities must enter through ports or parameters, and domain errors should extend WarpError.
Construct domain objects only in core when doing so establishes validated runtime truth; do not build infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core.
Prefer discriminated unions and explicit result types instead of boolean-flag bags, and model expected failures as return values rather than exceptions.
src/domain/ must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.
Domain code must not use the wall clock directly; time must enter through a port or parameter.

Files:

  • src/domain/api/OpenWarpIdentityFailure.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/Warp.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use explicit domain concepts with validated constructors, Object.freeze, and instanceof dispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.

Files:

  • src/domain/api/OpenWarpIdentityFailure.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/Warp.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Do not use direct imports from src/infrastructure/** in src/domain/** or src/ports/**; depend on a port instead.
Do not use direct Node built-ins in src/domain/** or src/ports/**; use a port instead.

Files:

  • src/domain/api/OpenWarpIdentityFailure.ts
  • src/domain/api/openWarp.ts
  • src/domain/api/Warp.ts
  • test/unit/domain/WarpFacade.test.ts
🔇 Additional comments (4)
src/domain/api/OpenWarpIdentityFailure.ts (1)

1-4: LGTM! Centralizes the previously duplicated identity-failure payload as requested in prior review feedback.

src/domain/api/Warp.ts (1)

1-39: LGTM! Resolves the prior duplicate-validator and duplicated-failure-literal review comments; identity validation is now centralized via assertIdentity + OPEN_WARP_IDENTITY_FAILURE, and timeline() correctly surfaces validation failures as promise rejections.

Also applies to: 41-48

src/domain/api/openWarp.ts (1)

1-9: LGTM! Promise.resolve().then(...) wrapping correctly defers assertOpenWarpOptions validation into the promise path (resolving the prior sync-throw issue), and OPEN_WARP_IDENTITY_FAILURE reuse removes the previously duplicated failure literal.

Also applies to: 10-15, 17-33, 35-43

test/unit/domain/WarpFacade.test.ts (1)

74-147: LGTM! Behavioral coverage for openWarp/Warp/Timeline (facade instantiation, freezing, hidden internals, root/browser export parity, validation error messages, and the OPEN_WARP_IDENTITY_FAILURE payload) matches the implementation and prior review resolutions.

Comment thread test/unit/domain/WarpFacade.test.ts
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@flyingrobots

Copy link
Copy Markdown
Member Author

Resolved the remaining CodeRabbit thread at test/unit/domain/WarpFacade.test.ts.

Thread Commit Outcome
PRRT_kwDOQ8bKSs6PKxIW 08b5ab9d Added allDeclaredNamesFor() and collectDeclaredStatementName() so the AST guard now scans top-level private declarations, not only exported names. The browser-root check still uses export-surface inspection; the identity-validator placement check now catches private assertNonEmpty / assertTimelineIdentity reintroductions.

Verification:

Gate Result
RED WARP_TEST_MIN_FREE_MB=64 npm run test:local -- --run test/unit/domain/WarpFacade.test.ts failed before the helper existed.
Targeted suite WARP_TEST_MIN_FREE_MB=64 npm run test:local -- --run test/unit/domain/WarpFacade.test.ts passed.
Typecheck npm run typecheck passed.
Lint npm run lint passed.
Pre-push IRONCLAD M9 passed during git push.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 8, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@flyingrobots

Copy link
Copy Markdown
Member Author

@codex self-review confirmation requested.

Code Lawyer Self-Review Findings

Severity File Lines Infraction Evidence Recommended mitigation
P2 Major src/domain/api/openWarp.ts, src/domain/api/Warp.ts, src/domain/api/Timeline.ts, src/domain/api/assertIdentity.ts, src/domain/utils/RefLayout.ts openWarp.ts:35-43, Warp.ts:24-36, Timeline.ts:21-35, assertIdentity.ts:8-16, RefLayout.ts:85-160 Public facade identity validation is weaker than the runtime ref-layout model. The new public facade accepts and exposes non-empty-but-invalid identities via assertIdentity, while the runtime ref layout rejects invalid graph names and writer IDs with validateGraphName() and validateWriterId(). openWarp({ writer: 'bad writer' }) can resolve a Warp whose public writer is not a valid runtime writer ID, and Warp.timeline('bad name') delegates invalid timeline names to the lower runtime instead of rejecting at the facade boundary. new Timeline({ name: 'bad name', writer: 'bad writer' }) also succeeds even though the exported runtime-backed value represents an invalid graph/writer identity. Align the facade with runtime truth: add deterministic tests for invalid-but-nonempty writer/timeline identities, then delegate facade validation to the canonical graph/writer validators or a named facade validator built on them. Ensure openWarp() rejects invalid writer IDs in its promise path, Warp cannot be constructed with an invalid writer, Warp.timeline() rejects invalid timeline names before invoking its opener, and Timeline cannot be constructed with invalid name/writer identities.

Mitigation prompt for an implementation agent:

In the v19 openWarp facade, align public identity validation with the runtime ref-layout model. Add facade-level validation for writer IDs and timeline names using the existing validateWriterId and validateGraphName domain validators, or a named facade validator that delegates to them while preserving public error intent. Ensure openWarp() rejects invalid non-empty writer IDs in its promise path, Warp cannot be constructed with an invalid writer identity, Warp.timeline() rejects invalid timeline names before invoking the opener, and Timeline cannot be constructed with invalid name/writer identities. Add deterministic regression tests for invalid-but-nonempty writer/timeline names such as whitespace-containing IDs, path traversal, slash-containing writer IDs, and overlong writer IDs.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@flyingrobots

Copy link
Copy Markdown
Member Author

Code Lawyer Activity Summary

# Source Severity File Commit Outcome
1 Self P2 src/domain/api/assertIdentity.ts, src/domain/api/openWarp.ts, src/domain/api/Warp.ts, src/domain/api/Timeline.ts, test/unit/domain/WarpFacade.test.ts 237bc8bd Public facade identity validation now delegates to canonical validateWriterId() and validateGraphName() after preserving the existing non-empty facade error path. openWarp(), Warp, Warp.timeline(), and Timeline now reject invalid non-empty identities at the public facade boundary.

Verification:

Gate Result
RED regression npx vitest run test/unit/domain/WarpFacade.test.ts failed before the fix because openWarp({ writer: 'agent 1' }) resolved.
Focused suite npx vitest run test/unit/domain/WarpFacade.test.ts passed after the fix: 9 tests passed.
Lint npm run lint passed.
Typecheck npm run typecheck passed.
Whitespace git diff --check origin/main...HEAD passed.
Pre-push IRONCLAD M9 passed during git push, including static gates and stable unit-test shards.
GitHub CI all current PR checks passed on 237bc8bdf19dbde3995eafb07a02490b0219b158.
Review threads GraphQL reports 7 total review threads, 0 unresolved.

Self-audit after the fix: no additional Code Lawyer findings.

Merge gate remains locked on human approvals: current latest-review count is 0 approvals, 0 changes requested.

@flyingrobots flyingrobots merged commit 65963d7 into main Jul 8, 2026
17 checks passed
@flyingrobots flyingrobots deleted the v19-openwarp-timeline-facade branch July 8, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v19: introduce openWarp and Timeline facade

1 participant