feat(SUP-463): add interactive bootstrap script and define new user install flow - #12
Conversation
- bootstrap.js: interactive + non-interactive (--yes / env vars) installer - checks Node.js ≥18 and openclaw CLI deps - prompts for ASSISTANT_NAME, USER_NAME, COACH_NAME, TIMEZONE, VAULT_PATH, WORKSPACE_PATH - creates vault/Notes, vault/Journal, vault/Tags, workspace/memory dirs - renders AGENTS.md, BOOTSTRAP.md, HEARTBEAT.md from templates/ - generates MEMORY.md and memory/YYYY-MM-DD.md - writes jarvos.config.json - built-in 5-step smoke test (no unreplaced placeholders check) - bootstrap.sh: thin shell wrapper (Node version guard + exec delegate) - package.json: enables npx jarvos-bootstrap / npx jarvos-init - tests/smoke-test.sh: 12-check non-interactive regression suite (all passing) - README.md: real prerequisites + step-by-step Quick Start with npm test Closes SUP-488
|
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:
📝 WalkthroughWalkthroughAdds a jarvOS bootstrap flow: a new Node.js CLI ( Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Shell as bootstrap.sh
participant CLI as bootstrap.js
participant FS as File System
participant Templates as Templates
User->>Shell: run script / CLI command
Shell->>Shell: verify Node.js ≥ 18
Shell->>CLI: exec node bootstrap.js
CLI->>CLI: check dependencies & detect env
alt Interactive
CLI->>User: prompt for config
User->>CLI: provide inputs
else Non-interactive
CLI->>CLI: use env / --yes values
end
CLI->>FS: createDirectories(config) (vault, workspace, workspace/memory)
CLI->>Templates: read template files
Templates-->>CLI: template content
CLI->>CLI: renderTemplate() substitute {{KEY}}
CLI->>FS: write overlays (`AGENTS.md`, `BOOTSTRAP.md`, `HEARTBEAT.md`, `MEMORY.md`, `ONTOLOGY.md`, `jarvos.config.json`)
CLI->>CLI: smokeTest()
CLI->>FS: verify files exist & no unreplaced {{...}}
CLI->>User: report PASS/FAIL
alt success
CLI->>User: exit 0
else failure
CLI->>User: exit 1
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ed1c62e38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| hdr('5/5 Smoke test'); | ||
|
|
||
| const ws = config.WORKSPACE_PATH; | ||
| const requiredFiles = ['AGENTS.md', 'BOOTSTRAP.md', 'HEARTBEAT.md', 'MEMORY.md']; |
There was a problem hiding this comment.
The bootstrap success criteria currently only require AGENTS.md, BOOTSTRAP.md, HEARTBEAT.md, and MEMORY.md, so the installer can report success even when key onboarding files are missing in a fresh workspace. In this same flow, users are told to edit ONTOLOGY.md, and BOOTSTRAP.md instructs reading SOUL.md/USER.md; because those files are not part of the generated/required set, new installs end up in a broken first-run state unless users manually discover and copy templates.
Useful? React with 👍 / 👎.
| name: 'OpenClaw CLI (openclaw)', | ||
| test: () => { | ||
| const r = spawnSync('openclaw', ['--version'], { encoding: 'utf8' }); | ||
| return r.status === 0; |
There was a problem hiding this comment.
This check makes openclaw a hard prerequisite and exits before any file generation, which breaks the provided non-interactive smoke test (npm test) on systems that have Node but not a globally installed OpenClaw CLI. Since bootstrap is primarily generating local files, this hard gate makes CI/local verification brittle and blocks setup scenarios that should otherwise be testable.
Useful? React with 👍 / 👎.
| function generateOverlays(config) { | ||
| hdr('4/5 Generating starter overlay files'); | ||
|
|
||
| const today = new Date().toISOString().slice(0, 10); |
There was a problem hiding this comment.
Using toISOString() here forces UTC day boundaries, so users outside UTC can get memory/YYYY-MM-DD.md created for the wrong calendar day around midnight local time. That causes journal/memory workflows keyed by local date to split or miss entries even though the user's configured timezone says otherwise.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/smoke-test.sh (1)
19-27: Cover the public non-interactive path and the rerun-safe contract.This only exercises a single
--yesinvocation, so it will not catch drift between the documented non-interactive entrypoint and the internal shortcut, or regressions in idempotence. Running bootstrap twice against the same temp workspace would make this smoke test match the feature claims much better.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/smoke-test.sh` around lines 19 - 27, Modify the smoke test to run the non-interactive bootstrap invocation twice to verify the public non-interactive path and rerun-safe contract: invoke the same command using the existing env vars (JARVOS_YES, JARVOS_ASSISTANT_NAME, JARVOS_USER_NAME, JARVOS_COACH_NAME, JARVOS_VAULT_PATH, JARVOS_WORKSPACE_PATH) against node "$BOOTSTRAP" --yes, then immediately run the exact same invocation a second time and ensure both runs exit successfully (preserve set -e or explicit exit checks) so we catch drift between the documented entrypoint and the internal shortcut and confirm idempotence.package.json (1)
12-16: Explicitly declare runtime assets in the npm package manifest.
bootstrap.jsloadstemplates/*from disk via__dirname(line 213), but the published tarball lacks an explicitfilesfield. While templates are currently included due to npm's default behavior, relying on implicit inclusion is fragile. Adding afilesfield would:
- Make runtime dependencies explicit and self-documenting
- Reduce the published package size by excluding unrelated test, example, and documentation files
- Protect against accidental omission if ignore rules change
📦 Suggested manifest hardening
"main": "bootstrap.js", + "files": [ + "bootstrap.js", + "templates/" + ], "bin": {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 12 - 16, Add an explicit "files" array to package.json to ensure runtime assets used by bootstrap.js (which loads templates via __dirname and reads templates/*) are included in the published package; update package.json to list "bootstrap.js" and the templates directory (e.g., "templates/") and any other runtime assets (binaries referenced under "bin") so tests/examples/docs are excluded and the tarball remains minimal and deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bootstrap.js`:
- Around line 47-76: The OpenClaw CLI check in the checks array currently runs
spawnSync('openclaw', ...) and makes bootstrap fail; change it so OpenClaw is
not a hard prerequisite by moving its entry out of checks into the optionals
array (or converting its object into an optional-style entry) and/or replace its
boolean test to only warn rather than return false on missing CLI; specifically
update the object with name 'OpenClaw CLI (openclaw)' that calls spawnSync and
the optionals array so the spawnSync('openclaw', ['--version'], { encoding:
'utf8' }) invocation no longer blocks bootstrap and instead reports as optional.
Ensure you keep the existing hint text but do not let its test cause overall
failure in the checks workflow.
- Around line 134-146: The non-interactive detection (isYes) currently checks
for '--yes', '-y', and JARVOS_YES but misses the advertised '--non-interactive'
flag; update the condition that defines isYes (the process.argv check near the
top of the bootstrap flow) to also treat '--non-interactive' (and any equivalent
long-form like '--no-interactive' if used elsewhere) as true so the
nonInteractiveConfig() branch is taken and the process returns without
prompting; modify the declaration that assigns isYes to include
process.argv.includes('--non-interactive') and ensure any callers relying on
isYes (the early return block that logs ASSISTANT_NAME, USER_NAME, etc.) will
run for that flag.
- Around line 223-239: The overlays array (const overlays) is missing entries
for USER.md and ONTOLOGY.md, causing bootstrap to finish without creating those
files; update the overlays array to include objects for
TEMPLATE_DIR/'USER-template.md' -> dest ws/'USER.md' and
TEMPLATE_DIR/'ONTOLOGY-template.md' -> dest ws/'ONTOLOGY.md' (with appropriate
label strings), and make the same additions wherever the overlays set is
constructed in the other block around the 260-310 region so the first-run
onboarding files are created; refer to the existing overlay objects (e.g.,
AGENTS-template.md, BOOTSTRAP-template.md, HEARTBEAT-template.md) as templates
for structure, keys (template, dest, label), and placement.
In `@README.md`:
- Around line 83-84: Update the README.md sentence about `npm test` to say it
runs the repository's smoke test rather than re-verifying the user's install:
change the line that currently reads "`bootstrap.js` generates ... Run `npm
test` at any time to re-verify the setup." to clarify that `npm test` executes
the repo smoke test (implemented by `tests/smoke-test.sh`) which boots a
temporary workspace and does not validate the user's local installation or the
`npx jarvos-bootstrap` path (which does not include that script); mention
`tests/smoke-test.sh` by name so readers know which script is run.
---
Nitpick comments:
In `@package.json`:
- Around line 12-16: Add an explicit "files" array to package.json to ensure
runtime assets used by bootstrap.js (which loads templates via __dirname and
reads templates/*) are included in the published package; update package.json to
list "bootstrap.js" and the templates directory (e.g., "templates/") and any
other runtime assets (binaries referenced under "bin") so tests/examples/docs
are excluded and the tarball remains minimal and deterministic.
In `@tests/smoke-test.sh`:
- Around line 19-27: Modify the smoke test to run the non-interactive bootstrap
invocation twice to verify the public non-interactive path and rerun-safe
contract: invoke the same command using the existing env vars (JARVOS_YES,
JARVOS_ASSISTANT_NAME, JARVOS_USER_NAME, JARVOS_COACH_NAME, JARVOS_VAULT_PATH,
JARVOS_WORKSPACE_PATH) against node "$BOOTSTRAP" --yes, then immediately run the
exact same invocation a second time and ensure both runs exit successfully
(preserve set -e or explicit exit checks) so we catch drift between the
documented entrypoint and the internal shortcut and confirm idempotence.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 082ed19b-1a4a-4ecb-a3aa-be910b118aed
📒 Files selected for processing (5)
README.mdbootstrap.jsbootstrap.shpackage.jsontests/smoke-test.sh
🤖 Autopilot Autonomous DecisionAfter 7 finding(s) remained unresolved across multiple fix passes, the autopilot evaluated them against project context (claw-ontology + Paperclip Goal Bridge). P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. If any of these dismissals are incorrect, push a new commit or comment to reopen discussion. |
1 similar comment
🤖 Autopilot Autonomous DecisionAfter 7 finding(s) remained unresolved across multiple fix passes, the autopilot evaluated them against project context (claw-ontology + Paperclip Goal Bridge). P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. P3 — warning | Dismissed
Autopilot Decision: Dismissed Severity: warning Project context: Project: claw-ontology + Paperclip Goal Bridge | Phase: in progress · 1 queued | Goal: Stand up a standalone ontology system that drives Paperclip execution | KRs: Stand up a standalone ontology system that drives Paperclip execution This finding was evaluated against the project's milestones, goals, and constraints. The autopilot determined it does not require a code change at this time. If you disagree, push a new commit or reopen the discussion. If any of these dismissals are incorrect, push a new commit or comment to reopen discussion. |
- Demote openclaw CLI from hard prerequisite to optional warning so bootstrap can run (and CI can pass) on machines without a global openclaw install - Add --non-interactive flag as alias for --yes/-y so automation wired to that flag is not left hanging on stdin - Use local-timezone wall-clock date for daily memory file naming instead of UTC toISOString(), preventing wrong-day files around midnight - Add USER.md and ONTOLOGY.md to generated overlay set so first-run users have the files the next-steps instructions point them to - Expand smoke-test required-file list to include USER.md and ONTOLOGY.md - Clarify README that npm test runs a repo smoke test in a temp workspace, not a re-check of the user's personal install
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c6abfc8e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const requiredDirs = [ | ||
| path.join(config.VAULT_PATH, 'Notes'), | ||
| path.join(config.VAULT_PATH, 'Journal'), | ||
| path.join(ws, 'memory') |
There was a problem hiding this comment.
createDirectories treats <vault>/Tags as part of the required vault structure, but the final smokeTest omits it from requiredDirs, so bootstrap can still print “All checks passed” and exit 0 when Tags creation failed (for example, if a file already exists at that path or permissions block directory creation). This reports a successful install while leaving a partially broken vault layout.
Useful? React with 👍 / 👎.
…test dirs - bootstrap.js:64 — when an optional dependency (e.g. openclaw) is not found, now shows its hint text via info() so users know where to install - bootstrap.js:341 — smokeTest requiredDirs now includes <vault>/Tags to match the directory structure created by createDirectories(), preventing false-pass when Tags creation fails
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
bootstrap.js (2)
188-206: Directory creation error handling could be more explicit.When
mkdirSyncfails, the script logs an error but continues. This is acceptable sincesmokeTestvalidates the final state, but consider tracking failures to provide earlier feedback rather than waiting for the smoke test to fail.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bootstrap.js` around lines 188 - 206, The createDirectories function currently logs errors on mkdirSync failures but continues silently; change it to track failures and surface them immediately by collecting failed paths (from the dirs array) during the try/catch in createDirectories, and after the loop either throw a descriptive Error or return a falsy failure indicator so callers can react instead of waiting for smokeTest; update callers (if any) to handle the thrown error or returned status. Ensure you reference the createDirectories function, the dirs array, and the err(...) logging path when implementing the change so failures like "Failed to create <path>: <message>" are both logged and cause an early visible failure.
252-261: Consider standardizing template file naming convention.The templates use inconsistent naming:
AGENTS-template.md,BOOTSTRAP-template.md,HEARTBEAT-template.md(dash convention) vsUSER.template.md,ONTOLOGY.template.md,MEMORY.template.md,TOOLS.template.md(dot convention). All files exist and bootstrap.js references them correctly, so there's no functional issue. However, adopting a single naming convention across all templates would improve consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bootstrap.js` around lines 252 - 261, The template filenames are using mixed conventions (dash vs dot); standardize them and update references: choose one convention (e.g., replace USER.template.md / ONTOLOGY.template.md with USER-template.md / ONTOLOGY-template.md or vice versa), rename the actual files in the templates directory accordingly, and update all usages in bootstrap.js where TEMPLATE_DIR is combined with filenames (the objects with template: path.join(TEMPLATE_DIR, 'USER.template.md') and similar entries for ONTOLOGY, MEMORY, TOOLS and the dash-named AGENTS-template.md, BOOTSTRAP-template.md, HEARTBEAT-template.md) so every template reference and label remains correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bootstrap.js`:
- Around line 369-381: The placeholder validation loop currently only iterates
over ['AGENTS.md','BOOTSTRAP.md']; update that array to also include 'USER.md',
'ONTOLOGY.md', and 'HEARTBEAT.md' so the same check (using path.join(ws, f),
fs.readFileSync, content.match(/\{\{[A-Z_]+\}\}/g), warn(...) and
ok(...)/passed++) runs for those template-generated files as well (do not add
MEMORY.md since it is generated inline).
---
Nitpick comments:
In `@bootstrap.js`:
- Around line 188-206: The createDirectories function currently logs errors on
mkdirSync failures but continues silently; change it to track failures and
surface them immediately by collecting failed paths (from the dirs array) during
the try/catch in createDirectories, and after the loop either throw a
descriptive Error or return a falsy failure indicator so callers can react
instead of waiting for smokeTest; update callers (if any) to handle the thrown
error or returned status. Ensure you reference the createDirectories function,
the dirs array, and the err(...) logging path when implementing the change so
failures like "Failed to create <path>: <message>" are both logged and cause an
early visible failure.
- Around line 252-261: The template filenames are using mixed conventions (dash
vs dot); standardize them and update references: choose one convention (e.g.,
replace USER.template.md / ONTOLOGY.template.md with USER-template.md /
ONTOLOGY-template.md or vice versa), rename the actual files in the templates
directory accordingly, and update all usages in bootstrap.js where TEMPLATE_DIR
is combined with filenames (the objects with template: path.join(TEMPLATE_DIR,
'USER.template.md') and similar entries for ONTOLOGY, MEMORY, TOOLS and the
dash-named AGENTS-template.md, BOOTSTRAP-template.md, HEARTBEAT-template.md) so
every template reference and label remains correct.
🪄 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35c2e7b539
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| hdr('5/5 Smoke test'); | ||
|
|
||
| const ws = config.WORKSPACE_PATH; | ||
| const requiredFiles = ['AGENTS.md', 'BOOTSTRAP.md', 'HEARTBEAT.md', 'MEMORY.md', 'USER.md', 'ONTOLOGY.md']; |
There was a problem hiding this comment.
BOOTSTRAP.md instructs first-run flow to read SOUL.md, and the generated operating files (notably HEARTBEAT.md/AGENTS.md) depend on TOOLS.md as canonical policy input, but requiredFiles does not include either file. That means a fresh install can report success while core onboarding/runtime references are missing, leaving users with a partially broken workspace. Fresh evidence in this revision: USER.md/ONTOLOGY.md were added to the success list, but SOUL.md/TOOLS.md are still omitted.
Useful? React with 👍 / 👎.
…holder check to all generated files
- Add SOUL.md and TOOLS.md to the overlay generation array so all files
referenced by first-run onboarding (BOOTSTRAP.md, HEARTBEAT.md, AGENTS.md)
are created by bootstrap
- Add templates/SOUL.template.md with {{ASSISTANT_NAME}} and {{USER_NAME}}
substitution
- Expand smokeTest requiredFiles to include SOUL.md and TOOLS.md
- Expand placeholder validation loop from ['AGENTS.md','BOOTSTRAP.md'] to
all requiredFiles except MEMORY.md (which is generated inline, not from
a template)
- Update tests/smoke-test.sh to check and validate all 8 generated files
Addresses review threads at bootstrap.js:342 and bootstrap.js:381.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42716e44e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (remaining) { | ||
| warn(`${f} still has unreplaced placeholders: ${[...new Set(remaining)].join(', ')}`); | ||
| } else { |
There was a problem hiding this comment.
When a required file already exists (idempotent reruns), the installer skips regeneration; if that file still contains {{...}} tokens, this branch only logs a warning and leaves failed unchanged, so the script can print “All checks passed” and exit 0 with a broken workspace. I confirmed this by precreating AGENTS.md with {{USER_NAME}}: bootstrap warned but still returned success. Placeholder residue should increment failure (or otherwise make smokeTest return false) so success status is trustworthy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
bootstrap.js (1)
36-39:⚠️ Potential issue | 🟡 MinorHandle a bare
~from the prompts.
expandHome()only expands~/.... If a user enters just~for the vault or workspace, bootstrap will create a literal~directory under the current working directory instead of using the home directory.Minimal fix
function expandHome(p) { if (!p) return p; + if (p === '~') return os.homedir(); if (p.startsWith('~/')) return path.join(os.homedir(), p.slice(2)); return p; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bootstrap.js` around lines 36 - 39, The expandHome function fails to handle a bare "~" input; update expandHome to treat both "~" and strings starting with "~/" as home-directory references by checking for p === "~" or p.startsWith("~/") and returning path.join(os.homedir(), p === "~" ? "" : p.slice(2)); keep using the existing function name expandHome and the os.homedir()/path.join calls so other callers still work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bootstrap.js`:
- Around line 125-132: Add a "runtime" field to the config model and persistence
so the installer can select runtime instead of hard-coding OpenClaw: update the
defaults object (const defaults) to include a runtime property (e.g., runtime:
process.env.JARVOS_RUNTIME || '<default>'), ensure the interactive and
non-interactive config builders capture and validate this runtime value, write
it into jarvos.config.json when saving (same code path that writes the other
defaults), and use that runtime value to branch post-install messaging and copy
logic instead of the current OpenClaw-specific path; update any code paths
referenced around the defaults and the save/load functions to read the runtime
field and alter next-step instructions accordingly.
- Around line 352-390: The smoke check is too permissive: make
jarvos.config.json mandatory by adding 'jarvos.config.json' to the requiredFiles
array and make unreplaced template tokens fail the check by treating matches in
the placeholder-check loop as errors (use err(...) and increment failed instead
of warn(...)), ensuring you do not increment passed when placeholders remain;
keep the existing passed++ in the "no unreplaced placeholders" branch so only
clean files count as passing.
In `@templates/SOUL.template.md`:
- Around line 33-41: Update the "## Continuity" section to stop instructing
agents to autonomously evolve SOUL.md and instead direct them to rely on
generated memory files (MEMORY.md and memory/) for session continuity and to
submit any SOUL.md edits as proposals for human review; specifically, modify the
paragraph under the "## Continuity" header to reference "MEMORY.md" and the
"memory/" directory as the canonical runtime-native memory sources and add a
sentence that changes to "SOUL.md" must be gated behind human review and
proposed rather than applied automatically.
---
Duplicate comments:
In `@bootstrap.js`:
- Around line 36-39: The expandHome function fails to handle a bare "~" input;
update expandHome to treat both "~" and strings starting with "~/" as
home-directory references by checking for p === "~" or p.startsWith("~/") and
returning path.join(os.homedir(), p === "~" ? "" : p.slice(2)); keep using the
existing function name expandHome and the os.homedir()/path.join calls so other
callers still work.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c0197cd-8c3c-45af-8882-a69aa788c1bb
📒 Files selected for processing (3)
bootstrap.jstemplates/SOUL.template.mdtests/smoke-test.sh
✅ Files skipped from review due to trivial changes (1)
- tests/smoke-test.sh
- expandHome: handle bare '~' input (expand to homedir) - smokeTest: add jarvos.config.json to requiredFiles list - smokeTest: change unreplaced-placeholder detection from warn to err (fail the check) - package.json: add explicit files field for runtime assets (bootstrap.js, bootstrap.sh, templates/) - SOUL.template.md: reference MEMORY.md/memory/ for continuity; require human approval for SOUL.md edits - tests/smoke-test.sh: run bootstrap twice to verify idempotence contract All 21 smoke tests passing (both runs).
…eld, smoke-test hardening, SOUL continuity gate
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0675e18785
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (fs.existsSync(p)) { ok(`${f} present`); passed++; } | ||
| else { err(`${f} missing at ${p}`); failed++; } |
There was a problem hiding this comment.
The smoke test currently treats any existing path as success, so a directory named MEMORY.md or jarvos.config.json is counted as "present" even though the actual file was never created. Because generation also skips when the path already exists, bootstrap can exit 0 with a broken workspace (I reproduced this by precreating workspace/jarvos.config.json/). This should verify isFile() for required files so success status is trustworthy.
Useful? React with 👍 / 👎.
…p fixes Resolved add/add conflicts in bootstrap.js, package.json, tests/smoke-test.sh, README.md by keeping the PR branch versions which contain all CodeRabbit-reviewed fixes.
Summary
Implements SUP-463: define real bootstrap/install flow for new users.
What's in this PR
bootstrap.sh/bootstrap.js: Interactive setup for new users--non-interactivemode accepts all defaults (CI)npx jarvos-bootstraptests/smoke-test.sh: 12-point smoke test verifying directory structure, config, and template placeholder replacementDone criteria
Closes SUP-463
Summary by CodeRabbit
New Features
Documentation
Tests