fix: allow refreshing a completed design handoff after spec changes - #326

👋 Thanks for opening your first PR to Comet, @baobaolaodie.

Before review, please make sure the PR title follows Conventional Commits, for example fix: handle project-scope init, and that the checklist in the PR template is up to date.

🧪 The most useful local checks are:

pnpm build
pnpm lint
pnpm format:check
pnpm test

🧰 If your change touches assets/skills/comet/scripts/, please also check script portability across macOS, Linux, and Windows Git Bash. Avoid sed -i, support both sha256sum and shasum -a 256, and guard optional grep pipelines with || true.

✨ We appreciate the contribution and will take a look as soon as we can.

✅ PR template check passed.

Reviewer's Guide

Allow the Classic design handoff command to refresh a completed handoff when OpenSpec artifacts change, adjust the corresponding handoff shell script behavior, update tests to reflect the new behavior, and bump version metadata/docs.

File-Level Changes

Assessment against linked issues

Possibly linked issues

• fix: Classic design deadlock after Spec Patch #324: PR updates classic-handoff to refresh hashes on --write, resolving the stale handoff deadlock described in the issue.

No actionable comments were generated in the recent review. 🎉

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 @coderabbitai help to get the list of available commands.}

Please update the versions in package.json and package-lock.json to beta19, and consolidate the changelog entries from versions later than beta18 into beta19; this appears to be a merge error from a previous fix.

At the same time, you need to review the content of the issue again #324 ; a new comment has pointed out a new problem.

Please update the versions in package.json and package-lock.json to beta19, and consolidate the changelog entries from versions later than beta18 into beta19; this appears to be a merge error from a previous fix.

At the same time, you need to review the content of the issue again #324; a new comment has pointed out a new problem.

Thanks for the review. Version and changelog are now aligned per your feedback:

• package.json / package-lock.json / assets/manifest.json and the version-assertion tests now read 0.4.0-beta.19.
• The changelog entries newer than beta.18 (including the previous beta.20 entry) have been consolidated into a single [0.4.0-beta.19] entry, with the handoff fix added under ### Fixed.

I also reviewed issue #324 again as you asked. The new comment pointed out two follow-on problems, both fixed in this PR:

1. --write no longer reports success while leaving stale context files behind when the recorded hash was aligned by hand — the short-circuit now verifies the on-disk markdown reflects the current sources.
2. The design handoff can now be regenerated after the guard has advanced the phase to build, so a later Spec Patch no longer requires the COMET_FORCE_PHASE repair escape hatch.

Both fixes are covered by new tests and verified end-to-end locally.

Thanks for the follow-up fixes. The Build-phase handoff refresh is headed in the right direction: the workflow phase and Runtime step remain unchanged after the refresh. However, the stale-context check still has one gap that should be fixed.

handoffMarkdownIsCurrent() currently checks only whether the SHA256 of every existing source appears in the old Markdown. It does not verify the Markdown's internal Context hash, and it cannot detect sources that have been removed from OpenSpec.

This can be reproduced as follows:

1. Generate a handoff containing a delta spec.
2. Delete the entire delta spec.
3. Apply the workaround from the Issue by aligning handoff_hash with the new source hash.
4. Run comet handoff <change> design --write again.

The command still exits with status 0 and prints wrote, but the handoff Markdown is not rewritten and still contains the deleted spec.

Please pass the current contextHash into handoffMarkdownIsCurrent() and at minimum verify that the Markdown contains the exact - Context hash: ${contextHash} line, or validate the complete source path + SHA256 set. Please also add a regression test confirming that deleting a delta spec forces the context files to be regenerated.

Additionally, the Build recovery test currently sets phase: build through COMET_FORCE_PHASE. It would be better to enter Build through the real comet guard <change> design --apply path and assert that both the workflow phase and Runtime currentStep remain unchanged after refreshing the handoff. This would cover the actual recovery path reported in the Issue.

Thanks for the detailed review — I've closed the stale-context gap you identified.

handoffMarkdownIsCurrent() currently checks only whether the SHA256 of every existing source appears in the old Markdown. It does not verify the Markdown's internal Context hash, and it cannot detect sources that have been removed from OpenSpec. ... Please pass the current contextHash into handoffMarkdownIsCurrent() and at minimum verify that the Markdown contains the exact - Context hash: ${contextHash} line ... Please also add a regression test confirming that deleting a delta spec forces the context files to be regenerated. Additionally, the Build recovery test currently sets phase: build through COMET_FORCE_PHASE. It would be better to enter Build through the real comet guard <change> design --apply path ...

1. handoffMarkdownIsCurrent() now verifies the exact Context hash line (c4df953): it receives the current contextHash and requires - Context hash: ${contextHash} in the on-disk markdown before the short circuit may report success. Deleting a delta spec changes the computed hash, which no longer matches the marker in the old markdown, so --write regenerates the context pack instead of short-circuiting on an aligned recorded hash while keeping the deleted spec embedded. I confirmed every markdown flavor (compact, full, and beta spec-context.md) carries the Context hash marker, so the check holds across all modes.

