fix: recover Classic roots and parse raw Codex patches - #283
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @benym, your pull request is larger than the review limit of 150000 diff characters
|
👋 Thanks for opening your first PR to Comet, @benym. Before review, please make sure the PR title follows Conventional Commits, for example 🧪 The most useful local checks are: pnpm build
pnpm lint
pnpm format:check
pnpm test🧰 If your change touches ✨ We appreciate the contribution and will take a look as soon as we can. |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR removes Classic dual-root blocking, adds raw Codex patch target extraction, regenerates bundled Comet assets, updates dependency overrides, and releases version ChangesClassic layout coexistence
Raw Codex patch parsing
Bundled runtime regeneration
Beta release metadata
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Codex
participant HookAdapter
participant HookRouter
participant NativePhaseGuard
Codex->>HookAdapter: send raw apply_patch text
HookAdapter->>HookRouter: return apply_patch write targets
HookRouter->>NativePhaseGuard: validate project phase
NativePhaseGuard-->>HookRouter: allow build or reject other phases
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Reviewer's GuideUpdates the Classic root CLI to safely inspect and migrate between legacy and docs layouts (including dual-root scenarios), and enhances the Hook router/adapter to attribute and enforce raw Codex apply_patch inputs as Native writes before runtime dispatch, while bumping the bundled Comet runtime. Sequence diagram for parsing Codex apply_patch as Native writessequenceDiagram
participant Codex
participant HookRouter
participant HookAdapter
participant NativeGuard
Codex->>HookRouter: apply_patch payload
HookRouter->>HookAdapter: parseCometHookRequest(source, filePath)
activate HookAdapter
HookAdapter->>HookAdapter: JSON.parse(source)
alt parse fails
HookAdapter->>HookAdapter: patchTargets(source)
alt targets found
HookAdapter-->>HookRouter: intent write, targets, toolName apply_patch
else no targets
HookAdapter-->>HookRouter: intent unknown, targets [], toolName null
end
else parse succeeds
HookAdapter-->>HookRouter: structured request
end
deactivate HookAdapter
HookRouter->>NativeGuard: inspectNative(projectRoot, request)
NativeGuard-->>HookRouter: allow / block decision
HookRouter-->>Codex: platform-specific response (allow/deny)
Flow diagram for Classic dual-root inspection and migrationflowchart TD
A[User runs
comet classic root show] --> B[classicRootCommand]
B --> C[discoverClassicProject]
C --> D[assertClassicLayoutReadable
allowAlternateRoot true]
D --> E[inspectClassicLayout]
E --> F{dualRoots?}
F -- yes --> G[Emit layout JSON
with alternateRoot
and dualRoots true]
F -- no --> H[Emit layout JSON
with single root]
subgraph Doctor
I[comet doctor
checkClassicLayout]
I --> J[inspectClassicLayout]
J --> K{dualRoots?}
K -- yes --> L[Fail with detailed
conflict message]
K -- no --> M[Pass Classic
layout check]
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Greptile SummaryThis PR makes the configured Classic artifact layout the exclusive Comet ownership boundary and adds target extraction for raw Codex patches.
Confidence Score: 4/5The PR is not yet safe to merge because quoted unified-diff paths can still be evaluated against the wrong target during Native phase enforcement. The new raw-patch parser captures quoted Git paths without removing quotes, stripping the enclosed Files Needing Attention: domains/comet-entry/hook-adapter.ts
|
| Filename | Overview |
|---|---|
| domains/comet-entry/hook-adapter.ts | Adds raw patch target extraction, but the previously reported quoted Git-path parsing defect remains at HEAD. |
| domains/comet-classic/classic-layout.ts | Makes the configured artifact layout authoritative and permits an alternate standalone OpenSpec root. |
| domains/comet-classic/classic-root-command.ts | Reports root inspection details and preserves no-op behavior once the docs layout is configured. |
| domains/comet-classic/classic-evidence.ts | Restricts Classic evidence detection to the configured Comet-owned root. |
| domains/factory/package.ts | Aligns Factory routing with the configured Classic artifact root. |
Reviews (5): Last reviewed commit: "fix(test): align standalone OpenSpec lay..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #283 +/- ##
=======================================
Coverage 74.71% 74.72%
=======================================
Files 242 242
Lines 23483 23488 +5
Branches 7563 7564 +1
=======================================
+ Hits 17546 17551 +5
Misses 3008 3008
Partials 2929 2929
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@domains/comet-classic/classic-root-command.ts`:
- Around line 212-214: Update
domains/comet-classic/classic-root-command.ts:212-214 around
assertClassicLayoutReadable to detect a docs-layout dual-root conflict and
return an actionable conflict result rather than an already-migrated success.
Update app/commands/doctor.ts:315-315 to recommend move docs --dry-run only for
legacy layouts, with safe cleanup or layout-switch guidance for docs layouts.
Update domains/comet-classic/classic-layout.ts:66-66 so
ClassicLayoutConflictError provides layout-specific remediation. Add a
regression test in
test/domains/comet-classic/classic-root-command.test.ts:100-123 verifying
docs-layout dual-root handling does not report completed migration.
🪄 Autofix
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 Plus
Run ID: 9a29382b-2665-47ce-a62a-2cfd82ae4bd0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
CHANGELOG.mdapp/commands/doctor.tsassets/manifest.jsonassets/skills/comet-native/scripts/comet-native-hook-guard.mjsassets/skills/comet-native/scripts/comet-native-runtime.mjsassets/skills/comet/scripts/comet-archive.mjsassets/skills/comet/scripts/comet-guard.mjsassets/skills/comet/scripts/comet-handoff.mjsassets/skills/comet/scripts/comet-hook-guard.mjsassets/skills/comet/scripts/comet-hook-router.mjsassets/skills/comet/scripts/comet-resume-probe.mjsassets/skills/comet/scripts/comet-runtime.mjsassets/skills/comet/scripts/comet-state.mjsassets/skills/comet/scripts/comet-yaml-validate.mjsdomains/comet-classic/classic-layout.tsdomains/comet-classic/classic-root-command.tsdomains/comet-entry/hook-adapter.tspackage.jsontest/app/cli-help.test.tstest/app/doctor.test.tstest/domains/comet-classic/classic-root-command.test.tstest/domains/comet-entry/hook-adapter.test.tstest/domains/comet-entry/hook-router-runtime.test.tstest/domains/comet-entry/project-status.test.tstest/repository/release-metadata.test.ts
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@assets/skills/comet/scripts/comet-archive.mjs`:
- Around line 136-166: Regenerate the Classic runtime bundles from their current
source so they pass pnpm build:classic-runtime --check and remain consistent:
update assets/skills/comet/scripts/comet-archive.mjs lines 136-166,
assets/skills/comet/scripts/comet-hook-guard.mjs lines 136-154, and
assets/skills/comet/scripts/comet-state.mjs lines 136-166; no source-level
behavior changes are required.
In `@assets/skills/comet/scripts/comet-runtime.mjs`:
- Around line 4-5: Add the missing reproducibility check for the generated
Classic runtime bundles by running pnpm build:classic-runtime with --check
before merge. Apply this to assets/skills/comet/scripts/comet-runtime.mjs lines
4-5, assets/skills/comet/scripts/comet-yaml-validate.mjs lines 4-5, and
assets/skills/comet/scripts/comet-handoff.mjs lines 4-5 so the release assets
are compared against the checked build output.
- Around line 177-186: Update the Classic root-move write flow in
classic-root-command.ts and classic-root-move.ts to call
assertClassicLayoutWritable() before performing writes, including the recovery
path. Do not rely solely on guarded file operations, and ensure the check occurs
after resolving the layout so managed root paths are rejected before any
root-move changes are applied.
In `@assets/skills/comet/scripts/comet-yaml-validate.mjs`:
- Line 4: Do not edit the generated comet-yaml-validate.mjs asset directly;
update the corresponding source under domains/comet-classic, then run pnpm
build:classic-runtime to regenerate and synchronize the asset.
In `@domains/comet-classic/classic-evidence.ts`:
- Around line 229-235: Update the layout-validation flow around
assertClassicLayoutReadable and linkedFileEvidence so a failed layout check
cannot continue into the generic file check or return satisfied: true. Propagate
the layout error to linked evidence, or explicitly return unsatisfied linked
evidence until a valid ClassicLayoutPaths value is available, while preserving
normal linked evidence behavior for valid layouts.
In `@domains/comet-classic/classic-layout.ts`:
- Around line 233-236: Update assertClassicLayoutReadable so
assertClassicManagedRootsPhysical validates only the Comet-managed
artifact_layout root and does not validate inspection.alternateRoot. Keep
alternateRoot available as untrusted reporting state, preserve configured-root
reads and writes when it is unsafe, and add a regression case covering an unsafe
alternate-root symlink or junction with a valid configured root.
🪄 Autofix
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 Plus
Run ID: 818a31e3-c8bb-4cda-a8c3-9fbd0d13042a
📒 Files selected for processing (28)
CHANGELOG.mdapp/commands/doctor.tsassets/skills/comet/scripts/comet-archive.mjsassets/skills/comet/scripts/comet-guard.mjsassets/skills/comet/scripts/comet-handoff.mjsassets/skills/comet/scripts/comet-hook-guard.mjsassets/skills/comet/scripts/comet-hook-router.mjsassets/skills/comet/scripts/comet-resume-probe.mjsassets/skills/comet/scripts/comet-runtime.mjsassets/skills/comet/scripts/comet-state.mjsassets/skills/comet/scripts/comet-yaml-validate.mjsdocs/comet/specs/unified-product-catalogue/spec.mddomains/comet-classic/classic-evidence.tsdomains/comet-classic/classic-layout-initialization.tsdomains/comet-classic/classic-layout.tsdomains/comet-classic/classic-root-command.tsdomains/factory/package.tstest/app/doctor.test.tstest/domains/comet-classic/classic-evidence.test.tstest/domains/comet-classic/classic-layout-initialization.test.tstest/domains/comet-classic/classic-layout.test.tstest/domains/comet-classic/classic-openspec-command.test.tstest/domains/comet-classic/classic-resume-probe.test.tstest/domains/comet-classic/classic-runtime.test.tstest/domains/comet-classic/comet-scripts-guard.test.tstest/domains/comet-entry/hook-router.test.tstest/domains/comet-entry/project-status.test.tstest/domains/factory/factory-package.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/domains/comet-entry/project-status.test.ts
- CHANGELOG.md
- assets/skills/comet/scripts/comet-resume-probe.mjs
- assets/skills/comet/scripts/comet-guard.mjs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
🤖 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 `@assets/skills/comet/scripts/comet-archive.mjs`:
- Around line 136-166: Regenerate the Classic runtime bundles from their current
source so they pass pnpm build:classic-runtime --check and remain consistent:
update assets/skills/comet/scripts/comet-archive.mjs lines 136-166,
assets/skills/comet/scripts/comet-hook-guard.mjs lines 136-154, and
assets/skills/comet/scripts/comet-state.mjs lines 136-166; no source-level
behavior changes are required.
In `@assets/skills/comet/scripts/comet-runtime.mjs`:
- Around line 4-5: Add the missing reproducibility check for the generated
Classic runtime bundles by running pnpm build:classic-runtime with --check
before merge. Apply this to assets/skills/comet/scripts/comet-runtime.mjs lines
4-5, assets/skills/comet/scripts/comet-yaml-validate.mjs lines 4-5, and
assets/skills/comet/scripts/comet-handoff.mjs lines 4-5 so the release assets
are compared against the checked build output.
- Around line 177-186: Update the Classic root-move write flow in
classic-root-command.ts and classic-root-move.ts to call
assertClassicLayoutWritable() before performing writes, including the recovery
path. Do not rely solely on guarded file operations, and ensure the check occurs
after resolving the layout so managed root paths are rejected before any
root-move changes are applied.
In `@assets/skills/comet/scripts/comet-yaml-validate.mjs`:
- Line 4: Do not edit the generated comet-yaml-validate.mjs asset directly;
update the corresponding source under domains/comet-classic, then run pnpm
build:classic-runtime to regenerate and synchronize the asset.
In `@domains/comet-classic/classic-evidence.ts`:
- Around line 229-235: Update the layout-validation flow around
assertClassicLayoutReadable and linkedFileEvidence so a failed layout check
cannot continue into the generic file check or return satisfied: true. Propagate
the layout error to linked evidence, or explicitly return unsatisfied linked
evidence until a valid ClassicLayoutPaths value is available, while preserving
normal linked evidence behavior for valid layouts.
In `@domains/comet-classic/classic-layout.ts`:
- Around line 233-236: Update assertClassicLayoutReadable so
assertClassicManagedRootsPhysical validates only the Comet-managed
artifact_layout root and does not validate inspection.alternateRoot. Keep
alternateRoot available as untrusted reporting state, preserve configured-root
reads and writes when it is unsafe, and add a regression case covering an unsafe
alternate-root symlink or junction with a valid configured root.
🪄 Autofix
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 Plus
Run ID: 818a31e3-c8bb-4cda-a8c3-9fbd0d13042a
📒 Files selected for processing (28)
CHANGELOG.mdapp/commands/doctor.tsassets/skills/comet/scripts/comet-archive.mjsassets/skills/comet/scripts/comet-guard.mjsassets/skills/comet/scripts/comet-handoff.mjsassets/skills/comet/scripts/comet-hook-guard.mjsassets/skills/comet/scripts/comet-hook-router.mjsassets/skills/comet/scripts/comet-resume-probe.mjsassets/skills/comet/scripts/comet-runtime.mjsassets/skills/comet/scripts/comet-state.mjsassets/skills/comet/scripts/comet-yaml-validate.mjsdocs/comet/specs/unified-product-catalogue/spec.mddomains/comet-classic/classic-evidence.tsdomains/comet-classic/classic-layout-initialization.tsdomains/comet-classic/classic-layout.tsdomains/comet-classic/classic-root-command.tsdomains/factory/package.tstest/app/doctor.test.tstest/domains/comet-classic/classic-evidence.test.tstest/domains/comet-classic/classic-layout-initialization.test.tstest/domains/comet-classic/classic-layout.test.tstest/domains/comet-classic/classic-openspec-command.test.tstest/domains/comet-classic/classic-resume-probe.test.tstest/domains/comet-classic/classic-runtime.test.tstest/domains/comet-classic/comet-scripts-guard.test.tstest/domains/comet-entry/hook-router.test.tstest/domains/comet-entry/project-status.test.tstest/domains/factory/factory-package.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/domains/comet-entry/project-status.test.ts
- CHANGELOG.md
- assets/skills/comet/scripts/comet-resume-probe.mjs
- assets/skills/comet/scripts/comet-guard.mjs
🛑 Comments failed to post (4)
assets/skills/comet/scripts/comet-archive.mjs (1)
136-166: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Confirm domains/comet-classic sources exist and the build script regenerates # comet-archive.mjs, comet-hook-guard.mjs, and comet-state.mjs consistently. set -euo pipefail rg -n '"build:classic-runtime"' package.json fd . domains/comet-classic --type f fd 'comet-(archive|hook-guard|state)\.mjs$' assets/skills/comet/scriptsRepository: rpamis/comet
Length of output: 2450
🏁 Script executed:
#!/bin/bash # Inspect the Classic build script usage and the generated/imported source fragments. set -euo pipefail printf '--- build-classic-runtime outline ---\n' sed -n '1,220p' scripts/build/build-classic-runtime.mjs printf '\n--- package build script context ---\n' python3 - <<'PY' import json from pathlib import Path data=json.loads(Path('package.json').read_text()) for k in ('build:classic-runtime','clean:skills','build'): print(f'{k}: {data.get("scripts", {}).get(k)!r}') PY printf '\n--- generated script entry/import references ---\n' for f in assets/skills/comet/scripts/comet-{archive,hook-guard,state}.mjs; do echo "### $f" sed -n '1,80p' "$f" | sed -n '/import\|export\|process\.exitCode/p' doneRepository: rpamis/comet
Length of output: 241
🏁 Script executed:
# No results to apply yet.Repository: rpamis/comet
Length of output: 150
🏁 Script executed:
#!/bin/bash # Inspect the Classic build script and generated entry references. set -euo pipefail printf '%s\n' '--- build-classic-runtime outline ---' sed -n '1,220p' scripts/build/build-classic-runtime.mjs printf '%s\n' '' printf '%s\n' '--- package build script context ---' python3 - <<'PY' import json from pathlib import Path data=json.loads(Path('package.json').read_text()) for k in ('build:classic-runtime','clean:skills','build'): print(f'{k}: {data.get("scripts", {}).get(k)!r}') PY printf '%s\n' '' printf '%s\n' '--- generated script entry/import references ---' for f in assets/skills/comet/scripts/comet-{archive,hook-guard,state}.mjs; do printf '### %s\n' "$f" sed -n '1,80p' "$f" | sed -n '/import\|export\|process\.exitCode/p' doneRepository: rpamis/comet
Length of output: 23687
Make these Classic assets pass
pnpm build:classic-runtime --check. The Classic runtime build generates separate bundles forassets/skills/comet/scripts/comet-archive.mjs,assets/skills/comet/scripts/comet-hook-guard.mjs, andassets/skills/comet/scripts/comet-state.mjs; regenerate them with the same source to keep the asset outputs consistent.📍 Affects 3 files
assets/skills/comet/scripts/comet-archive.mjs#L136-L166(this comment)assets/skills/comet/scripts/comet-hook-guard.mjs#L136-L154assets/skills/comet/scripts/comet-state.mjs#L136-L166🤖 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 `@assets/skills/comet/scripts/comet-archive.mjs` around lines 136 - 166, Regenerate the Classic runtime bundles from their current source so they pass pnpm build:classic-runtime --check and remain consistent: update assets/skills/comet/scripts/comet-archive.mjs lines 136-166, assets/skills/comet/scripts/comet-hook-guard.mjs lines 136-154, and assets/skills/comet/scripts/comet-state.mjs lines 136-166; no source-level behavior changes are required.Source: Coding guidelines
assets/skills/comet/scripts/comet-runtime.mjs (2)
4-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Verify Classic source ownership and generated-bundle reproducibility. set -euo pipefail # Confirm the declared generator command. rg -n '"build:classic-runtime"' package.json # Confirm Classic logic lives in the source domain. rg -n -C4 'dualRoots|alternateRoot|configuredRootExists|standalone OpenSpec root' domains --type=ts || true # Confirm the repository asserts generated-runtime parity in tests. fd -t f -g '*runtime-assets*.test.ts' test || true rg -n -C4 'comet-runtime.mjs|comet-handoff.mjs|comet-yaml-validate.mjs' test/repository config/repository-layout.json assets/manifest.json || trueRepository: rpamis/comet
Length of output: 18534
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== classic-runtime build script outline ==" wc -l scripts/build/build-classic-runtime.mjs ast-grep outline scripts/build/build-classic-runtime.mjs || true echo echo "== key build-classic-runtime sections ==" rg -n -C4 'classicRuntime|manifest|repository-layout|readJson|cpSync|generate|outputs|dual|alternate|standalone' scripts/build/build-classic-runtime.mjs || true echo echo "== source references to validation/standalone/evidence text in bundles ==" python3 - <<'PY' from pathlib import Path import re bundles = [ Path('assets/skills/comet/scripts/comet-runtime.mjs'), Path('assets/skills/comet/scripts/comet-yaml-validate.mjs'), Path('assets/skills/comet/scripts/comet-handoff.mjs'), ] patterns = { 'dual': r'dualRoots|alternateRoot|configuredRootExists|Artifact root is not found in the configured Classic OpenSpec root|dual |two|^' + re.escape('standalone OpenSpec root is not a Comet artifact root') + r'|refusing to move|Refusing to move', 'validate': r'Refusing to remove an OpenSpec root that does not match project config|Refusing to remove conflicting legacy and docs OpenSpec roots|dualRoots|alternateRootExists', 'handoff': r'Configured Classic OpenSpec root is missing|alternate is .*present|standalone OpenSpec root is not a Comet artifact root|root ownership boundary', } for bundle in bundles: text = bundle.read_text(errors='replace') print(f'-- {bundle}') for name, pat in patterns.items(): print(f'{name}:', 'YES' if re.search(pat, text, re.S) else 'NO') if name != 'validate': for i, line in enumerate(text.splitlines(), 1): if re.search(pat, line): print(f' {i}: {line[:220]!r}') print() PY echo "== test coverage around runtime assets reproducibility ==" wc -l test/repository/classic-runtime-assets.test.ts cat -n test/repository/classic-runtime-assets.test.ts echo echo "== git status stat/diff summary ==" git status --short git diff --stat echoRepository: rpamis/comet
Length of output: 9019
Run the reproducibility check for the three Classic runtime bundles. These are generated release assets, and the current test coverage does not compare them against
pnpm build:classic-runtime. Run the build with--checkbefore merge to covercomet-runtime.mjs,comet-yaml-validate.mjs, andcomet-handoff.mjs.📍 Affects 3 files
assets/skills/comet/scripts/comet-runtime.mjs#L4-L5(this comment)assets/skills/comet/scripts/comet-yaml-validate.mjs#L4-L5assets/skills/comet/scripts/comet-handoff.mjs#L4-L5🤖 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 `@assets/skills/comet/scripts/comet-runtime.mjs` around lines 4 - 5, Add the missing reproducibility check for the generated Classic runtime bundles by running pnpm build:classic-runtime with --check before merge. Apply this to assets/skills/comet/scripts/comet-runtime.mjs lines 4-5, assets/skills/comet/scripts/comet-yaml-validate.mjs lines 4-5, and assets/skills/comet/scripts/comet-handoff.mjs lines 4-5 so the release assets are compared against the checked build output.Source: Coding guidelines
177-186: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Verify that Classic write paths use the write-guarded layout resolver. set -euo pipefail # Find both resolvers in source and list their callers. rg -n -C5 'classic-root-move.json' domains --type=ts || true rg -n -C3 'ensureClassicLayout|requireWritableClassicLayout|resolveClassicLayout' domains app --type=ts || true # Confirm regression coverage for the read-relaxed and write-blocked cases. rg -n -C4 'dualRoots|alternateRootExists|configuredRootExists' test --type=ts || trueRepository: rpamis/comet
Length of output: 6612
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== classic-layout exports and layout resolver ==" ast-grep outline domains/comet-classic/classic-layout.ts --view expanded | sed -n '1,220p' echo echo "== classic-layout relevant sections ==" sed -n '140,280p' domains/comet-classic/classic-layout.ts | cat -n echo echo "== writes using assertClassicLayoutWritable / direct Classic writer APIs ==" rg -n -C3 'assertClassicLayoutWritable|writeClassicArtifactLayout|runClassic|commitClassicClassic|classic-root-move|requireWritableClassicLayout|resolveClassicLayout' --glob '!**/dist/**' --glob '!**/build/**' . echo echo "== command callers of layout resolvers ==" for f in app/commands/doctor.ts app/commands/*.ts domains/comet-classic/*.ts; do [ -f "$f" ] || continue echo "-- $f" rg -n 'resolveClassicLayout|assertClassicLayoutWritable|inspectClassicLayout|writeClassicArtifactLayout|classic-root-move' "$f" || true doneRepository: rpamis/comet
Length of output: 50369
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== classic command tests with layout assertions ==" rg -n -C4 'assertClassicLayoutWritable|resolveClassicLayout|dualRoots|alternateRootExists|configuredRootExists|root move|old|new|legacy|docs' test/domains test/app --type=ts || true echo echo "== relevant test files ==" wc -l test/domains/comet-classic/classic-root-command.test.ts domains/comet-classic/classic-layout.ts domains/comet-classic/classic-root-move.tsRepository: rpamis/comet
Length of output: 50369
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== sources importing resolver functions ==" rg -n 'from .*classic-layout\.js|from ./classic-layout' domains app --type=ts --type-add ts:*.ts --type-add ts:*.tsx --type-add ts:*.mjs --type-add ts:*.cjs || true echo echo "== Classic command/source files under domains/comet-classic ==" git ls-files 'domains/comet-classic/*.ts' 'app/commands/*.ts' | sed 's#^\./##' | sort echo echo "== exports/usages in source file names ==" for f in $(git ls-files 'domains/comet-classic/*.ts' 'app/commands/*.ts' | sort); do echo "-- $f" rg -n 'resolveClassicLayout|assertClassicLayoutWritable|inspectClassicLayout|writeClassicArtifactLayout|classic-root-move|classic/layout|resolveClassic.*Root|Ee|G\(' "$f" || true done echo echo "== classic root move relevant section ==" sed -n '280,520p' domains/comet-classic/classic-root-move.ts | cat -n echo echo "== deterministic behavior probe from source text ==" python3 - <<'PY' from pathlib import Path p=Path('domains/comet-classic/classic-layout.ts') s=p.read_text() start=s.find('async function G(') end=s.find('async function Ee(', start) print(s[start:end]) print('\n--- assertClassicLayoutWritable ---') a=s.find('export async function assertClassicLayoutWritable(') print(s[a:a+1200]) PYRepository: rpamis/comet
Length of output: 24346
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== inspectClassicLayout source ==" sed -n '150,240p' domains/comet-classic/classic-layout.ts | cat -n echo echo "== exports from classic-layout ==" sed -n '1,180p' domains/comet-classic/classic-layout.ts | cat -n echo echo "== classic-layout-initialization around legacy root write guards ==" sed -n '920,1185p' domains/comet-classic/classic-layout-initialization.ts | cat -n echo echo "== writeClassicArtifactLayout source ==" sed -n '90,150p' domains/comet-classic/classic-layout.ts | cat -n echo echo "== focused Classic write entry points using assertClassicLayoutWritable/source paths directly ==" python3 - <<'PY' from pathlib import Path for p in Path('domains/comet-classic').glob('*.ts'): text=p.read_text() if 'resolveClassicLayout(' in text or 'assertClassicLayoutReadable(' in text: print(f'-- {p}') for i,line in enumerate(text.splitlines(),1): if 'resolveClassicLayout(' in line or 'assertClassicLayoutReadable(' in line or 'classicLayoutPaths(' in line and 'resolveClassicLayout' not in line: print(f'{i}: {line.strip()}') if 'assertClassicLayoutWritable(' in text: print(f'-- {p}') for i,line in enumerate(text.splitlines(),1): if 'assertClassicLayoutWritable(' in line: print(f'{i}: {line.strip()}') PYRepository: rpamis/comet
Length of output: 29439
Gate root-move writes through the write-safe layout resolver.
resolveClassicLayout()now skips managed root paths, whileclassic-root-command.tsstill calls it beforeclassic-root-move.tscan apply root-move write guards. UseassertClassicLayoutWritable()before Classic root-move writes, including the recovery path, before relying only on guarded file operations.🤖 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 `@assets/skills/comet/scripts/comet-runtime.mjs` around lines 177 - 186, Update the Classic root-move write flow in classic-root-command.ts and classic-root-move.ts to call assertClassicLayoutWritable() before performing writes, including the recovery path. Do not rely solely on guarded file operations, and ensure the check occurs after resolving the layout so managed root paths are rejected before any root-move changes are applied.assets/skills/comet/scripts/comet-yaml-validate.mjs (1)
4-4: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Confirm Classic layout changes exist in source and that generated bundles are reproducible. set -euo pipefail # Locate the Classic layout source that owns configured-root inspection. fd -t f 'classic-layout*.ts' domains || true rg -n -C4 'ClassicLayoutConflictError|dualRoots|alternateRoot|configuredRootExists' domains --type=ts || true # Confirm the build script exists and is the declared generator for the bundles. rg -n '"build:classic-runtime"' package.json rg -n -C3 'comet-yaml-validate|comet-handoff|comet-runtime' package.json scripts 2>/dev/null || trueRepository: rpamis/comet
Length of output: 17027
🏁 Script executed:
#!/bin/bash # Inspect the generated bundle size and whether it is tracked / gitignored. set -euo pipefail printf 'tracked:\n' git ls-files assets/skills/comet/scripts/comet-yaml-validate.mjs assets/skills/comet/scripts/comet-runtime.mjs || true printf '\nignore patterns:\n' git check-ignore -v assets/skills/comet/scripts/comet-yaml-validate.mjs assets/skills/comet/scripts/comet-runtime.mjs || true printf '\nalphabetical hashes and sizes:\n' git ls-tree HEAD assets/skills/comet/scripts --no-renames | sort printf '\nsize:\n' wc -c assets/skills/comet/scripts/comet-yaml-validate.mjs assets/skills/comet/scripts/comet-runtime.mjs 2>/dev/null || trueRepository: rpamis/comet
Length of output: 1043
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'package scripts around build:classic-runtime:\n' python3 - <<'PY' import json from pathlib import Path p=Path('package.json') if p.exists(): data=json.loads(p.read_text()) print(data.get('scripts',{}).get('build:classic-runtime')or '') PY printf '\nbuild script references:\n' rg -n 'build-classic-runtime|mjs$|bundle|esbuild|vite|rollup|webpack' scripts/build -g '*.mjs' -g '*.ts' || true fd -t f 'build-classic-runtime\.mjs|build-classic-runtime\.ts' scripts/assets scripts -i || true printf '\nasset files in comet scripts:\n' git ls-tree --name-only HEAD -- assets/skills/comet/scripts/ | sort printf '\nfile sizes:\n' wc -c assets/skills/comet/scripts/comet-yaml-validate.mjs assets/skills/comet/scripts/comet-runtime.mjsRepository: rpamis/comet
Length of output: 3190
Keep generated Classic assets in sync with the build script.
comet-yaml-validate.mjsis produced bypnpm build:classic-runtime, whose sources live underdomains/comet-classic/. Run that build and check the generated asset; edit source code beforeassets/skills/comet/scripts/comet-yaml-validate.mjsinstead.🤖 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 `@assets/skills/comet/scripts/comet-yaml-validate.mjs` at line 4, Do not edit the generated comet-yaml-validate.mjs asset directly; update the corresponding source under domains/comet-classic, then run pnpm build:classic-runtime to regenerate and synchronize the asset.Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
classic.artifact_layoutthe sole Comet ownership boundary:openspec/anddocs/openspec/can coexist, while Classic commands, status, guards, evidence, and Factory routing use only the configured root.docs/openspec/beside an existing pureopenspec/root.comet classic root move docs --applyremains fail-closed when its destination is non-empty.apply_patchinput, including Add/Update/Delete and standard+++ b/...headers, before Native phase enforcement.0.4.0-beta.16and refreshes generated runtimes.Verification
npx vitest run test/domains/comet-classic test/domains/comet-entry: 829 passed, 16 skipped.npx vitest run test/domains/factory/factory-package.test.ts: 39 passed.pnpm buildpassed.pnpm check:generatedpassed.pnpm lintand architecture lint passed.pnpm testwas attempted locally on Windows but exceeded 10 minutes without a failure assertion; CI remains the final full-suite check.Closes #277
Closes #282
Summary by CodeRabbit