fix(sources): retarget shared imports to gominimal and regenerate locks#104
Merged
Merged
Conversation
The 8 sdd-* sources pinned shared/* at norrietaylor/spectacles@main, but this repo lives at gominimal/spectacles. The drift was masked while the two orgs' shared/ contents matched; once norrietaylor/main advanced (ADR 0012 fast-path), the committed locks fell behind and CI's lint drift gate started failing on main. Retargets every sdd-*.md imports: block from norrietaylor/spectacles to gominimal/spectacles and re-runs gh aw compile so the inlined locks come from this repo's own shared/ on @main. Pairs with #103 (which retargets the wrappers' uses: lines). After both land, scripts/quick-setup.sh still references norrietaylor in its installer sed pattern and header comment - follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughSwitch workflow imports from norrietaylor to gominimal, regenerate compiled prompts and lockfile metadata (heredoc IDs, safe-output-tools insertion, MCP tool fragments), and update embedded SDD interaction contract to document fast-path states and revised command semantics. ChangesSpectacles Source Migration and Fast-Path Interaction Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/sdd-execute-haiku.lock.yml (1)
891-897:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe misclassification reset targets the wrong lifecycle label.
Step 2 already moved the tracking issue from
sdd:fastpathtosdd:in-progressbefore implementation starts (Lines 777-790). By the time this escalation path runs,sdd:fastpathis no longer present, so a reset that removes onlysdd:fastpathcan leave the issue stuck insdd:in-progress. This reset description should clearsdd:in-progress(or otherwise describe the real transition) before returning tosdd:spec.🤖 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 @.github/workflows/sdd-execute-haiku.lock.yml around lines 891 - 897, The misclassification-reset text currently only removes sdd:fastpath but must also clear the lifecycle label sdd:in-progress before adding sdd:spec; update the escalation description so the reset removes sdd:in-progress (and sdd:fastpath if present), then adds sdd:spec and triggers the full pipeline with the stub spec as the starting point, ensuring the issue does not remain stuck in sdd:in-progress.
🤖 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 @.github/workflows/sdd-dispatch.lock.yml:
- Line 411: The `/dispatch` noop handling currently redirects fast-path
`sdd:in-progress` users to `/approve`; change the branching so the `/approve`
redirect is emitted only for `sdd:fastpath` and `sdd:fastpath-review`, and for
`sdd:in-progress` produce a noop comment that clearly states execution is
already running (no `/approve` suggestion). Locate the code that decides noop
comments for `/dispatch` (references: `/dispatch`, `sdd:ready`,
`sdd:in-progress`, `sdd:fastpath`, `sdd:fastpath-review`, `/approve`) and update
the conditional logic and message text accordingly so `sdd:in-progress` does not
reference `/approve`.
In @.github/workflows/sdd-triage.lock.yml:
- Around line 445-449: The workflow prompt must explicitly guard phase C tree
materialization against fast-path issues: update the source prompt text that
defines behavior for the `/approve` and `/dispatch` commands so that if the
tracking issue has label/state `sdd:fastpath` (or fast-path states like
`sdd:fastpath-review`/`sdd:in-progress`) the agent does NOT perform full
Unit/task tree materialization or advance to `sdd:ready`; instead it should
dispatch a single `sdd-execute-{tier}` per ADR 0012 and make `/dispatch` a noop
with an explanatory comment. Locate the strings mentioning `/approve`,
`/dispatch`, `sdd:fastpath`, `sdd-triage`, and ADR 0012 in the prompt source,
add the fast-path conditional/guard before phase C behavior, then recompile the
lockfile to regenerate .github/workflows/sdd-triage.lock.yml.
In @.github/workflows/sdd-validate.lock.yml:
- Around line 368-381: The workflow still advances any clean implementation pass
to the sdd:review label; update the prompt logic and the safe-output label
allowlist so that fast-path flows (labels sdd:fastpath and the fast-path
implementation from sdd:in-progress) transition directly to sdd:done instead of
sdd:review. Concretely, modify the state-advancement logic that currently maps
sdd:validate -> sdd:review to exclude issues carrying sdd:fastpath (and
sdd:fastpath-review) and add a branch mapping sdd:validate (fast-path case) or
sdd:in-progress+ sdd:fastpath -> sdd:done; also update the safe-output label
allowlist to not require sdd:review for fast-path runs, then recompile the
workflow so the new prompt and allowlist are applied.
---
Outside diff comments:
In @.github/workflows/sdd-execute-haiku.lock.yml:
- Around line 891-897: The misclassification-reset text currently only removes
sdd:fastpath but must also clear the lifecycle label sdd:in-progress before
adding sdd:spec; update the escalation description so the reset removes
sdd:in-progress (and sdd:fastpath if present), then adds sdd:spec and triggers
the full pipeline with the stub spec as the starting point, ensuring the issue
does not remain stuck in sdd:in-progress.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 69f94603-bf3a-4c70-bcc3-3e61a1f72f44
📒 Files selected for processing (16)
.github/workflows/sdd-dispatch.lock.yml.github/workflows/sdd-dispatch.md.github/workflows/sdd-execute-haiku.lock.yml.github/workflows/sdd-execute-haiku.md.github/workflows/sdd-execute-opus.lock.yml.github/workflows/sdd-execute-opus.md.github/workflows/sdd-execute-sonnet.lock.yml.github/workflows/sdd-execute-sonnet.md.github/workflows/sdd-review.lock.yml.github/workflows/sdd-review.md.github/workflows/sdd-spec.lock.yml.github/workflows/sdd-spec.md.github/workflows/sdd-triage.lock.yml.github/workflows/sdd-triage.md.github/workflows/sdd-validate.lock.yml.github/workflows/sdd-validate.md
…ispatch noop Addresses three CodeRabbit findings on PR #104 (one skipped as already-guarded). All fixes are in source .md prompts; the matching .lock.yml diff is regenerated for source-side fragments. The shared/ fragment change does not propagate into .lock.yml until merge (locks import shared/* via pinned-ref @main per ADR 0002/0004) - this is expected and propagates on the next compile after merge. - shared/sdd-interaction.md (finding 1): split the /dispatch noop semantics by lifecycle state. For a fast-path tracking issue at sdd:fastpath / sdd:fastpath-review the noop comment continues to point at /approve; for a fast-path sdd:in-progress the comment now states that execution is already running and /approve must not be used again. - sdd-validate (finding 3): add sdd:done to the add-labels allowlist and route the clean-implementation-boundary move to sdd:done on a fast-path feature (linked tracking issue carrying sdd:fastpath / sdd:fastpath-review or showing fast-path history), preserving sdd:in-progress -> sdd:review on a full-path feature. Updates the procedure, idempotence rule, summary, and verification section to match. Per ADR 0012, a fast-path feature never carries sdd:review. - sdd-execute-{haiku,opus,sonnet} (finding 4): reword the misclassification-escalation reset description so it is label-agnostic. Once this agent has flipped sdd:fastpath -> sdd:in-progress, an escalation /spec must clear sdd:in-progress, not sdd:fastpath; before that flip the reset still clears sdd:fastpath / sdd:fastpath-review. The description now says the reset removes whichever lifecycle label is currently present. Finding 2 (sdd-triage fast-path guard) skipped: wrappers/sdd-triage.yml already detects sdd:fastpath / sdd:fastpath-review on /approve and routes to a separate fastpath-approve job that dispatches sdd-execute, never the sdd-triage agent. /dispatch is handled by sdd-dispatch, not sdd-triage. The lock-file lines the reviewer cited are imported documentation (sdd-interaction.md), not sdd-triage agent behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
CodeRabbit findings — verification + fixesVerified each finding against current code; pushed
Note on finding 1 lock-file propagationSource fix lives in Findings 3 + 4 are agent-source edits, so their lock content is updated in this commit ( |
…s sdd:done The previous commit (772facf) added sdd:done to sdd-validate's add-labels allowlist and prose so fast-path features would skip sdd:review. That violated the single-writer invariant defined in scripts/lifecycle-states.yml: sdd-execute is the declared writer of sdd:done, and the lifecycle state-machine test fails fast on two writers for the same label. The safe-output allowlist test also flagged the new prose ("Add the next lifecycle label") as not matching the imperative "Add the X label" pattern, masking both sdd:review and sdd:done from the parser. Corrected approach for the same CodeRabbit finding (fast-path must not pass through sdd:review): on a fast-path feature this agent posts the findings comment but performs no lifecycle move. sdd-execute, as the declared writer of sdd:done, handles the sdd:in-progress -> sdd:done transition when the implementation PR merges (ADR 0012). The allowlist returns to [needs-human, sdd:review] and the prose returns to imperative "Add the sdd:review label" for the full-path branch only. Verified locally: - python3 scripts/test-lifecycle-state-machine.py - OK - python3 scripts/test-safe-output-allowlists.py - OK Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 21, 2026
norrietaylor
added a commit
that referenced
this pull request
May 21, 2026
chore: regenerate locks after #104 merge (unblock main lint)
6 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
sdd-*.mdsourceimports:fromnorrietaylor/spectacles/shared/*.md@maintogominimal/spectacles/shared/*.md@mainso the canonical source of shared fragments is this repo, not the legacy org.gh aw compile; commits the 8 regenerated.lock.ymlfiles alongside their sources.Root cause
The
lintworkflow'scompilejob runsgh aw compile --no-check-updatethen fails if any.lock.ymldrifts. CI started failing onmainafter fast-path content (ADR 0012) was added tonorrietaylor/spectacles@main'sshared/sdd-interaction.mdwithout the matching recompile-and-commit landing here. Pinning at the upstream org meant every push pulled fresh content gh-aw inlined into locks — locks committed before that change were stale.Reproduced locally on
main:Why this scope (not PR #103)
PR #103 retargets the wrapper-side
uses:refs. This PR retargets the source-sideimports:refs and ships the resulting lock regeneration. Disjoint file sets; either can merge first.Follow-up not in this PR
scripts/quick-setup.sh:10,232still referencesnorrietaylor/spectacles(installer header comment + sed pattern that rewrites consumer-sideuses:lines). Belongs with #103 or a dedicated installer PR.Test plan
lint/compilejob goes green on this PRleak-scan,docs,MCP smoke testremain greenlintonmainpush goes greengh aw compileon a fresh clone produces no drift🤖 Generated with Claude Code