Rewrite README around witnessed causal history#20
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds witnessed-causality schema changes, extends warp init to record/run Wesley runner/commandSet (native Rust supported), implements warpspace lock/verify/sync/doctor + CLI, adds tests, and updates docs and warpspace example manifests. ChangesDoctrine & Boundary Reframing
Schema & Contract Changes
Warp init: Wesley runner & invocation
Warpspace tooling & CLI
Tests
Warpspaces artifacts & docs
Sequence Diagram(s)sequenceDiagram
participant User as CLI
participant Warp
participant Toolchain
participant Wesley
participant FS as Filesystem
User->>Warp: warp init (manifest)
Warp->>Toolchain: assemble toolchain (includes wesley.runner/commandSet)
Warp->>Wesley: buildWesleyInvocation(toolchain) -> run emit commands
Wesley-->>Warp: emit artifacts + install receipts
Warp->>FS: write .warpspace/ and warpspace.toml (wesley_runner/command_set)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 190-195: Make the external link formatting consistent in the
"Active sibling repositories" list by removing or adding backticks so all
entries use the same style; update the link label for `git-warp` to match `Echo`
and `AIΩN` (or vice versa) so that the entries Echo, git-warp, and AIΩN use
identical markdown link label formatting.
- Around line 26-30: The README's admonition block uses GitHub-flavored Markdown
(`> [!WARNING]`) which may not render elsewhere; update the admonition by either
marking it as "GitHub-flavored Markdown" and keeping the existing block or
adding a plain-text fallback (e.g., a simple "WARNING: Continuum is under active
research..." paragraph immediately before or after the admonition) so non-GitHub
renderers still convey the status; locate the existing admonition in the README
and modify that block accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/design/0027-witnessed-causal-suffix-sync/README.md (1)
108-112:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
Pluralhere, notbraid.This step still says
braid, but the shared outcome family below now exportsPlural. Keeping both terms in the same admission path makes the doc read like there are two different result kinds.🤖 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 `@docs/design/0027-witnessed-causal-suffix-sync/README.md` around lines 108 - 112, In the admission steps list, replace the term "braid" with the exported outcome name "Plural" so step 4 reads "... admit, stage, Plural, conflict, or obstruct"; ensure the terminology matches the "shared outcome family" export ("Plural") to avoid implying two distinct result kinds and update any nearby mentions of "braid" in the same admission path to "Plural" for consistency.
🤖 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 `@apps/warp/README.md`:
- Around line 37-41: Clarify the two Wesley runner modes in the README by
updating the bulleted point that mentions "legacy Node Wesley entrypoint and the
native Rust Wesley binary runner" to include brief parenthetical examples—e.g.,
"legacy Node Wesley entrypoint (.mjs) — Node runtime" and "native Rust Wesley
binary runner — direct Rust binary execution"—and ensure the existing references
to staging under `.warpspace/packages/wesley/` and invocation via the real
`warpspace.toml` remain unchanged.
In `@apps/warp/src/init.mjs`:
- Around line 857-865: initWarp() is still validating/installing
manifest.toolchain.node even when commandSet === 'native-rust', causing
Rust-only runs to fail on Node; update the startup flow so that when commandSet
equals 'native-rust' (the branch that calls invokeRustNativeWesleyForFamily),
initWarp() bypasses Node runtime validation/installation for
manifest.toolchain.node (i.e., short-circuit the Node setup logic before any
node version checks or installs run), ensuring the native-rust path does not
require a Node runtime.
- Around line 1156-1175: manifestWesleyRunner and manifestWesleyCommandSet
currently choose runner/command-set in a different order than installWesleyTool,
causing warpspace.toml to disagree with staged installs; update
manifestWesleyRunner to prefer wesleySpec.install.runner over wesleySpec.runner
(match installWesleyTool’s precedence) and preserve the same
local-sibling-binary coercion behavior used at install time so that when
installWesleyTool coerces a runner/command-set pair (e.g., converting
local-sibling-binary to native-binary or forcing legacy-node),
manifestWesleyCommandSet uses the resulting normalized runner to pick
'native-rust' vs 'legacy-node' (i.e., call manifestWesleyRunner(wesleySpec)
after using install.* precedence and base command-set decision on that
normalized value).
In `@apps/warp/test/warp-init.test.mjs`:
- Around line 71-73: The current wesley object sets package/version with nullish
coalescing then spreads ...wesleyToolchain which can reassign those fields;
change the construction so defaults come before the spread (set package:
'@wesley/host-node' and version: '0.1.0' first), then spread ...wesleyToolchain,
and finally set install: wesleyInstall so that provided values in
wesleyToolchain override the defaults and install remains correct; update the
object where wesleyToolchain is used (referencing the wesley property and
wesleyInstall symbol) accordingly.
- Around line 641-647: The test assertion for invocations[0].args currently only
checks the first five elements and omits the actual output path; update the
assertion that uses assert.deepEqual to validate the full args array (including
the '--out' value) just like the earlier Wesley test does, so locate the
invocations variable and its args array (invocations[0].args) and replace or
extend the expected array to include the full output path string used by the
sibling test to ensure the complete command-line arguments are asserted.
In `@schemas/continuum-runtime-boundary-family.graphql`:
- Around line 291-299: The ImportOutcome schema stores bundle identity twice
(fields bundleDigest and bundle) but lacks an invariant tying them together; add
a state invariant for ImportOutcome that enforces
ImportOutcome.bundle.bundleDigest == ImportOutcome.bundleDigest (i.e., reference
the embedded CausalSuffixBundle's bundleDigest field equals the top-level
bundleDigest) so the schema proves the two values must agree; update the
ImportOutcome type's invariants/annotations (where other `@wes_stateField`
invariants live) to include this equality check referencing ImportOutcome,
bundleDigest, and bundle (and CausalSuffixBundle.bundleDigest).
---
Outside diff comments:
In `@docs/design/0027-witnessed-causal-suffix-sync/README.md`:
- Around line 108-112: In the admission steps list, replace the term "braid"
with the exported outcome name "Plural" so step 4 reads "... admit, stage,
Plural, conflict, or obstruct"; ensure the terminology matches the "shared
outcome family" export ("Plural") to avoid implying two distinct result kinds
and update any nearby mentions of "braid" in the same admission path to "Plural"
for consistency.
🪄 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: c7a0894f-c6a7-4377-ab44-fe417cdef80d
📒 Files selected for processing (14)
README.mdapps/warp/README.mdapps/warp/src/init.mjsapps/warp/test/warp-init.test.mjsdocs/contract-family-registry.mddocs/design/0025-warp-native-distribution-and-node-runtime-policy/README.mddocs/design/0027-witnessed-causal-suffix-sync/README.mddocs/design/0028-minimum-runtime-boundary-contract-family/README.mddocs/design/0029-cross-repo-contract-family-registry/README.mddocs/design/0029-cross-repo-contract-family-registry/backlog-origin.mddocs/method/backlog/asap/SOURCE_minimum-runtime-boundary-contract-family.mddocs/method/backlog/up-next/SOURCE_warp-managed-toolchain-install-and-wesley-handoff.mddocs/releases/demo/continuum-stack-release.jsonschemas/continuum-runtime-boundary-family.graphql
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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 `@apps/warp/README.md`:
- Around line 58-62: Update the README example so the warpspace lock command
does not overwrite the checked-in lockfile: change the example invocation of the
CLI command "warp warpspace lock docs/warpspaces/jedit-echo-dev.toml" to include
an explicit "--lock" path that points to a scratch/temp location (e.g. /tmp or a
docs/tmp file) and then update the subsequent "warp warpspace sync" and "warp
warpspace verify" examples to reference that same temporary lock file path
(using the same "--lock" value) so the committed
docs/warpspaces/jedit-echo-dev.lock.json is never overwritten when following the
README.
In `@apps/warp/src/cli.mjs`:
- Around line 92-97: The top-level runWarpspace handler currently calls
hasHelpFlag(argv) which inspects the entire argv (including the subcommand) and
causes subcommand-specific help to be swallowed; change the logic so
runWarpspace only prints the generic renderWarpspaceUsage() when no subcommand
is provided, and remove the hasHelpFlag(argv) check here — instead let each
subcommand branch (the blocks that test command === '<sub>') call
hasHelpFlag(rest) and print their own Usage/help and return 0 (reusing the
existing inline usage strings or helper functions) so flags like `warp warpspace
lock --help` reach the per-subcommand help handlers.
- Around line 99-188: The CLI currently prints full JS stack traces for user
input errors because the top-level catch in the warpspace command block
indiscriminately prints error.stack; update the error handling so that errors
originating from argument parsing or usage checks (thrown by parseWarpspaceArgs
and requireValue, and similar ones from runInit) are treated as usage errors:
detect the sentinel (e.g. error.code === 'EUSAGE' or a custom UsageError type)
and in that branch print only error.message plus the appropriate usage text (use
renderWarpspaceUsage() or the per-command usage strings) to stderr and exit 1,
while letting other unexpected exceptions continue to print stack traces as
before. Ensure you reference parseWarpspaceArgs, requireValue, runInit,
renderWarpspaceUsage, and the top-level catch when applying this change.
In `@apps/warp/src/warpspace.mjs`:
- Around line 79-80: Lock entries' repo.path may escape the intended warpspace
root via path traversal; update the code that computes checkoutPath (where
path.resolve(resolvedRoot, repo.path ?? repo.name) is used) to validate and
reject any repo.path that would resolve outside resolvedRoot. After reading
entries in readLock() and before using them in verifyWarpspace() and
syncWarpspace(), compute the absolute resolved path and ensure it starts with
resolvedRoot (or is equal) and that repo.path is not absolute or contains ..
segments; if the check fails, throw or reject the lock entry so
verifyWarpspace() and syncWarpspace() never inspect or mutate outside the root.
Ensure the same validation is applied at all usages noted (the checkoutPath
calculation and related blocks).
- Around line 182-194: syncWarpspace() currently only calls
assertCleanWorktree(...) for existing checkouts and then runs
runGitChecked(['fetch','origin',...]) using whatever origin is configured;
update syncWarpspace() to validate and reconcile the configured origin before
fetching by using git remote get-url origin (or equivalent helper) and comparing
it to the expected repo.url, and if they differ either update the remote with
git remote set-url origin <expected> or fail early with a clear error; ensure
this check happens after assertCleanWorktree(...) and before the
runGitChecked(['fetch', 'origin', ...]) call so verifyWarpspace() won't later
report origin-mismatch.
- Around line 169-181: The clone fails for nested repo.path because parent
directories of checkoutPath aren't created; before calling
runGitChecked(['clone', ...]) ensure the parent directory exists by creating
path.dirname(checkoutPath) recursively (e.g., use fs.promises.mkdir(..., {
recursive: true }) or a mkdirp helper) so that checkoutPath can be created by
git; update the block around checkoutPath/checkout existence checks (referencing
checkoutPath, resolvedRoot, and runGitChecked) to create the intermediate
directories when !existed and before invoking runGitChecked.
In `@apps/warp/test/warp-warpspace.test.mjs`:
- Around line 70-89: The test assumes stable ordering; make assertions
order-independent: for syncResult.repos (from syncResult in this test) compare
sets instead of array order by mapping to names/cloned pairs and asserting both
expected entries are present (or sort both lists before deepEqual), and for
verifyWarpspace/dirtyResult, do not rely on issues[0] — assert that
dirtyResult.ok is false and that some entry in dirtyResult.issues has code ===
'dirty-worktree' (e.g., use Array.prototype.some or find to assert existence);
do the same for the CLI JSON output (inspect parsed.stdout.ok and ensure the
specific issue code exists) so the test no longer depends on enumeration order.
- Around line 95-111: The test uses git(['init', '-b', 'main']) in createGitRepo
which requires Git ≥2.28; update createGitRepo to avoid the -b flag by calling
git(['init'], repoPath) and then git(['symbolic-ref', 'HEAD',
'refs/heads/main'], repoPath) (or alternatively detect git version and fallback
to the symbolic-ref sequence) so the repo is initialized with a main branch on
older Git versions; locate the git(['init', '-b', 'main'], repoPath) call in
createGitRepo and replace it with the two-step init + symbolic-ref (or add a
version-check + conditional) to ensure compatibility.
In `@docs/warpspaces/jedit-echo-dev.toml`:
- Around line 6-24: The manifest uses SSH-only git URLs (e.g., the git fields
under [repos.wesley], [repos.echo], [repos.jedit], [repos.warp-ttd]) which
requires SSH keys; change each git value from the "git@github.com:..." SSH form
to the HTTPS form "https://github.com/owner/repo.git" so anonymous CI and users
without SSH work, then regenerate the corresponding lock file
docs/warpspaces/jedit-echo-dev.lock.json and update the manifest sha256 to match
the new content; alternatively keep SSH but add a prominent note in
docs/warpspaces/README.md about the SSH key prerequisite (choose one approach
and apply consistently across the four repo entries).
- Around line 26-34: The TOML has inconsistent keys "wesley-core" vs
"wesley_core" that appear unused; pick one convention (prefer kebab-case
"wesley-core") and make the manifest consistent or remove both keys if they are
dead; update docs/warpspaces/jedit-echo-dev.toml to either (a) use a single key
"wesley-core = \"0.0.2\"" in both [crates] and [contracts.jedit-text], or (b)
delete both keys if they are not required, and add a short comment explaining
the choice if they are kept for downstream tooling; also scan for references
and, if these keys remain, add a note in the manifest and adjust
apps/warp/src/warpspace.mjs to normalize the key name if it must be consumed by
downstream code.
In `@docs/warpspaces/README.md`:
- Around line 10-13: Replace the inline code reference to jedit-echo-dev.toml
with a relative markdown link so the filename is clickable; locate the README
line mentioning `jedit-echo-dev.toml` and change it to a link like
[jedit-echo-dev.toml](./jedit-echo-dev.toml) (or simply
[jedit-echo-dev.toml](jedit-echo-dev.toml)) so readers can click through
directly.
🪄 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: acec1406-d55a-495d-aa7d-e908102681f4
📒 Files selected for processing (9)
CHANGELOG.mdapps/warp/README.mdapps/warp/src/cli.mjsapps/warp/src/warpspace.mjsapps/warp/test/warp-warpspace.test.mjsdocs/design/0024-warp-cli-and-warpspace-toml/README.mddocs/warpspaces/README.mddocs/warpspaces/jedit-echo-dev.lock.jsondocs/warpspaces/jedit-echo-dev.toml
Resolved in the commits above. |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
apps/warp/src/init.mjs (1)
1158-1181: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
manifestWesleyRunnerignoresinstall.sourceprecedence, allowing operator mistakes to cascade through validation and installation.The bug is real and systematic: when a manifest declares
install.source = "local-sibling-binary"and setsinstall.runner = "node-entrypoint"(operator error),installWesleyToolat line 571 hard-codesrunner = "native-binary"and correctly ignores the declared field. ButmanifestWesleyRunnerhere reverses that precedence—it returns whateverinstall.runnerclaims because the explicit-runner branch (lines 1159–1162) executes before the source check (lines 1163–1165). This causesmanifestRequiresNodeto returntruefor what will actually install as a native-binary, forcingvalidateManifest(line 240) to demandtoolchain.nodeand triggering a fullinstallNodeRuntime()call (lines 399–405). The result: a Node toolchain is materialized, locked intowarpspace.toml, and never invoked—wasteful and misleading.Same bug applies to
local-sibling-entrypoint(line 523):installWesleyToolhard-codesrunner = "node-entrypoint", butmanifestWesleyRunnerwill respect an erroneous explicitinstall.runner = "native-binary"if declared, causing the inverse problem.Option A — make source authoritative in
manifestWesleyRunner(cheapest, mirrorsinstallWesleyToolprecedence):Reorder to check both
local-sibling-binaryandlocal-sibling-entrypointsources before explicit runner. The proposed diff is incomplete—it shows only the binary case; add the entrypoint case too:♻️ Corrected Option A diff
function manifestWesleyRunner(wesleySpec) { + if (wesleySpec?.install?.source === 'local-sibling-binary') { + return 'native-binary'; + } + if (wesleySpec?.install?.source === 'local-sibling-entrypoint') { + return 'node-entrypoint'; + } const explicitRunner = wesleySpec?.install?.runner ?? wesleySpec?.runner; if (explicitRunner != null) { return explicitRunner; } - if (wesleySpec?.install?.source === 'local-sibling-binary') { - return 'native-binary'; - } return 'node-entrypoint'; }Option B — validate
install.runnerconsistency at manifest parse time:Add a pre-install check in
validateManifestthat rejects explicitinstall.runnerwhen it conflicts withinstall.sourcefor thelocal-sibling-*cases, surfacing the operator error instead of silently coercing.🤖 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 `@apps/warp/src/init.mjs` around lines 1158 - 1181, manifestWesleyRunner currently returns an explicit install.runner before honoring install.source, causing mismatches with installWesleyTool; change manifestWesleyRunner to check install.source for 'local-sibling-binary' and 'local-sibling-entrypoint' first (returning 'native-binary' and 'node-entrypoint' respectively), then fall back to explicit install.runner and lastly to 'node-entrypoint' default so manifestWesleyCommandSet and manifestRequiresNode (which call manifestWesleyRunner) reflect the same source-first precedence as installWesleyTool; alternatively, add a validateManifest check that rejects conflicting install.runner when install.source is a local-sibling variant.
🤖 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 `@apps/warp/src/cli.mjs`:
- Around line 295-326: The parser parseWarpspaceArgs currently accepts all
warpspace flags for every subcommand; change its signature to accept a second
argument like allowedFlags (a Set or array of flag names) and, inside the token
switch/handling, validate each encountered flag against allowedFlags—if a flag
is not in allowedFlags throw a UsageError—so each subcommand calls
parseWarpspaceArgs with only the flags it supports (or pass the full set for
backward compatibility where needed); update callers of parseWarpspaceArgs to
supply the appropriate allowed flag set per subcommand (or a shared full-set
helper) to limit the advertised surface to what each command actually honors.
- Around line 328-334: The requireValue function currently only checks
value.startsWith('--') so short flags like '-h' slip through; update the guard
in requireValue to detect any flag prefix (e.g., value.startsWith('-') or a
regex like /^-/) and throw UsageError when the next token looks like a short or
long flag; if you need to allow values that begin with '-' instead, additionally
handle an explicit '--' separator or check against the known set of short flags
before treating it as a value.
In `@apps/warp/src/init.mjs`:
- Around line 948-998: The design spec
0025-warp-native-distribution-and-node-runtime-policy must explicitly document
the projection constraint enforced by invokeRustNativeWesleyForFamily: add a
clear statement in the Wesley command surfaces section that the native-rust
command surface only supports the 'typescript' and 'rust' projections and will
reject others (e.g., 'zod', 'echo-ir', 'warp-ttd'); also note that non-supported
projections must be handled via the legacy-node command surface and update any
examples or tables to reflect this constraint so the code behavior is
documented.
In `@apps/warp/src/warpspace.mjs`:
- Around line 437-481: The git-ref resolution prefers peeled tag
(`refs/tags/X^{}`) over annotated tag (`refs/tags/X`) over branch
(`refs/heads/X`) over exact ref, so a name that exists both as a tag and a
branch (e.g., "main") will resolve to the tag; update the public-facing
documentation and add an inline code comment above the resolveGitRef function
(and/or near the candidates/selected logic) that explicitly states this
precedence policy and why peeled-tag wins, referencing the resolveGitRef
function and the candidates/selected variables so future readers/operators
understand the designed behavior.
- Around line 341-369: parseTomlValue currently accepts unquoted barewords by
falling through to return value; update parseTomlValue (and any callers like
splitTomlArray usage) to reject unquoted barewords instead of treating them as
strings: after handling quoted strings, arrays, booleans and integers, throw a
descriptive Error(`${manifestPath}:${lineNumber}: unquoted bareword '${value}'
is not allowed`) for any remaining token so only true/false, integers, quoted
strings, and bracketed arrays are accepted.
In `@apps/warp/test/warp-init.test.mjs`:
- Around line 643-657: Add assertions that verify the rendered
.warpspace/warpspace.toml contains the expected toolchain keys for Wesley so
this test mirrors the sibling local-sibling-binary test; after the
install-receipt checks use the same projectDir + readFile + path.join approach
to read '.warpspace/warpspace.toml' and assert it contains wesley_runner =
"native-binary" and wesley_command_set = "native-rust" (use the same assertion
style as the other test that checks these keys).
In `@README.md`:
- Around line 26-33: The README contains duplicated status messaging: a
plain-text paragraph starting with "WARNING: Continuum is under active research
and development..." and the subsequent GFM admonition block beginning with ">
[!WARNING]" containing "**Status: Nascent / Experimental**"; remove the
plain-text fallback paragraph and keep only the GFM admonition block (i.e.,
delete the lines containing the standalone "WARNING: Continuum..." paragraph and
leave the "> [!WARNING]" admonition intact) so the warning is shown once on
GitHub-rendered repos.
In `@schemas/continuum-runtime-boundary-family.graphql`:
- Around line 295-298: ImportOutcome stores target identity in multiple fields
(targetWorldlineId, targetBasis, and admission.targetBasis) but only the
bundle-level fields are constrained; add target-coherence invariants to ensure
these three locations always match. Update the ImportOutcome GraphQL type (and
the repeated block at lines ~353-360) to include an invariant that verifies
targetWorldlineId === bundle.targetWorldlineId && targetBasis ===
bundle.targetBasis && admission.targetBasis === bundle.targetBasis (or
equivalent checks referencing the bundle and admission), referencing the fields
targetWorldlineId, targetBasis, admission.targetBasis and any bundle.* fields
used in the type so malformed records cannot claim differing identities.
---
Duplicate comments:
In `@apps/warp/src/init.mjs`:
- Around line 1158-1181: manifestWesleyRunner currently returns an explicit
install.runner before honoring install.source, causing mismatches with
installWesleyTool; change manifestWesleyRunner to check install.source for
'local-sibling-binary' and 'local-sibling-entrypoint' first (returning
'native-binary' and 'node-entrypoint' respectively), then fall back to explicit
install.runner and lastly to 'node-entrypoint' default so
manifestWesleyCommandSet and manifestRequiresNode (which call
manifestWesleyRunner) reflect the same source-first precedence as
installWesleyTool; alternatively, add a validateManifest check that rejects
conflicting install.runner when install.source is a local-sibling variant.
🪄 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: cb69c358-675e-47f0-a6b9-ee27edef2677
📒 Files selected for processing (11)
README.mdapps/warp/README.mdapps/warp/src/cli.mjsapps/warp/src/init.mjsapps/warp/src/warpspace.mjsapps/warp/test/warp-init.test.mjsapps/warp/test/warp-warpspace.test.mjsdocs/warpspaces/README.mddocs/warpspaces/jedit-echo-dev.lock.jsondocs/warpspaces/jedit-echo-dev.tomlschemas/continuum-runtime-boundary-family.graphql
Resolved review threads are now marked complete, the branch is pushed, and the second feedback pass is covered by commit |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/design/0025-warp-native-distribution-and-node-runtime-policy/README.md (1)
158-167:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
prefer-systemis undefined in the policy surface.This section says the allowed runtime sources are
managedandsystem, then immediately recommendsprefer-systemfor local-dev profiles. That leaves the manifest/lockfile contract ambiguous in the very place this doc is trying to freeze it. Defineprefer-systemalongside the allowed sources, or remove it here.🤖 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 `@docs/design/0025-warp-native-distribution-and-node-runtime-policy/README.md` around lines 158 - 167, The doc references a non‑existent runtime source "prefer-system" in the "Recommended profile defaults" while the "Allowed runtime sources" list only defines `managed` and `system`; either add a clear definition for `prefer-system` to the allowed runtime sources (describe its semantics and how it differs from `system` and `managed`) or remove `prefer-system` from the recommendation to eliminate the ambiguity; update the "Allowed runtime sources" block to include `prefer-system` with its semantics if you choose to keep it, and make sure the `Recommended profile defaults` entry references only the defined symbols `managed`, `system`, and `prefer-system`.apps/warp/src/cli.mjs (1)
259-280:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
-qis still unreachable inwarp init.
parseInitArgs()classifies every token that doesn't start with--as positional, so the advertised short quiet flag becomes a second project directory and trips the arity check instead of settingoptions.quiet.Suggested fix
- if (!token.startsWith('--')) { + if (!token.startsWith('-')) { positionals.push(token); continue; }🤖 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 `@apps/warp/src/cli.mjs` around lines 259 - 280, The parser treats any token not starting with "--" as positional, so the short flag "-q" is mistakenly classified as a positional and never reaches the switch; update parseInitArgs's token handling so single-dash flags are recognized: before pushing to positionals, check for tokens that start with "-" (e.g., token === "-q") and handle them in the switch (or explicitly check for "-q" and set options.quiet), ensuring you still use requireValue(argv, ++index, token, usage) for flags that need values (like "-p" if added) and keep the existing cases (--profile, --manifest, --authority-root, --json, --quiet) working with token, positionals, options, argv, index, requireValue, and usage.
🤖 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 `@apps/warp/src/warpspace.mjs`:
- Around line 438-443: The code path that immediately accepts 40-hex revs (the
regex block in warpspace.mjs) must verify the SHA actually exists in the target
repository before returning a resolved literal-sha; update the logic in the
function that contains that regex (used by lockWarpspace()) to call the repo/git
backend to check the object/commit exists (e.g., via an existing helper that
queries the remote or git cat-file -e) and only return { resolved:
rev.toLowerCase(), resolution: 'literal-sha' } if that existence check succeeds;
if the SHA does not exist, throw/return an error so lockWarpspace() fails
instead of writing a stale/typo lock.
- Around line 592-601: The helper defaultRunCommand currently returns
status/stdout/stderr but ignores spawnSync's result.error; update
defaultRunCommand to surface execution errors by checking result.error and
either throwing it or appending result.error.message to the returned stderr
(e.g., include result.error.message when building the returned object) so
callers see failures like missing git; refer to the defaultRunCommand function
and the spawnSync result.error field when making the change.
In `@schemas/continuum-runtime-boundary-family.graphql`:
- Around line 349-351: The invariant bundle_links_source_shell incorrectly ties
the target frontier to the source shell, causing cross-worldline imports to
collapse into self-imports; update the expr in bundle_links_source_shell (which
currently uses CausalSuffixBundle, sourceSuffix, and targetFrontier) so
targetFrontier.worldlineId is not equated to sourceSuffix.sourceWorldlineId but
remains tied to the target side (e.g., compare targetFrontier.worldlineId to the
appropriate target suffix/worldline id or ImportOutcome.targetWorldlineId) so
that ImportOutcome.targetWorldlineId is free to differ from
b.sourceSuffix.sourceWorldlineId.
---
Outside diff comments:
In `@apps/warp/src/cli.mjs`:
- Around line 259-280: The parser treats any token not starting with "--" as
positional, so the short flag "-q" is mistakenly classified as a positional and
never reaches the switch; update parseInitArgs's token handling so single-dash
flags are recognized: before pushing to positionals, check for tokens that start
with "-" (e.g., token === "-q") and handle them in the switch (or explicitly
check for "-q" and set options.quiet), ensuring you still use requireValue(argv,
++index, token, usage) for flags that need values (like "-p" if added) and keep
the existing cases (--profile, --manifest, --authority-root, --json, --quiet)
working with token, positionals, options, argv, index, requireValue, and usage.
In `@docs/design/0025-warp-native-distribution-and-node-runtime-policy/README.md`:
- Around line 158-167: The doc references a non‑existent runtime source
"prefer-system" in the "Recommended profile defaults" while the "Allowed runtime
sources" list only defines `managed` and `system`; either add a clear definition
for `prefer-system` to the allowed runtime sources (describe its semantics and
how it differs from `system` and `managed`) or remove `prefer-system` from the
recommendation to eliminate the ambiguity; update the "Allowed runtime sources"
block to include `prefer-system` with its semantics if you choose to keep it,
and make sure the `Recommended profile defaults` entry references only the
defined symbols `managed`, `system`, and `prefer-system`.
🪄 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: c1ad2f25-587b-42f8-b0be-6d5247cca02a
📒 Files selected for processing (7)
README.mdapps/warp/src/cli.mjsapps/warp/src/warpspace.mjsapps/warp/test/warp-init.test.mjsapps/warp/test/warp-warpspace.test.mjsdocs/design/0025-warp-native-distribution-and-node-runtime-policy/README.mdschemas/continuum-runtime-boundary-family.graphql
|
Summary
Rewrites the Continuum README around the "there is no graph" doctrine.
This PR:
git-warpWhy
Continuum should introduce itself through the same ontology it enforces: the shared boundary is witnessed causal history, not a privileged graph stored by one runtime. Graph-like values are useful readings over that history, but they are not substrate truth.
Validation
npx --yes markdownlint-cli -- README.mdgit diff --check origin/main...HEAD