feat(skills): propose /opsx:update planning-artifact update skill#1278
feat(skills): propose /opsx:update planning-artifact update skill#1278clay-good wants to merge 12 commits into
Conversation
…te + cohesive audit Dogfooded OpenSpec proposal for the missing first-class "update" action: a /opsx:update workflow that propagates an edit to one artifact across its downstream dependents (targeted mode) or audits a whole change for stale/ incoherent artifacts (audit mode) — driven by the schema's artifact graph, never hardcoded filenames, editing planning artifacts only (never code). - artifact-graph: expose reverse-dependency queries (getDependents/getDownstream) + a requires-edge mtime staleness signal (the engine already builds the dependents map at graph.ts:98 and discards it). - cli-artifact-workflow: surface requires/dependents/stale on `openspec status --json` and add a `--impact <artifact>` downstream-revisit-order selector. - opsx-update-skill: the user-facing /opsx:update command (targeted + audit). Supersedes the proposal-only stub add-artifact-regeneration-support. Addresses the cluster Fission-AI#1188/Fission-AI#705/Fission-AI#673/Fission-AI#247 (closes), Fission-AI#694/Fission-AI#684/Fission-AI#618 (answers), and is graph-driven to avoid the Fission-AI#777/Fission-AI#666 hardcoded-artifact-pattern bug class. Validates clean under `openspec validate --strict`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Changes/opsx:update Workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…bish review) Reframe per the steer "more deterministic and grounded in reality": - Deterministic spine: the CLI computes the impact set (which downstream artifacts to revisit, in build order, with paths) as a pure function of schema edges + filesystem. The agent only rewrites prose. Grounded in real APIs already present: getUnlockedArtifacts (direct dependents), getBuildOrder (order), resolveArtifactOutputs (paths); reverse map built at graph.ts:82-87. - Replace fragile mtime staleness with a newline-normalized SHA-256 content digest (reproducible cross-platform). Drift = upstream digest vs recorded baseline; no baseline => "unknown", never a false positive. mtime and pure-git rejected with rationale; digest ledger is a separable, optional layer. - Explicit determinism boundary decision (CLI decides files/order/drift; agent rewrites). Skill MUST source the file list/order from `openspec status --impact`, never compute it. - Corrected all code citations to verified lines (graph.ts:82-87, instruction-loader.ts:366/429, status.ts); noted Fission-AI#1277's coverage helpers are not in this branch's base (coordinate, don't reuse). - Specs updated: artifact-graph Content Digest requirement; cli status digest + deterministic impact ordering; skill determinism + baseline-aware audit. tasks add digest/determinism/cross-platform tests + optional ledger section. Still validates clean under `openspec validate add-update-workflow --strict`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ame refs - Digest ledger tracks DIRECT upstream digests; document that transitive drift emerges hop-by-hop as downstream is reconciled (no transitive bookkeeping). - Ground audit's no-baseline structural facts on signals available in this branch (missing/empty output, blocked/incomplete); capability-coverage is an add-on only when Fission-AI#1277's validateChangeCapabilityCoverage is present. - Add the "update revises only existing downstream; defer not-yet-created ones to /opsx:continue" rule across proposal/design/specs/tasks; impact entries now carry existence/status. - Note artifact-level (not file-level) granularity and that getDownstream terminates by the schema's acyclic guarantee. - Remove direct personal references from the docs. Validates clean under `openspec validate add-update-workflow --strict`; 10 deltas. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sign After a comprehensive sweep of open issues, PRs, and discussions, grounded the proposal in the complete adjacent landscape and answered the open design questions the cluster raises: - Fission-AI#783 (Cross-artifact quality review before apply) is now a primary Closes: it IS audit mode. Answer its open "new skill vs. extend validate" question via the determinism split — deterministic checks (drift/completeness/coverage) are CLI/validate-shaped; the semantic cross-artifact review is the skill. Added a skill spec scenario for the Fission-AI#783 patterns (scope contradiction, spec gap, duplication). - Discussion Fission-AI#1206 ("refine proposal now?") + prior-art PR Fission-AI#372: official answer is /opsx:update. - New design Decision 8 (command family): delineate /opsx:update from /opsx:clarify (Fission-AI#702, within-artifact), /opsx:review (Fission-AI#1251, plan-vs-code), and verify; /opsx:update consolidates update+regen+refine into one action, addressing skill-sprawl (Fission-AI#1263, Fission-AI#783). - Reuse, don't reinvent: audit's empty/incomplete check reuses Fission-AI#1098's artifactOutputComplete (same outputs.ts the digest helper lives in); capability coverage reuses Fission-AI#1277's validateChangeCapabilityCoverage. - New open questions: surface deterministic coherence in `validate` for a CI gate (Fission-AI#783-B, Fission-AI#829); naming reconciliation with Fission-AI#783's /opsx:refine. - Confirmed add-update-command* branches are the `openspec update` tool-file refresh (not artifact update) — no collision. Validates clean under --strict; 10 deltas; all relative links resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…in scope Per review steer, every open question is now a committed happy-path decision so build-out has no dangling forks, and the deterministic drift baseline is pulled into scope (it is what makes audit-mode drift deterministic vs. agent-guessed): - Digest ledger IN SCOPE (design Decision 3): per-artifact DIRECT upstream digests in ChangeMetadataSchema, written by a deterministic `openspec status --record`; pre-existing changes (no baseline) degrade to drift `unknown` + structural checks. Generating-flow auto-recording stays optional (graceful). - cli-artifact-workflow spec: folded drift into the digest requirement (record baseline / drift vs baseline / unknown-without-baseline) — stays at 10 deltas. - opsx-update-skill spec: skill records baseline via `--record` after each confirmed edit, so audits clear once reconciled. - Replaced "## Open Questions" with "## Decisions resolved": ledger in scope; targeted entry baseline-aware; apply stays standalone (points to update on drift); cross-change (Fission-AI#247), continue/ff de-hardcoding (Fission-AI#777), and validate CI-gate (Fission-AI#783-B/Fission-AI#829) are named follow-ups, not deferrals of the core feature; /opsx:update kept as the umbrella name. - Migration Plan + Capabilities + Impact + tasks updated; status JSON gains `drift`, CLI gains `--record`. Re-synced with upstream main (0 behind). Validates clean under --strict; 10 deltas; all links resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y, edge gaps Stress-tested every claim against live source and fixed the soft spots: - Cross-OS digest determinism (real bug): resolveArtifactOutputs (outputs.ts:34) sorts ABSOLUTE paths via .sort(), which differs by OS — so a multi-file glob artifact (specs/**/*.md) would hash differently on Windows vs POSIX. Digest now specified to order files by change-relative forward-slash path and hash relpath+content. Added spec scenarios (cross-platform glob stability; rename changes digest) and a cross-OS test task. - Read-only status invariant: moved baseline recording OFF `openspec status` (a read command silently mutating the drift reference is a footgun) to a dedicated `openspec reconcile` write verb. Updated spec, skill, design, impact, capabilities, tasks; reconciled the "no new verb" claims. - Edge case: missing upstream at record time is stored as an explicit `absent` marker so later creating it registers as drift (spec scenario added). - Edge case: coherent change yields no edits (clean-path scenario). - Grounding fixes: continue-change hardcoded block is duplicated (skill 103-112 + command 225-234) — both must be fixed in the Fission-AI#777 follow-up; verified no content-hash util exists. - Fixed two stale claims the layered edits left: the Impact digest bullet (concatenation→relative-path) and the naming-boundary line. Validates clean under --strict; 10 deltas (4+3+3), 44 scenarios; all links resolve; re-synced with upstream main (0 behind); issue/PR/discussion sweep re-run, no new items. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ission-AI#880 Grounded the surface so an implementer builds it without guessing, and added proportionate forward-compatibility: - New design "Data contracts" section with exact shapes: extended ArtifactStatus (requires/dependents/digest/drift/driftFrom — additive to the real interface at instruction-loader.ts:120), the --impact response, and the `.openspec.yaml` baselines ledger. All additive; nothing existing changes type. - Digest scheme tag (`sha256-relpath-v1:`) + forward-compat: drift compares only same-scheme digests; an unrecognized/older scheme reports `unknown` rather than silently mis-comparing — re-reconcile restores it. Added a cli spec scenario and tasks for it. - Grounded the ledger write: there is no central change-metadata writer today (change-metadata/index.ts only re-exports schema), so reconcile does a safe read-modify-write of .openspec.yaml mirroring the store's parse/serialize/writeStoreMetadataState pattern (foundation.ts). - Coverage: re-swept; folded Fission-AI#880 (/opsx:validate code-vs-living-specs) into the plan-vs-code delineation alongside Fission-AI#1251/Fission-AI#1073. Main unchanged (546224e); all citations still valid. Validates clean under --strict; 10 deltas; links resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alfred-openspec
left a comment
There was a problem hiding this comment.
This is the right shape for the proposal: graph impact and drift belong in deterministic CLI data, while the skill only does confirmed planning-artifact rewrites. Main implementation note is to ship this in layers, impact set first and digest-ledger audit second, so the stateful reconciliation piece does not block the core update workflow.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
openspec/changes/add-update-workflow/design.md (2)
99-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify scheme version comparison in forward-compat design.
The data contracts specify that digests compare "only same-scheme" and that "unrecognized or older" schemes report
unknown. However, the design doesn't define how "older" is determined — by exact string inequality, parsed version number, or some other ordering. For a deterministic system, explicit comparison rules matter:Is the intent that only exact string matches are comparable, and any scheme tag that doesn't match the current one exactly is treated as
unknown? Or is there a defined ordering (e.g.,v1 < v2)?Recommend stating explicitly: "Only digests with the exact same scheme tag as the current canonicalization are compared; any mismatch (including different version suffixes) is treated as
unknown." This eliminates ambiguity and matches the deterministic philosophy.🤖 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 `@openspec/changes/add-update-workflow/design.md` around lines 99 - 137, The forward-compat section in the data contracts leaves scheme comparison ambiguous, especially around what “older” means. Update the `Digest scheme tag & forward-compat` wording to explicitly define comparison rules for `ArtifactStatus.digest`, `baselines.scheme`, and drift handling: only exact scheme-tag matches against the current canonicalization are comparable, and any mismatch or unrecognized tag must yield `unknown`. Keep the guidance aligned with `reconcile` and the `drift`/`driftFrom` behavior so implementers have one deterministic rule.
9-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the update flow schema-driven
src/core/templates/workflows/continue-change.tsstill hardcodesproposal → specs → design → tasksin both the skill and command templates, despite the guardrail to use the schema’s artifact sequence. Remove both copies so the new skill follows the CLI JSON ids/edges instead.🤖 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 `@openspec/changes/add-update-workflow/design.md` around lines 9 - 10, `continue-change.ts` still hardcodes the `proposal → specs → design → tasks` workflow in both the skill and command template copies, which conflicts with the guardrail to use the schema’s artifact sequence. Remove both duplicated prose blocks and update the template logic so it reads artifact ids and edges from the CLI JSON instead of assuming fixed names or ordering, keeping the behavior aligned with the schema-driven workflow.openspec/changes/add-update-workflow/specs/opsx-update-skill/spec.md (1)
92-111: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClarify baseline recording behavior when the user rejects a revision.
The spec states that baseline is recorded "after the skill has applied and confirmed a revision" (line 109). Consider adding an explicit scenario for when the user rejects a proposed revision, to ensure implementers do not record a baseline for an unapplied edit:
+#### Scenario: Rejected revision does not record baseline + +- **WHEN** the user rejects a proposed revision for an artifact +- **THEN** no baseline is recorded for that artifact +- **AND** the artifact remains in its previous drift state🤖 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 `@openspec/changes/add-update-workflow/specs/opsx-update-skill/spec.md` around lines 92 - 111, Add an explicit scenario under the User-Confirmed Incremental Application requirement to cover rejection of a proposed revision: when the user does not confirm the artifact change, the `/opsx:update` skill must not write the change and must not record the drift baseline. Update the spec near the existing Confirm before writing and Records the drift baseline after an applied edit scenarios so implementers clearly distinguish confirmed/applied edits from rejected proposals.openspec/changes/add-update-workflow/specs/cli-artifact-workflow/spec.md (1)
18-64: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDefine drift semantics for root artifacts with no upstream dependencies.
The spec does not specify how drift behaves for artifacts that have no direct upstreams (root artifacts in the dependency graph). With no upstreams to compare, drift is arguably always
clean(nothing to drift against) orunknown(no meaningful baseline). Consider adding an explicit scenario:+#### Scenario: Root artifact with no upstreams + +- **WHEN** an artifact has no direct upstream dependencies +- **THEN** its drift status is `clean` (nothing to drift against) +- **AND** recording a baseline stores an empty upstreams map🤖 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 `@openspec/changes/add-update-workflow/specs/cli-artifact-workflow/spec.md` around lines 18 - 64, The drift semantics for root artifacts with no direct upstream dependencies are undefined in the current spec, so add an explicit scenario in the content-digest/drift section of cli-artifact-workflow/spec.md covering this case. Update the Requirement or scenarios around openspec status --json and baseline recording to state whether a root artifact with no upstreams reports drift as clean or unknown, and ensure the wording aligns with the existing drift states used by the status JSON model.openspec/changes/add-update-workflow/specs/artifact-graph/spec.md (1)
22-45: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClarify deterministic tie-breaking in diamond scenario.
The diamond scenario states "B and C before D" but does not specify an ordering between B and C. The downstream consumer (
graph.ts:68-113) uses sorted queues for determinism. Consider adding explicit language that when multiple artifacts are ready simultaneously, they are ordered lexicographically by id (or by some other stable rule) to ensure cross-platform determinism.#### Scenario: Diamond downstream order - **WHEN** artifacts form a diamond (A → B, A → C, B → D, C → D) and getDownstream("A") is called -- **THEN** the result includes B and C before D +- **THEN** the result includes B and C before D +- **AND** when B and C are both ready at the same step, they are ordered lexicographically by artifact id for determinism🤖 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 `@openspec/changes/add-update-workflow/specs/artifact-graph/spec.md` around lines 22 - 45, Clarify the downstream ordering rule in the Transitive Downstream Query spec so the diamond scenario is deterministic when B and C are both ready before D. Update the requirement/scenario text in the artifact graph spec to state the tie-breaker used by graph.ts’s getDownstream logic, such as lexicographic ordering by artifact id, so the topological order is fully specified across platforms.openspec/changes/add-update-workflow/tasks.md (1)
17-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer symbol references over fragile line numbers in task cross-references.
Task 3.2 references
src/cli/index.ts:488as the registration point. Line numbers become stale quickly; prefer referencing by command registration symbol or pattern (e.g.,status command registration in src/cli/index.ts) so the task remains valid after unrelated edits.- (`src/commands/workflow/status.ts`, registered at `src/cli/index.ts:488`) + (`src/commands/workflow/status.ts`, registered alongside other workflow commands in `src/cli/index.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 `@openspec/changes/add-update-workflow/tasks.md` around lines 17 - 24, Task 3.2 hard-codes a line number for the status command registration, which will drift as the file changes. Update the task text to reference the `status` command registration symbol/pattern in `src/cli/index.ts` (and `StatusOptions` / `workflow status` for context) instead of a specific line, so the cross-reference stays stable after unrelated edits.
🤖 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 `@openspec/changes/add-update-workflow/specs/artifact-graph/spec.md`:
- Around line 46-79: The missing-output digest rule is inconsistent with the CLI
spec, so update the Artifact Content Digest requirement and the Missing Output
scenario to use one explicit representation everywhere (either omit digest or
emit null) and mirror that same wording in the CLI spec. Also extend Artifact
Content Digest with a scheme/version prefix requirement so digests carry an
identifier such as a canonicalization version tag, making future hashing changes
detectable. Keep the wording aligned across the affected spec sections so
implementers have one unambiguous contract.
In `@openspec/changes/add-update-workflow/tasks.md`:
- Around line 25-33: Task 4.4 references the “Update vs. Start Fresh” heuristic,
but the heuristic is not defined anywhere in this scope. Add a clear reference
in the tasks to the canonical document or spec section that defines this
heuristic, or inline the formal rule in the `4.4 Encode the intent-change guard`
item so the `/opsx:update` guardrail can point to an exact source. Keep the
wording aligned with the surrounding `update-change.ts` /
`getOpsxUpdateCommandTemplate()` workflow guidance.
---
Nitpick comments:
In `@openspec/changes/add-update-workflow/design.md`:
- Around line 99-137: The forward-compat section in the data contracts leaves
scheme comparison ambiguous, especially around what “older” means. Update the
`Digest scheme tag & forward-compat` wording to explicitly define comparison
rules for `ArtifactStatus.digest`, `baselines.scheme`, and drift handling: only
exact scheme-tag matches against the current canonicalization are comparable,
and any mismatch or unrecognized tag must yield `unknown`. Keep the guidance
aligned with `reconcile` and the `drift`/`driftFrom` behavior so implementers
have one deterministic rule.
- Around line 9-10: `continue-change.ts` still hardcodes the `proposal → specs →
design → tasks` workflow in both the skill and command template copies, which
conflicts with the guardrail to use the schema’s artifact sequence. Remove both
duplicated prose blocks and update the template logic so it reads artifact ids
and edges from the CLI JSON instead of assuming fixed names or ordering, keeping
the behavior aligned with the schema-driven workflow.
In `@openspec/changes/add-update-workflow/specs/artifact-graph/spec.md`:
- Around line 22-45: Clarify the downstream ordering rule in the Transitive
Downstream Query spec so the diamond scenario is deterministic when B and C are
both ready before D. Update the requirement/scenario text in the artifact graph
spec to state the tie-breaker used by graph.ts’s getDownstream logic, such as
lexicographic ordering by artifact id, so the topological order is fully
specified across platforms.
In `@openspec/changes/add-update-workflow/specs/cli-artifact-workflow/spec.md`:
- Around line 18-64: The drift semantics for root artifacts with no direct
upstream dependencies are undefined in the current spec, so add an explicit
scenario in the content-digest/drift section of cli-artifact-workflow/spec.md
covering this case. Update the Requirement or scenarios around openspec status
--json and baseline recording to state whether a root artifact with no upstreams
reports drift as clean or unknown, and ensure the wording aligns with the
existing drift states used by the status JSON model.
In `@openspec/changes/add-update-workflow/specs/opsx-update-skill/spec.md`:
- Around line 92-111: Add an explicit scenario under the User-Confirmed
Incremental Application requirement to cover rejection of a proposed revision:
when the user does not confirm the artifact change, the `/opsx:update` skill
must not write the change and must not record the drift baseline. Update the
spec near the existing Confirm before writing and Records the drift baseline
after an applied edit scenarios so implementers clearly distinguish
confirmed/applied edits from rejected proposals.
In `@openspec/changes/add-update-workflow/tasks.md`:
- Around line 17-24: Task 3.2 hard-codes a line number for the status command
registration, which will drift as the file changes. Update the task text to
reference the `status` command registration symbol/pattern in `src/cli/index.ts`
(and `StatusOptions` / `workflow status` for context) instead of a specific
line, so the cross-reference stays stable after unrelated edits.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d39db6b-11f6-42b7-9ffe-74007d8814fc
📒 Files selected for processing (7)
openspec/changes/add-update-workflow/.openspec.yamlopenspec/changes/add-update-workflow/design.mdopenspec/changes/add-update-workflow/proposal.mdopenspec/changes/add-update-workflow/specs/artifact-graph/spec.mdopenspec/changes/add-update-workflow/specs/cli-artifact-workflow/spec.mdopenspec/changes/add-update-workflow/specs/opsx-update-skill/spec.mdopenspec/changes/add-update-workflow/tasks.md
| ### Requirement: Artifact Content Digest | ||
|
|
||
| The system SHALL compute a deterministic content digest for an artifact from its output file(s), such that the same content yields the same digest on every run and on every platform. For an artifact with multiple output files (a glob), the digest SHALL be computed over the files ordered by their change-relative path expressed with forward slashes, and SHALL incorporate each file's relative path together with its content, so that the digest is stable regardless of the operating system's absolute-path sorting, separators, or drive letters. Content SHALL be line-ending-normalized (CRLF to LF) before hashing so that otherwise-identical content produces an identical digest regardless of encoding. | ||
|
|
||
| #### Scenario: Identical content yields identical digest | ||
|
|
||
| - **WHEN** an artifact's output content is unchanged between two computations | ||
| - **THEN** the digest is identical | ||
|
|
||
| #### Scenario: Changed content yields a different digest | ||
|
|
||
| - **WHEN** an artifact's output content changes | ||
| - **THEN** the digest changes | ||
|
|
||
| #### Scenario: Line endings do not affect the digest | ||
|
|
||
| - **WHEN** the same content is encoded with CRLF on one platform and LF on another | ||
| - **THEN** the digest is identical on both | ||
|
|
||
| #### Scenario: Glob output digest is stable across platforms | ||
|
|
||
| - **WHEN** an artifact generates a glob pattern (e.g. `specs/**/*.md`) with multiple files | ||
| - **THEN** the digest orders the files by change-relative forward-slash path before hashing | ||
| - **AND** the digest is identical on Windows and POSIX for the same file set and contents | ||
|
|
||
| #### Scenario: Renaming a file within a glob changes the digest | ||
|
|
||
| - **WHEN** a file in a glob artifact is renamed or moved (same total content, different relative path) | ||
| - **THEN** the digest changes, because the relative path is part of the hash | ||
|
|
||
| #### Scenario: Missing output has no digest | ||
|
|
||
| - **WHEN** an artifact's output does not exist on disk | ||
| - **THEN** no digest is reported for that artifact |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Align missing-output digest representation with CLI spec.
The artifact-graph spec says "no digest is reported" (line 78) while the CLI spec allows "omits digest (or reports it as null)" (line 35). Choose one representation and apply it consistently across both specs to avoid implementation ambiguity.
Additionally, the tasks.md (2.1) mentions digest scheme prefixes (e.g., sha256-relpath-v1:) for forward-compatibility, but this requirement does not mention scheme tags. Consider adding a requirement that digests include a scheme/version prefix so that future canonicalization changes are detectable rather than silently mis-compared.
🤖 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 `@openspec/changes/add-update-workflow/specs/artifact-graph/spec.md` around
lines 46 - 79, The missing-output digest rule is inconsistent with the CLI spec,
so update the Artifact Content Digest requirement and the Missing Output
scenario to use one explicit representation everywhere (either omit digest or
emit null) and mirror that same wording in the CLI spec. Also extend Artifact
Content Digest with a scheme/version prefix requirement so digests carry an
identifier such as a canonicalization version tag, making future hashing changes
detectable. Keep the wording aligned across the affected spec sections so
implementers have one unambiguous contract.
There was a problem hiding this comment.
This is looking decent, few comments below. I think the main thing to check here is if we're doing too much and if there's a simpler approach?
Could we for example just make it work by using the existing status command?
Coding agents tend to over-complicate things and are not very good at understanding agentic workflows, particularly skills.
I'd maybe see if this proposal can be simplified and what's actually needed to delivery the feature. - Maybe a good way to do this is to work backwards here, by thinking of what should go into the update skills file - whats the minimal instructions sets (both in terms of amount of tokens + commands the agent has to run). If you had to manually write the update skill what would be in there?
| ### Modified Capabilities | ||
|
|
||
| - `artifact-graph`: The graph exposes reverse-dependency queries (direct `dependents` and transitive, topologically-ordered `downstream`) and a deterministic, newline-normalized content `digest` per artifact — the primitives an update/audit traversal needs, all schema-agnostic and reproducible across platforms. | ||
| - `cli-artifact-workflow`: `openspec status --json` includes per-artifact dependency edges (`requires`, `dependents`), a content `digest`, and a deterministic drift signal (current upstream digest vs. recorded baseline, or `unknown` when none); a new `--impact <artifact>` selector returns the downstream revisit set in build order (with paths and digests), and a dedicated read/write split keeps `status` read-only while a separate `openspec reconcile` op writes the digest baseline — so skills consume deterministic graph structure as data instead of hardcoding artifact names. |
There was a problem hiding this comment.
What exactly would reconcile cmd do here? My preference for this is the agent can get back the current status of the change to figure out what else to be updated.
I wouldn't make it too complex to begin with.
The skill should mainly be something along the lines of:
- Understand the user request
- Run command to get status of what artifacts exist:
- Go and read those artifacts to check and see if anything else needs to get updated
^ A bit generalised but should be mainly this. In general we want to try and introduce as a little code as possible -> Only when there's a defined need for it
There was a problem hiding this comment.
Addressed in the b2f8639 rework: the reconcile op (and the digest/ledger machinery behind it) is gone entirely. The skill now does exactly the loop you describe — understand the request, openspec status --change <id> --json for what exists and where, read those artifacts, propose what else needs updating. Zero new commands.
| - `src/core/artifact-graph/graph.ts` — expose `getDependents(id)` (promotes the reverse-adjacency loop at lines 82-87, and the existing `getUnlockedArtifacts` logic) and `getDownstream(id)` (transitive, ordered by the existing `getBuildOrder`); both throw on unknown id. | ||
| - `src/core/artifact-graph/outputs.ts` (new digest helper) — `artifactDigest(changeDir, generates)`: from `resolveArtifactOutputs`, order files by their **change-relative forward-slash path** (not the OS absolute-path `.sort()` order, which differs across platforms), then SHA-256 over each file's relative path + newline-normalized (CRLF→LF) content; absent output → no digest. (No content-hash utility exists in the repo today; only `crypto.randomUUID` in telemetry.) | ||
| - `src/core/artifact-graph/instruction-loader.ts` (`formatChangeStatus`, ~397-453) — add `requires`, `dependents`, `digest` to each `ArtifactStatus`; the build-order sort at line 429 already gives revisit order. | ||
| - `src/commands/workflow/status.ts` (`StatusOptions`, `statusCommand`) + `src/cli/index.ts:488` — add the `--impact <artifact>` option returning the ordered downstream set (paths + digests); error on unknown artifact id; default human-readable output unchanged. |
There was a problem hiding this comment.
hmm I wonder how this would work. I'm not sure if downstream only is right approach here.
for example, this is the current technical graph
proposal -> specs -> design -> tasks
If design is updated there's a possibility that the proposal needs to change too
There was a problem hiding this comment.
You were right — addressed in b2f8639. Coherence is now explicitly bidirectional: the proposal calls out that editing design can require revising proposal, the spec has an "upstream artifact may be revised" scenario, and design Decision 1 rejects a downstream-only --impact primitive precisely because it encodes the wrong model.
…kill Rework per @TabishB review (PR Fission-AI#1278): the proposal over-built. Drop the deterministic-spine machinery and lean on the existing status command. - Cut the reverse-dependency graph API (getDependents/getDownstream), SHA-256 content digests, the .openspec.yaml baseline ledger, the `openspec reconcile` write op, the drift report, and `status --impact`. Removes the artifact-graph and cli-artifact-workflow spec deltas. - Reframe propagation as bidirectional coherence (editing design can require revising proposal), not downstream-only. - Center the feature on one thin skill over the existing `openspec status` / `openspec list`; design now sketches the actual minimal skill instruction body ("written by hand"). - v1 adds no new CLI/graph/schema code: just update-change.ts + wiring. Validates clean: `openspec validate add-update-workflow --strict`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @TabishB — you were right, it was doing too much. I reworked it to do exactly what you described and pushed (b2f8639): work off the existing Net result: −476 / +161 lines. The whole feature is now one thin skill. Simpler approach using the existing "What would reconcile do?" Gone. The skill does what you sketched: understand the request → run Working backwards from the skill file. Downstream-only was wrong. Agreed — if Guardrails kept: planning artifacts only (never code; hands off to If a deterministic staleness signal is ever genuinely needed, the smallest first step is exposing the schema's |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openspec/changes/add-update-workflow/design.md (1)
30-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifier to fenced code block.
The skill body code block lacks a language specifier, triggering
markdownlintMD040. Usetextormarkdownif no syntax highlighting is desired.-``` +```text🤖 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 `@openspec/changes/add-update-workflow/design.md` around lines 30 - 63, The fenced code block in the skill body is missing a language specifier, causing the markdown lint warning. Update the affected fenced block in the design artifact to include an explicit specifier such as text or markdown, and keep the surrounding content unchanged so the planning guidance remains coherent.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In `@openspec/changes/add-update-workflow/design.md`:
- Around line 30-63: The fenced code block in the skill body is missing a
language specifier, causing the markdown lint warning. Update the affected
fenced block in the design artifact to include an explicit specifier such as
text or markdown, and keep the surrounding content unchanged so the planning
guidance remains coherent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e058237e-6ae9-4037-a9fa-66fb044f419a
📒 Files selected for processing (4)
openspec/changes/add-update-workflow/design.mdopenspec/changes/add-update-workflow/proposal.mdopenspec/changes/add-update-workflow/specs/opsx-update-skill/spec.mdopenspec/changes/add-update-workflow/tasks.md
✅ Files skipped from review due to trivial changes (1)
- openspec/changes/add-update-workflow/tasks.md
alfred-openspec
left a comment
There was a problem hiding this comment.
Thanks for the simplification. This is much closer to the right shape, but I found one blocker before approving: the proposal now depends on openspec status --change <id> --json returning artifactPaths / resolved paths, and current status --json does not include that field. I checked the PR checkout with openspec status --change add-update-workflow --json; it returns changeName, schemaName, isComplete, applyRequires, and artifacts[] with id, outputPath, status only.
That breaks the core premise that v1 can be implemented with no CLI/status changes while still writing only to resolved artifact paths, especially for glob artifacts like specs/**/*.md. Please either scope the minimal additive status field needed for resolved/existing paths, or update the skill design to resolve paths through an existing command that actually returns them. After that, I think the thin-skill direction is approvable.
…Paths Address @alfred-openspec's review: the skill's write target was described loosely as "resolved paths." Make it precise across proposal/design/spec/tasks: - `openspec status --json` already returns everything the skill needs, in the top-level `artifactPaths` map — `resolvedOutputPath` and `existingOutputPaths` per artifact. No new CLI field is required. - The skill edits `existingOutputPaths` (the concrete, glob-expanded files) and never writes to `resolvedOutputPath`, which for a glob artifact like `specs/**/*.md` remains the glob pattern rather than a real file. - Add spec scenarios for editing a glob artifact's concrete files and for deferring a brand-new file under a glob artifact to `/opsx:continue`. - Tighten the cross-platform scenario and add a template test (3.4) asserting the write target is `existingOutputPaths`, not a glob `resolvedOutputPath`. Validates clean under `openspec validate add-update-workflow --strict`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — you're right that the write target needed pinning down, and I've made it explicit. One clarification on the field itself, then what I changed.
That said, you pointed at a real bug in how I described it:
Net: no new CLI field needed — the data's already there — and the write contract is now unambiguous for globs. Validates clean under |
| Two guardrails make it the command the cluster asked for: | ||
|
|
||
| - **Planning artifacts only, never code.** If a revised plan implies code changes, it hands off to `/opsx:apply` ([#1188](https://github.com/Fission-AI/OpenSpec/issues/1188)). | ||
| - **Schema-driven, not name-driven.** Artifact ids and paths come from `openspec status`, so the skill works for custom schemas, not just the default `proposal → specs → design → tasks` ([#777](https://github.com/Fission-AI/OpenSpec/issues/777), [#666](https://github.com/Fission-AI/OpenSpec/issues/666)). |
There was a problem hiding this comment.
I wonder what should happen if the change is already built out, or the changes are implemented, but we want to add or update something to that change proposal. I don't think it should implement anything for sure, but perhaps we should provide some guidance on the next step after the update.
Maybe account for any intelligent next-step guidance as part of the skill?
There was a problem hiding this comment.
Good call — added (be92e8a). The skill now ends with a next-step guidance step: after applying confirmed revisions it reports where the change stands and recommends the next command — /opsx:continue if artifacts are missing, /opsx:apply when the change was already implemented (the code may no longer match the revised plan), /opsx:archive when everything is done. Guidance only — it never implements, mirroring the hand-off continue-change.ts already uses. Spec'd as a new "Next-Step Guidance" requirement with an "updating an already-implemented change" scenario, plus a template test (3.5).
| ## Impact | ||
|
|
||
| - `src/core/templates/workflows/update-change.ts` (**new**) — the `openspec-update-change` skill template and the `/opsx:update` command template, mirroring the structure of `continue-change.ts`. Reads artifact ids and paths from `openspec status --json`; embeds no artifact-name patterns. | ||
| - Skill/command registration + the expanded-workflow profile that already lists `continue`, `ff`, `verify`, … so `/opsx:update` installs alongside its siblings. |
There was a problem hiding this comment.
Would this be part of the default workflow or the expanded workflow? I'm leaning towards having it included as part of the default. WDYT?
There was a problem hiding this comment.
Agreed, default happy path seems easiest for users.
There was a problem hiding this comment.
Encoded in be92e8a: update goes into ALL_WORKFLOWS and the default core profile in src/core/profiles.ts (proposal Impact + task 1.4, with a profiles test in 3.6), so /opsx:update is part of the default install.
|
|
||
| > The whole feature is one new skill template over the existing `openspec status` / `openspec list` commands. No changes to the graph engine, the `status` command, or the metadata schema. | ||
|
|
||
| ## 1. The `/opsx:update` skill |
There was a problem hiding this comment.
I'm wondering if we need to be more careful about the skill naming. "Update" might be too generic. I'm thinking about this in two dimensions. Generally, we want very specific skill names.
We also need to consider how this works with future projects. Is the "update" command only related to changes, or can it be applied to any graph? I'm leaning towards making it specific to change proposals for now, and then we can generalize it later.
There was a problem hiding this comment.
Agreed on keeping it specific — pinned in be92e8a (design, Naming). Resolution:
- Scope: change proposals only for v1. "Updating anything other than a change's planning artifacts" is now an explicit non-goal; a future graph type would get its own specific skill then, nothing here blocks that.
- Skill name:
openspec-update-change— change-scoped, following theopenspec-<verb>-changeconvention of its siblings (openspec-continue-change,openspec-new-change, …). - Command: kept
/opsx:updatebecause every verb in the/opsx:family already operates on a change (continue,apply,archive— none says-change), so the namespace carries the scope. Happy to rename to/opsx:update-changeif you'd rather the command itself say it — one-line change.
|
Unrelated, but try and follow the conventional commit standard for PR titles where possible! |
…idance, change-scoped naming - Register /opsx:update in the default core profile, not expanded-only (maintainer call on the PR) - Add next-step guidance: after updating, recommend /opsx:continue, /opsx:apply (esp. when the change was already implemented), or /opsx:archive — guidance only, never acted on - Pin naming scope: skill openspec-update-change, change proposals only; generalizing update to other graph types is an explicit non-goal Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TabishB
left a comment
There was a problem hiding this comment.
Looks good. Let me know if you want to do the implementation as part of this PR or another PR. Either is fine.
Thank you, I'll build out in this same PR next. |
Implements the approved add-update-workflow change: one thin skill over the existing status/list commands, in the default core profile. - new update-change.ts template (skill + command), registered across init, profiles, skill-generation, tool-detection, profile-sync-drift - update joins CORE_WORKFLOWS and ALL_WORKFLOWS - docs: opsx.md command row + usage note, commands.md reference section, supported-tools.md skill list - retire the superseded add-artifact-regeneration-support stub - template tests pin the guardrails (schema-driven ids, planning-only, existingOutputPaths write contract, next-step guidance); parity hashes regenerated; profile/init/update/config tests cover the new core set - tasks.md checked off; validate --strict passes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks @TabishB! Went with implementation in this PR — |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Branch is now current with @alfred-openspec your CHANGES_REQUESTED (2026-07-01) is now stale on two counts: (1) the factual blocker was resolved on-thread — |
alfred-openspec
left a comment
There was a problem hiding this comment.
Re-reviewed the current branch. My earlier changes-requested review is stale: this now uses the existing top-level artifactPaths contract from status --json, edits only existingOutputPaths, and has tests pinning the no-glob-write / no-frontier-advance guardrails. I also verified the PR checkout with focused template/profile/update tests, validate add-update-workflow --strict, and a live status --json check showing concrete existingOutputPaths for the specs glob. Looks good to merge.
Status
Proposal approved and now implemented in this PR (per @TabishB's "either is fine" — implementation commit
f5ed1bb). Full suite green locally (the 17 zsh-installer failures are a known local oh-my-zsh environment issue);openspec validate add-update-workflow --strictpasses; tasks all checked off.What was missing (the motivation)
OPSX names four first-class actions — "create, implement, update, archive" (docs/opsx.md) — and shipped three. Revising a plan meant hand-editing files, with nothing keeping the change's other artifacts coherent, and nothing stopping the agent from editing code when you only meant the plan. Most-requested gap: #1188, #705, #673, #247 (intra-change), #694/#684/#618, discussion #1206.
What it does
One thin skill,
openspec-update-change(/opsx:update), over the existing CLI — no new commands, graph, or schema code:src/core/templates/workflows/update-change.ts(mirrorscontinue-change.ts)updatejoinsALL_WORKFLOWSand thecoreprofile (maintainer call on-thread)opsx.mdrow + "Updating a change" note,commands.mdreference section,supported-tools.mdopenspec/changes/add-artifact-regeneration-support/removedThe skill: resolve the change →
openspec status --change <id> --json→ read artifacts → apply the edit → reconcile the other existing artifacts in any direction → confirm each write → end with next-step guidance (/opsx:continue//opsx:apply//opsx:archive), never acting on it.Guardrails (each pinned by a template test in
test/core/templates/update-change.test.ts): planning artifacts only, code hands off to/opsx:apply; ids/paths come from status JSON, never hardcoded artifact names (anti-#777); writes go toexistingOutputPaths, never a globresolvedOutputPath; missing artifacts defer to/opsx:continue; intent changes redirect to/opsx:new.Proof it works
openspec init --tools claude(built CLI) now generates.claude/skills/openspec-update-change/SKILL.mdand.claude/commands/opsx/update.mdin the default core profile.list --json→status --change --json(ids, statuses,artifactPathswith glob-expandedexistingOutputPaths) →instructions design --json— all return the fields the skill relies on.Notes
openspec-<verb>-change; command stays/opsx:update(the/opsx:namespace already means change-scoped). Happy to rename the command to/opsx:update-changeif preferred.requiresedges onstatus --json.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
/opsx:updateto revise an existing change’s planning artifacts in place./opsx:continue,/opsx:apply, or/opsx:archive.Documentation
/opsx:updateand how to use it.Bug Fixes