2. Regression tests added, covering the full matrix of source-set changes rather than deletion alone:

   • deleting a delta spec forces context regeneration (your exact repro: handoff → delete spec → align handoff_hash → --write);
   • adding a delta spec forces context regeneration — the same gap on the other side, since the SHA256 loop alone cannot detect an added source either;
   • beta context_compression regenerates spec-context.md after a spec deletion, proving the fix holds for the beta markdown flavor too.

3. Build recovery test now enters Build through the real guard path: it records a Design Doc for the full workflow, runs comet guard <change> design --apply, and asserts phase=build and currentStep=full.build.plan. After the handoff refresh it asserts both remain unchanged — covering the actual recovery path reported in issue fix: Classic design deadlock after Spec Patch #324.

I verified the fix with a red/green check: without the Context hash verification, the deletion regression test fails exactly as you described (the stale markdown keeps the deleted spec); with the verification in place, the handoff/guard/runtime suites pass locally — classic-handoff.test.ts runs 10/10 green.

Heads-up on the current CI state: the Dashboard browser E2E job has timed out twice in a row at the exact same step, and it is unrelated to this PR's code changes.

Failure evidence

• Step: pnpm exec playwright install --with-deps chromium
• The run log shows apt-get update stalling against archive.ubuntu.com — Get:4/Get:5 ... InRelease [126 kB] at 08:26:33, then no further progress until ##[error]The operation was canceled. at 08:40:12.
• Annotation: The job has exceeded the maximum execution time of 15m0s (workflow timeout-minutes: 15).

Why it's environmental

• The E2E tests themselves never ran; the job died inside Playwright's --with-deps apt deps step.
• All other 10 jobs on this commit pass (Quality & coverage, Package install + Runtime smoke on all platforms, Node 22, Workflow lint, Eval static tests, CodeQL).
• Other concurrent PRs on this repo (feat/grok-platform, feat/issue-323-*, beta19) hit the same cancellations/failures in the same window, pointing to the shared ubuntu runner/apt mirror rather than this branch.

I cannot rerun the workflow myself as a fork contributor (gh run rerun requires admin access to this repository). Could you rerun the failed job once the apt/runner situation recovers, or let me know a preferred way to proceed? The code-side checks are all green locally and in CI.

Thanks for the follow-up fixes. Apart from the Dashboard CI, I found no new functional blockers, and the scenarios described in the related issue are now covered.

Before merging, please update the bilingual Skill documentation:

• comet-design should explain that the handoff must be regenerated after adding, modifying, or deleting a delta spec.
• comet-build should explain that during the Build phase, users can run comet handoff <change-name> design --write directly to refresh the handoff without reverting the current phase or step.

The Runtime already supports these behaviors, but the documentation has not been updated, so users or agents may not discover this recovery path. Once the documentation is synchronized, I think this PR is ready to merge apart from the Dashboard CI.

The unreachable else branch and the unused test env parameter are non-blocking cleanup items and can be addressed separately.

Thanks for the review. The bilingual Skill documentation is now synchronized, and I also took the opportunity to address the two non-blocking cleanup items you listed — all in commits 5c6e053 (docs) and 9488b09 (refactor, after rebasing onto the merged #338/master update):

1. comet-design (Chinese + English): Step 3 now states the handoff must be regenerated after a delta spec is added, modified, or removed, and explains that deleting a delta spec otherwise leaves the removed spec embedded in the stale handoff pack, so the design guard's traceability check cannot pass.
2. comet-build (Chinese + English): Step 4 now documents that during Build you can run comet handoff <change-name> design --write directly to refresh the handoff without reverting the current phase or Runtime step.
3. Cleanup: removed the unreachable else branch in the stale-handoff check (the usage guard above already guarantees --write), and dropped the unused env parameter from the test run() helper.

The CHANGELOG update is recorded under the same 0.4.0-beta.19 entry. Local checks pass: classic-handoff 10/10, runtime/guard suites, and lint. Ready to merge apart from the Dashboard E2E CI, which remains an upstream apt-network timeout unrelated to this PR — happy to have it rerun once the runner recovers.

Thanks for the update. I reviewed the latest HEAD (9488b090). The implementation now fully addresses issue #324, and I found no remaining code issues. There are only three documentation corrections left:

1. comet-build/SKILL.md states that both the Design and Build guards validate the latest handoff. Only the Design guard performs these checks. Please describe regeneration as rebuilding the handoff from the current OpenSpec artifacts without changing the phase or Runtime currentStep.

2. comet-design/SKILL.md states that deleting a delta spec causes the traceability check to fail. The actual blocker is that the recorded handoff_hash no longer matches the current OpenSpec artifacts. Please adjust the wording accordingly.

3. Please remove the separate Classic handoff refresh documentation Changelog entry. The Fixed entry already describes the complete user-visible behavior, so the documentation-sync entry is redundant.

After these wording-only corrections, I think the PR is ready to merge.