feat: prepare beta.11 configuration and native UX - #253
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. |
Reviewer's GuideThis PR removes the Native signed-v2 creation/authorization flow, switches verification to a single legacy-v1 protocol with risk-based independent review requirements, and updates project configuration, CLI, skills, and tests to default Classic layout to docs while preserving legacy artifacts via update. Sequence diagram for risk-based independent review requirementsequenceDiagram
participant Caller as NativeVerificationCaller
participant Runtime as inspectNativeVerificationEvidence
participant Facts as NativeVerificationFacts
Caller->>Runtime: inspectNativeVerificationEvidence(state, result)
Runtime->>Facts: getBundleScope()
Facts-->>Runtime: bundle.scope
Runtime->>Runtime: isNativeHighRiskScope(bundle.scope.changes)
alt !bundle.scope.complete OR isNativeHighRiskScope
Runtime->>Runtime: [independentReviewRequired]
Runtime->>Caller: throw Error("independent review required for pass")
else bundle.scope.complete AND !highRisk
Runtime->>Caller: return (no independent review needed)
end
Flow diagram for selecting Classic artifact layoutflowchart TD
A[Start] --> B{classic.artifact_layout present
in project config?}
B -->|Yes| C[classicArtifactLayout = config.classic.artifact_layout]
B -->|No| D{root-level openspec/
exists and docs/openspec/
does NOT exist?}
D -->|Yes| E[classicArtifactLayout = legacy]
D -->|No| F[classicArtifactLayout = docs]
C --> G[Use classicArtifactLayout
for init/update]
E --> G
F --> G
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe PR changes Classic artifact-layout defaults and migration detection, removes Native creation authorization and signed-v2 creation flows, bases independent review on scope and risk, hardens bundled Native runtime validation, and updates release metadata, documentation, fixtures, and tests. ChangesClassic layout and configuration
Native protocol and runtime
Release and supporting updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+344
to
347
| if (value !== 'legacy-v1') { | ||
| throw new Error('Native verification_protocol must be legacy-v1'); | ||
| } | ||
| return value; |
There was a problem hiding this comment.
When a project upgrades from beta.10 with an existing Native change whose state records verification_protocol: signed-v2, this parser rejects the persisted state before it can be inspected or migrated, causing listing, resuming, and advancing that change to fail with Native verification_protocol must be legacy-v1.
Greptile SummaryThis PR prepares beta.11 by changing project configuration defaults and simplifying Native authorization and review behavior.
Confidence Score: 3/5The PR should not merge until existing signed-v2 Native changes can be read or migrated after upgrade. The preceding beta created signed-v2 state, while the new parser accepts only legacy-v1 and provides no migration before normal read and resume operations. Files Needing Attention: domains/comet-native/native-change.ts, domains/comet-native/native-types.ts, assets/skills/comet-native/scripts/comet-native-runtime.mjs
|
| Filename | Overview |
|---|---|
| domains/comet-native/native-change.ts | Removes signed creation authorization and signed-v2 parsing, leaving existing beta.10 signed changes unreadable. |
| domains/comet-native/native-verification-runtime.ts | Recomputes independent-review requirements from current implementation scope and consistently treats incomplete scope as high risk. |
| app/commands/update.ts | Infers a missing Classic layout from explicit raw configuration and existing artifact roots before merging managed defaults. |
| domains/skill/platform-install.ts | Adds missing managed configuration defaults while preserving explicit nested configuration values. |
| assets/skills/comet-native/scripts/comet-native-runtime.mjs | Synchronizes the Native runtime behavior, including the same persisted signed-v2 compatibility break. |
Reviews (1): Last reviewed commit: "feat: prepare beta.11 configuration and ..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (63.33%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 73.97% 74.06% +0.09%
==========================================
Files 205 204 -1
Lines 22720 22614 -106
Branches 7230 7204 -26
==========================================
- Hits 16806 16748 -58
+ Misses 3070 3035 -35
+ Partials 2844 2831 -13
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
domains/workflow-contract/project-config.ts (1)
735-741: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win将 runtime helper 的回退也改为
docs，并重新生成 Runtime bundle。
domains/workflow-contract/project-config.ts中workflowProjectConfigRuntimeHelperScript()里的normalizeClassicArtifactLayout(classic.artifact_layout, 'legacy')仍覆盖 helper 的默认值，导致缺少布局字段的 managed workflow 配置继续解析为 legacy；修复该处后，需同步生成comet-entry-runtime.mjs和comet-runtime.mjs。🤖 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 `@domains/workflow-contract/project-config.ts` around lines 735 - 741, 将 workflowProjectConfigRuntimeHelperScript() 中对 normalizeClassicArtifactLayout 的显式回退值从 legacy 改为 docs，并重新生成 assets/skills/comet/scripts/comet-entry-runtime.mjs（7409-7415）和 assets/skills/comet/scripts/comet-runtime.mjs（7445-7451），确保三个位置一致且缺少布局字段时解析为 docs。Source: Coding guidelines
domains/comet-native/native-cli.ts (1)
838-845: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift放开“活跃 change 期间禁止创建 trust policy”限制前，确保独立审查路径不会因 baseline 缺失而永久阻塞。
loadNativeReviewTrustPolicy会要求.comet/native-review-trust.json出现在 change 创建时的 baseline 中，且 Build snapshot 保持相同哈希/大小；review-policy-after-creation场景在创建 change 后才生成 policy，后续若被isNativeHighRiskScope判定为高风险需要独立审查，当前实现没有 baseline 刷新或后续补录机制。🤖 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 `@domains/comet-native/native-cli.ts` around lines 838 - 845, 在放开活跃 change 期间创建 trust policy 的限制前，补充 baseline 更新或后续补录机制，确保 `review-policy-after-creation` 创建的 `.comet/native-review-trust.json` 能被独立审查流程识别。调整 `loadNativeReviewTrustPolicy` 相关校验及 change 创建后的处理，使 policy 的哈希/大小基线与实际文件同步，并保持 `isNativeHighRiskScope` 判定为高风险时不会因 baseline 缺失永久阻塞。
🧹 Nitpick comments (1)
assets/skills/comet-native/scripts/comet-native-runtime.mjs (1)
14114-14120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
assertNativeVerificationProtocolBindingis now unreachable validation.
verificationProtocol()already rejects anything other thanlegacy-v1, so thestate.verification_protocol !== "legacy-v1"branch can never run for a parsed state, and thebaseline === nullearly return makes the function a no-op otherwise. Consider removing the helper (or giving it a real binding check) indomains/comet-native/native-change.tsand regenerating, so the bundled runtime does not carry dead protocol validation.Also applies to: 14475-14481
🤖 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-native/scripts/comet-native-runtime.mjs` around lines 14114 - 14120, Remove the unreachable assertNativeVerificationProtocolBinding helper and its generated runtime usage, or implement a genuine binding check instead of duplicating verificationProtocol validation. Update the source definition in native-change.ts and regenerate comet-native-runtime.mjs so the bundled runtime no longer contains dead protocol validation.Source: Coding guidelines
🤖 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 `@AGENTS.md`:
- Around line 221-222: 更新 AGENTS.md 第 221-222 行和 CLAUDE.md 第 219-220 行的
Superpowers Skill 使用规则，为仓库自有 Comet 工作流增加例外：用户调用 /comet 即视为明确授权，允许按 Comet 工作流要求调用
OpenSpec/Superpowers 依赖 Skill；其他场景仍需用户显式同意。
In `@app/commands/update.ts`:
- Around line 1343-1357: Move the Classic artifact-layout resolution currently
using rawClassic and filesystem checks into the resolver under
domains/comet-classic, including explicit classic.artifact_layout precedence and
legacy/docs fallback rules. Add domain tests covering explicit layout, only
openspec/, only docs/openspec/, both roots, and neither root; reduce update.ts
to orchestration that calls the domain resolver.
In `@assets/skills/comet/scripts/comet-hook-router.mjs`:
- Around line 11775-11785: Update isWithin to reject only the parent-directory
path prefix (“..” or “..” followed by the platform separator), not arbitrary
paths beginning with “..” such as “..foo”. Apply the same boundary logic used by
the corresponding isInside* helper, then regenerate the asset through the
established build:entry-runtime process so comet-hook-router.mjs stays
synchronized.
In `@domains/comet-entry/init-workflow.ts`:
- Around line 129-139: 在覆盖 init workflow 布局推断的测试中，补充 classicAlreadyEnabled 且缺少
classic.artifact_layout 时的分支：当项目根目录存在 openspec/ 时断言解析为 legacy，并在该目录不存在时断言回退为
docs。复用现有测试中的文件系统模拟和布局解析入口，确保显式布局及其他已有场景保持不变。
In `@test/domains/comet-classic/classic-layout.test.ts`:
- Around line 82-84: Update the fallback value used by
readClassicArtifactLayout() in the classic layout runtime code from legacy to
docs, so missing configuration resolves to docs and classicLayoutPaths() selects
the docs/openspec location.
---
Outside diff comments:
In `@domains/comet-native/native-cli.ts`:
- Around line 838-845: 在放开活跃 change 期间创建 trust policy 的限制前，补充 baseline
更新或后续补录机制，确保 `review-policy-after-creation` 创建的
`.comet/native-review-trust.json` 能被独立审查流程识别。调整 `loadNativeReviewTrustPolicy`
相关校验及 change 创建后的处理，使 policy 的哈希/大小基线与实际文件同步，并保持 `isNativeHighRiskScope`
判定为高风险时不会因 baseline 缺失永久阻塞。
In `@domains/workflow-contract/project-config.ts`:
- Around line 735-741: 将 workflowProjectConfigRuntimeHelperScript() 中对
normalizeClassicArtifactLayout 的显式回退值从 legacy 改为 docs，并重新生成
assets/skills/comet/scripts/comet-entry-runtime.mjs（7409-7415）和
assets/skills/comet/scripts/comet-runtime.mjs（7445-7451），确保三个位置一致且缺少布局字段时解析为
docs。
---
Nitpick comments:
In `@assets/skills/comet-native/scripts/comet-native-runtime.mjs`:
- Around line 14114-14120: Remove the unreachable
assertNativeVerificationProtocolBinding helper and its generated runtime usage,
or implement a genuine binding check instead of duplicating verificationProtocol
validation. Update the source definition in native-change.ts and regenerate
comet-native-runtime.mjs so the bundled runtime no longer contains dead protocol
validation.
🪄 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 Plus
Run ID: 917a5664-dd6e-47ab-80d0-693e3060cf10
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (53)
AGENTS.mdCHANGELOG.mdCLAUDE.mdapp/commands/update.tsassets/manifest.jsonassets/skills-zh/comet-native/SKILL.mdassets/skills-zh/comet-native/reference/commands.mdassets/skills-zh/comet/reference/classic-layout.mdassets/skills/comet-native/SKILL.mdassets/skills/comet-native/reference/commands.mdassets/skills/comet-native/scripts/comet-native-runtime.mjsassets/skills/comet/reference/classic-layout.mdassets/skills/comet/scripts/comet-entry-runtime.mjsassets/skills/comet/scripts/comet-hook-router.mjsassets/skills/comet/scripts/comet-runtime.mjsdocs/comet/specs/native-completion-loop/spec.mddocs/comet/specs/native-verification-evidence/spec.mddomains/comet-classic/classic-layout-initialization.tsdomains/comet-entry/init-workflow.tsdomains/comet-native/native-change.tsdomains/comet-native/native-cli.tsdomains/comet-native/native-controller-trust.tsdomains/comet-native/native-creation-authorization.tsdomains/comet-native/native-review-trust.tsdomains/comet-native/native-snapshot.tsdomains/comet-native/native-types.tsdomains/comet-native/native-verification-runtime.tsdomains/skill/platform-install.tsdomains/workflow-contract/project-config.tseval/local/tasks/comet-native-workflow/instruction.mdeval/local/tasks/comet-native-workflow/validation/test_native_workflow.pyeval/local/tests/conftest.pyeval/local/tests/scaffold/test_native_wave_evaluations.pypackage.jsontest/app/cli-help.test.tstest/app/init-e2e.test.tstest/app/update.test.tstest/domains/comet-classic/classic-layout.test.tstest/domains/comet-entry/init-workflow.test.tstest/domains/comet-native/native-change.test.tstest/domains/comet-native/native-cli.test.tstest/domains/comet-native/native-config.test.tstest/domains/comet-native/native-evidence-transitions.test.tstest/domains/comet-native/native-review-trust.test.tstest/domains/comet-native/native-skill.test.tstest/domains/comet-native/native-verification-runtime.test.tstest/domains/skill/classic-layout-skill-contract.test.tstest/domains/skill/skills.test.tstest/domains/workflow-contract/workflow-contract.test.tstest/helpers/native-controller-trust.tstest/repository/native-runtime-assets.test.tstest/repository/release-metadata.test.tswebsite
💤 Files with no reviewable changes (3)
- domains/comet-native/native-creation-authorization.ts
- domains/comet-native/native-snapshot.ts
- test/helpers/native-controller-trust.ts
Comment on lines
+221
to
+222
| 除非用户明确同意，否则不得使用 Superpowers 的任何 Skill。 | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a '^(AGENTS|CLAUDE)\.md$' . || true
echo "== relevant AGENTS.md sections =="
if [ -f AGENTS.md ]; then
sed -n '1,260p' AGENTS.md | nl -ba | sed -n '180,235p'
fi
echo "== relevant CLAUDE.md sections =="
if [ -f CLAUDE.md ]; then
sed -n '180,235p' CLAUDE.md | nl -ba | sed -n '185,225p'
fi
echo "== references to Superpowers/Superpowers/Superpower and comet =="
rg -n "Superpowers|Superpower|/comet|comet resume-probe|nextCommand|Comet|Comet workflow|Comet workflow|Comet 工作流" -S . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: rpamis/comet
Length of output: 429
🏁 Script executed:
#!/bin/bash
set -u
echo "== AGENTS.md around cited lines =="
awk 'NR>=180 && NR<=235 { printf "%5d: %s\n", NR, $0 }' AGENTS.md
echo "== CLAUDE.md around cited lines =="
awk 'NR>=180 && NR<=235 { printf "%5d: %s\n", NR, $0 }' CLAUDE.md
echo "== eval local skill context =="
if [ -f eval/local/skills/benchmarks/dependency/claude-md/comet-workflow/CLAUDE.md ]; then
wc -l eval/local/skills/benchmarks/dependency/claude-md/comet-workflow/CLAUDE.md
awk 'NR>=1 && NR<=260 { printf "%5d: %s\n", NR, $0 }' eval/local/skills/benchmarks/dependency/claude-md/comet-workflow/CLAUDE.md
fi
echo "== references to Superpowers/Superpowers Skill and Comet workflow =="
grep -RIn "除非用户明确同意|Superpowers|Superpowers Skill|/comet|resume-probe|nextCommand|comet workflow|Comet workflow|Comet 工作流" . \
--exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build \
2>/dev/null | sed -n '1,240p'Repository: rpamis/comet
Length of output: 5245
为 Comet 工作流保留 Superpowers Skill 例外
AGENTS.md 和 CLAUDE.md 现在都要求 Superpowers Skill 必须经用户显式同意，但 eval/local/skills/benchmarks/dependency/claude-md/comet-workflow/CLAUDE.md 明确规定 Comet 工作流在涉及 OpenSpec/Superpowers 依赖 Skill 时必须调用它。把这两条规则直接并列会阻碍正常 /comet 执行；应说明用户调用 /comet 即视为授权，或为仓库自有 Comet 工作流单独增加例外。
📍 Affects 2 files
AGENTS.md#L221-L222(this comment)CLAUDE.md#L219-L220
🤖 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 `@AGENTS.md` around lines 221 - 222, 更新 AGENTS.md 第 221-222 行和 CLAUDE.md 第
219-220 行的 Superpowers Skill 使用规则，为仓库自有 Comet 工作流增加例外：用户调用 /comet 即视为明确授权，允许按
Comet 工作流要求调用 OpenSpec/Superpowers 依赖 Skill；其他场景仍需用户显式同意。
Comment on lines
+1343
to
+1357
| const rawClassic = projectConfigDocument?.value.classic; | ||
| const explicitClassicArtifactLayout = | ||
| rawClassic !== null && | ||
| typeof rawClassic === 'object' && | ||
| !Array.isArray(rawClassic) && | ||
| ((rawClassic as Record<string, unknown>).artifact_layout === 'legacy' || | ||
| (rawClassic as Record<string, unknown>).artifact_layout === 'docs') | ||
| ? ((rawClassic as Record<string, unknown>).artifact_layout as 'legacy' | 'docs') | ||
| : null; | ||
| const classicArtifactLayout = | ||
| explicitClassicArtifactLayout ?? | ||
| ((await fileExists(path.join(projectPath, 'openspec'))) && | ||
| !(await fileExists(path.join(projectPath, 'docs', 'openspec'))) | ||
| ? 'legacy' | ||
| : 'docs'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^app/commands/update\.ts$|^domains/.*comet[-_]?classic|classic)' | head -200
echo "== update outline =="
ast-grep outline app/commands/update.ts --view compact | sed -n '1,220p'
echo "== relevant update lines =="
sed -n '1310,1375p' app/commands/update.ts
sed -n '1770,1810p' app/commands/update.ts
echo "== classic resolver references =="
rg -n "artifact_layout|classicArtifactLayout|classic.*layout|openspec|docs.*openspec|legacy|docs" app/commands/update.ts domains app -g '*.ts' | head -200Repository: rpamis/comet
Length of output: 11661
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== update relevant code =="
sed -n '1320,1365p' app/commands/update.ts
sed -n '1760,1810p' app/commands/update.ts
echo "== classic layout domain files =="
wc -l domains/comet-classic/classic-layout*.ts domains/comet-classic/classic-project-config.ts test/domains/comet-classic/classic-layout*.ts
sed -n '1,260p' domains/comet-classic/classic-layout.ts
sed -n '1,220p' domains/comet-classic/classic-layout-initialization.ts
sed -n '1,220p' domains/comet-classic/classic-project-config.ts
echo "== layout config tests =="
sed -n '1,260p' test/domains/comet-classic/classic-layout.test.ts
sed -n '1,260p' test/domains/comet-classic/classic-layout-initialization.test.ts
rg -n "artifact_layout|docs.*openspec|openspec|legacy|classic.*layout|ProjectPath|projectPath" test/domains/comet-classic/classic-layout*.ts domains/comet-classic/classic-layout*.ts test/repository/classic-layout-documentation.test.ts test app/commands/update.ts -g '*.ts' | head -300Repository: rpamis/comet
Length of output: 50368
将 Classic 布局解析下沉到 domains/comet-classic。
app/commands/update.ts:1343-1357 直接实现了 classic.artifact_layout 优先级、文件系统检测和 legacy/docs 业务规则，这在 app/ 中违反了领域边界。把这些逻辑放入 Classic domain resolver，并在 domain tests 中覆盖显式布局、仅 openspec/、仅 docs/openspec/、双根和无根目录场景；update.ts 只保留编排调用。
🤖 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 `@app/commands/update.ts` around lines 1343 - 1357, Move the Classic
artifact-layout resolution currently using rawClassic and filesystem checks into
the resolver under domains/comet-classic, including explicit
classic.artifact_layout precedence and legacy/docs fallback rules. Add domain
tests covering explicit layout, only openspec/, only docs/openspec/, both roots,
and neither root; reduce update.ts to orchestration that calls the domain
resolver.
Source: Coding guidelines
Comment on lines
11775
to
11785
| function isWithin(parent, target) { | ||
| const relative = path19.relative(parent, target); | ||
| return relative === "" || !relative.startsWith("..") && !path19.isAbsolute(relative); | ||
| const relative = path17.relative(parent, target); | ||
| return relative === "" || !relative.startsWith("..") && !path17.isAbsolute(relative); | ||
| } | ||
| function requestTargetsAreControlOnly(projectRoot, nativeRoot, request) { | ||
| return request.targets.length > 0 && request.targets.every((targetPath) => { | ||
| const target = path19.resolve(projectRoot, targetPath); | ||
| const target = path17.resolve(projectRoot, targetPath); | ||
| if (!isWithin(projectRoot, target)) return true; | ||
| const relative = path19.relative(projectRoot, target).replaceAll("\\", "/"); | ||
| const relative = path17.relative(projectRoot, target).replaceAll("\\", "/"); | ||
| return relative === ".comet/config.yaml" || isWithin(nativeRoot, target); | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 定位 isWithin 的源实现，确认生成物与源码一致
rg -nP --type=ts -C4 'function isWithin\s*\(' domains app platform 2>/dev/null
rg -nP --type=ts -C4 'function isInside\s*\(' domains 2>/dev/nullRepository: rpamis/comet
Length of output: 7567
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== relevant native-hook-guard lines ==="
sed -n '1,80p' domains/comet-native/native-hook-guard.ts
sed -n '90,135p' domains/comet-native/native-hook-guard.ts
sed -n '168,230p' domains/comet-native/native-hook-guard.ts
echo "=== isWithin usages in source ==="
rg -n -C3 'isWithin\(' domains/comet-native/native-hook-guard.ts
echo "=== generated isWithin / requestTargetsAreControlOnly context ==="
sed -n '11758,11792p' assets/skills/comet/scripts/comet-hook-router.mjs
rg -n -C3 'requestTargetsAreControlOnly|externalTarget' assets/skills/comet/scripts/comet-hook-router.mjs | head -120
echo "=== Node path relative behavior probe ==="
node - <<'JS'
const {relative, isAbsolute} = require('path');
const cases = [
['project', 'project/foo'],
['project', 'project/.comet/config.yaml'],
['project', 'project/..foo'],
['project', 'project/foo/..foo'],
['project', 'project/natural-parent'],
['project', 'project/../..foo'],
['project', 'target'],
];
for (const [parent, target] of cases) {
const rel = relative(parent, target);
const current = rel === '' || (!rel.startsWith('..') && !isAbsolute(rel));
const proposed = rel === '' || (!isAbsolute(rel) && rel !== '..' && !rel.startsWith('..' + require('path').sep));
const normalizedDir = rel === '' || (!isAbsolute(rel) && rel !== '..' && !rel.startsWith('..' + require('path').sep) && !/^(?:\.\.|\.\\.)(?:\/|\\)/.test(rel));
console.log(JSON.stringify({parent,target,rel,current,proposed,cwdIsLikelyInParent: current && parent.startsWith(process.cwd()), normalizedDir }));
}
JSRepository: rpamis/comet
Length of output: 10803
修正 isWithin 的 startsWith("..") 判定，避免误把 ..foo 这类内部目标当作外部目标。
path.relative() 对 project/..foo 会返回以 .. 开头的相对路径，但当前判定会认为它在项目根之外；这会让 Hook Guard 将该写入目标记为 externalTarget 并放行，绕过非 build 阶段的拦截。建议与 isInside* 保持一致，只排除 .. 和 ..${sep} 前缀，并在 domains/comet-native/native-hook-guard.ts 修改后通过 pnpm build:entry-runtime 同步生成资产。
🔒 建议修正
function isWithin(parent, target) {
- const relative = path17.relative(parent, target);
- return relative === "" || !relative.startsWith("..") && !path17.isAbsolute(relative);
+ const relative = path17.relative(parent, target);
+ return relative === "" || !path17.isAbsolute(relative) && relative !== ".." && !relative.startsWith(`..${path17.sep}`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function isWithin(parent, target) { | |
| const relative = path19.relative(parent, target); | |
| return relative === "" || !relative.startsWith("..") && !path19.isAbsolute(relative); | |
| const relative = path17.relative(parent, target); | |
| return relative === "" || !relative.startsWith("..") && !path17.isAbsolute(relative); | |
| } | |
| function requestTargetsAreControlOnly(projectRoot, nativeRoot, request) { | |
| return request.targets.length > 0 && request.targets.every((targetPath) => { | |
| const target = path19.resolve(projectRoot, targetPath); | |
| const target = path17.resolve(projectRoot, targetPath); | |
| if (!isWithin(projectRoot, target)) return true; | |
| const relative = path19.relative(projectRoot, target).replaceAll("\\", "/"); | |
| const relative = path17.relative(projectRoot, target).replaceAll("\\", "/"); | |
| return relative === ".comet/config.yaml" || isWithin(nativeRoot, target); | |
| }); | |
| function isWithin(parent, target) { | |
| const relative = path17.relative(parent, target); | |
| return relative === "" || !path17.isAbsolute(relative) && relative !== ".." && !relative.startsWith(`..${path17.sep}`); | |
| } | |
| function requestTargetsAreControlOnly(projectRoot, nativeRoot, request) { | |
| return request.targets.length > 0 && request.targets.every((targetPath) => { | |
| const target = path17.resolve(projectRoot, targetPath); | |
| if (!isWithin(projectRoot, target)) return true; | |
| const relative = path17.relative(projectRoot, target).replaceAll("\\", "/"); | |
| return relative === ".comet/config.yaml" || isWithin(nativeRoot, target); | |
| }); | |
| } |
🤖 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-hook-router.mjs` around lines 11775 -
11785, Update isWithin to reject only the parent-directory path prefix (“..” or
“..” followed by the platform separator), not arbitrary paths beginning with
“..” such as “..foo”. Apply the same boundary logic used by the corresponding
isInside* helper, then regenerate the asset through the established
build:entry-runtime process so comet-hook-router.mjs stays synchronized.
Source: Coding guidelines
Comment on lines
+129
to
+139
| const rawClassic = snapshot.document?.value.classic; | ||
| const hasExplicitClassicLayout = | ||
| rawClassic !== null && | ||
| typeof rawClassic === 'object' && | ||
| !Array.isArray(rawClassic) && | ||
| (rawClassic as Record<string, unknown>).artifact_layout !== undefined; | ||
| const inferredClassicLayout: ClassicArtifactLayout = hasExplicitClassicLayout | ||
| ? (existing.classic?.artifact_layout ?? 'docs') | ||
| : classicAlreadyEnabled && (await fileExists(path.join(projectRoot, 'openspec'))) | ||
| ? 'legacy' | ||
| : 'docs'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
为已有 Classic 配置的布局推断补充直接测试。
当前测试只覆盖了显式 classic.artifact_layout 和无配置时的 legacy 证据；没有覆盖这里新增的“Classic 已启用、布局缺失、根级 openspec/ 存在”分支。请至少断言该状态解析为 legacy，并覆盖根级目录不存在时回退为 docs。As per coding guidelines, “测试范围必须与改动风险匹配”。
🤖 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 `@domains/comet-entry/init-workflow.ts` around lines 129 - 139, 在覆盖 init
workflow 布局推断的测试中，补充 classicAlreadyEnabled 且缺少 classic.artifact_layout
时的分支：当项目根目录存在 openspec/ 时断言解析为 legacy，并在该目录不存在时断言回退为
docs。复用现有测试中的文件系统模拟和布局解析入口，确保显式布局及其他已有场景保持不变。
Source: Coding guidelines
Comment on lines
+82
to
+84
| await expect(readClassicArtifactLayout(root)).resolves.toBe('docs'); | ||
| expect(classicLayoutPaths(root, 'docs').changesDir).toBe( | ||
| path.join(root, 'docs', 'openspec', 'changes'), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
同步运行时代码的默认值。
readClassicArtifactLayout() 当前仍以 legacy 作为缺失值回退，因此此断言会失败，并且直接调用该 API 时仍会选择根级 openspec/。请将 domains/comet-classic/classic-layout.ts 的回退值改为 docs。
建议修复
- return document.classic?.artifact_layout ?? 'legacy';
+ return document.classic?.artifact_layout ?? 'docs';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/domains/comet-classic/classic-layout.test.ts` around lines 82 - 84,
Update the fallback value used by readClassicArtifactLayout() in the classic
layout runtime code from legacy to docs, so missing configuration resolves to
docs and classicLayoutPaths() selects the docs/openspec location.
21 tasks
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
🎯 Scope
init,status,doctor,update)assets/skills/,assets/skills-zh/)assets/skills/comet/scripts/)🧪 Testing
pnpm buildpnpm lintpnpm run lint:architecturepnpm format:checkpnpm testpnpm test -- test/domains/comet-classic/comet-scripts.test.ts✅ Checklist
fix: handle project-scope initREADME.md,README-zh.md, orCONTRIBUTING.mdCHANGELOG.mdis updated when behavior changesassets/manifest.jsonand relevant tests👀 Notes for Reviewers
Summary by Sourcery
Remove Native signed-v2 creation authorization flow, update configuration defaults and CLI/docs around Native and Classic workflows, and bump release metadata to 0.4.0-beta.11.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Summary by CodeRabbit
New Features
docs/openspec/, while existing root-levelopenspec/artifacts are preserved.Removed
trust authorizecommand and the legacy signed creation-authorization protocol.Release
0.4.0-beta.11.