trails-cool/trails
Watch
1
1
Fork
You've already forked trails
 0
Code Issues 2 Pull requests Projects Releases Packages Activity Actions 9

fix(openspec): stop two MODIFIED blocks from dropping shipped scenarios #80

Merged
ullrich merged 1 commit from fix/openspec-modified-blocks-drop-scenarios into main 2026-08-19 14:34:44 +00:00
Conversation 0 Commits 1 Files changed 2 +32 -9
ullrich commented 2026-08-19 14:20:37 +00:00
Owner
Copy link

pnpm openspec validate --all --strict fails on change/route-features and change/wahoo-production-cutover as soon as @fission-ai/openspec reaches 1.9.0. That currently blocks #38, but the bump is incidental: 1.9.0 added a check 1.6.0 lacked, and these two documents have been wrong since they were written. Verified the asymmetry directly — 1.6.0 passes both, 1.9.0 fails both.

What the new check catches

A MODIFIED requirement replaces the whole block, so a MODIFIED block that omits scenarios the current spec still has would silently delete them on archive. 1.9.0 refuses that — so it caught a real latent data-loss path rather than being newly fussy:

Change Requirement Scenarios it would have deleted
route-features Activity visibility "New activities default to private", "Owner changes an activity's visibility"
wahoo-production-cutover Server-side route push pipeline "First push of a route", "Re-push after editing the route", "PUT against a deleted Wahoo route falls back to POST"

Five scenarios across two changes, both of which are un-archived and would have taken effect eventually.

The wahoo change needed more than the validator asked for

Copying the three scenarios in would have satisfied the check while still regressing the requirement text. The delta describes a POST-only pipeline; the shipped spec has POST → PUT idempotency with a 404 fallback and last_pushed_version.

So the shipped text is now the base, with the delta's genuinely new normative content appended as a second paragraph:

  • the route[file] data URI (Wahoo silently discards plain base64)
  • the COURSE capabilities bitmask — valid | distance | position (0x1A), not time-only (0x04)
  • non-zero LAP total_elapsed_time / total_timer_time

I checked that none of those three appear anywhere in openspec/specs/wahoo-route-push/spec.md, so they are real additions, not duplication. Its near-duplicate "Successful push" scenario is folded into "First push of a route" as two extra AND lines, where those assertions belong in the flow.

Judgement calls worth reviewing

I read the POST-only wording as staleness, not intent. The change has 0 of 18 tasks done and predates the PUT work. If it genuinely means to revert to POST-only, this merge is wrong and the change wants rewriting instead — that is the one thing here a reviewer should actually decide.

The capabilities and LAP requirements arguably belong in the separate "GPX to FIT Course conversion" requirement, since they describe the encoder rather than the push pipeline. Left where the change author put them to keep this a validation fix.

Testing

  • 81 passed, 0 failed under both 1.6.0 (what main has today) and 1.9.0 (what #38 brings), so this is safe now and unblocks the bump later.
  • Checked mechanically rather than by eye: for each MODIFIED requirement, the delta's scenario set is a superset of openspec/specs' — dropped=none for both.

Docs only; no code paths touched. After this lands, #38 should go green on a rebase without changing its dependency content.

`pnpm openspec validate --all --strict` fails on `change/route-features` and `change/wahoo-production-cutover` as soon as `@fission-ai/openspec` reaches 1.9.0. That currently blocks #38, but the bump is incidental: **1.9.0 added a check 1.6.0 lacked, and these two documents have been wrong since they were written.** Verified the asymmetry directly — 1.6.0 passes both, 1.9.0 fails both. ## What the new check catches A `MODIFIED` requirement replaces the **whole** block, so a MODIFIED block that omits scenarios the current spec still has would silently delete them on archive. 1.9.0 refuses that — so it caught a real latent data-loss path rather than being newly fussy: | Change | Requirement | Scenarios it would have deleted | |---|---|---| | `route-features` | Activity visibility | "New activities default to private", "Owner changes an activity's visibility" | | `wahoo-production-cutover` | Server-side route push pipeline | "First push of a route", "Re-push after editing the route", "PUT against a deleted Wahoo route falls back to POST" | Five scenarios across two changes, both of which are un-archived and would have taken effect eventually. ## The wahoo change needed more than the validator asked for Copying the three scenarios in would have satisfied the check while still regressing the requirement **text**. The delta describes a **POST-only** pipeline; the shipped spec has POST → PUT idempotency with a 404 fallback and `last_pushed_version`. So the shipped text is now the base, with the delta's genuinely new normative content appended as a second paragraph: - the `route[file]` data URI (Wahoo silently discards plain base64) - the COURSE `capabilities` bitmask — `valid | distance | position` (`0x1A`), not time-only (`0x04`) - non-zero LAP `total_elapsed_time` / `total_timer_time` I checked that none of those three appear anywhere in `openspec/specs/wahoo-route-push/spec.md`, so they are real additions, not duplication. Its near-duplicate "Successful push" scenario is folded into "First push of a route" as two extra `AND` lines, where those assertions belong in the flow. ## Judgement calls worth reviewing **I read the POST-only wording as staleness, not intent.** The change has 0 of 18 tasks done and predates the PUT work. If it genuinely means to revert to POST-only, this merge is wrong and the change wants rewriting instead — that is the one thing here a reviewer should actually decide. **The capabilities and LAP requirements arguably belong in the separate "GPX to FIT Course conversion" requirement**, since they describe the encoder rather than the push pipeline. Left where the change author put them to keep this a validation fix. ## Testing - `81 passed, 0 failed` under **both** 1.6.0 (what main has today) and 1.9.0 (what #38 brings), so this is safe now and unblocks the bump later. - Checked mechanically rather than by eye: for each MODIFIED requirement, the delta's scenario set is a superset of `openspec/specs`' — `dropped=none` for both. Docs only; no code paths touched. After this lands, #38 should go green on a rebase without changing its dependency content.
fix(openspec): stop two MODIFIED blocks from dropping shipped scenarios
Some checks failed
CI / Security Scan (pull_request) Successful in 1m42s
Details
CI / Dockerfile Package Check (pull_request) Successful in 1m18s
Details
CI / Checks (pull_request) Successful in 4m20s
Details
CI / Visual Tests (pull_request) Successful in 5m37s
Details
CI / E2E Tests (pull_request) Failing after 3m51s
Details
Cancel superseded CI / Cancel in-flight CI (pull_request) Successful in 1m38s
Details
CI / Journal Image Smoke Test (pull_request) Successful in 50m6s
Details
3449b06f1e
 
`openspec validate --all --strict` fails on `change/route-features` and
`change/wahoo-production-cutover` as soon as @fission-ai/openspec reaches 1.9.0.
That currently blocks #38, but the bump is incidental — 1.9.0 added a check that
1.6.0 lacked, and these two documents have been wrong since they were written.
Verified the asymmetry directly: 1.6.0 passes both, 1.9.0 fails both.

A MODIFIED requirement replaces the whole block, so a MODIFIED block that omits
scenarios the current spec still has would silently delete them on archive.
1.9.0 refuses that, which means it caught a real latent data-loss path rather
than being newly fussy:

  route-features / activity-feed — "Activity visibility" omitted
    "New activities default to private"
    "Owner changes an activity's visibility"

  wahoo-production-cutover / wahoo-route-push — "Server-side route push
  pipeline" omitted
    "First push of a route"
    "Re-push after editing the route"
    "PUT against a deleted Wahoo route falls back to POST"

Both blocks now carry every shipped scenario plus their own. Checked
mechanically: for each MODIFIED requirement, the set of scenarios in the delta
is a superset of the set in openspec/specs — dropped=none for both.

The wahoo change needed more than the validator asked for. Copying the three
scenarios in would have satisfied the check while still regressing the
requirement TEXT: the delta describes a POST-only pipeline, but the shipped spec
has POST -> PUT idempotency with a 404 fallback and `last_pushed_version`. So the
shipped text is now the base, with the delta's genuinely new normative content
appended as a second paragraph — the `route[file]` data URI, the COURSE
`capabilities` bitmask (`valid | distance | position` = 0x1A, not time-only
0x04), and non-zero LAP `total_elapsed_time`/`total_timer_time`. None of those
three appear anywhere in the current spec, so they are additions rather than
duplication.

Reading the POST-only wording as staleness rather than intent: the change has
0 of 18 tasks done and predates the PUT work. If it really means to revert to
POST-only, this merge is wrong and the change wants rewriting instead.

The delta's near-duplicate "Successful push" scenario is folded into "First push
of a route" as two extra AND lines, which is where those assertions belong in
the flow.

Kept on a branch off main rather than on #38's renovate branch: Renovate
regenerates that branch on its next rebase and could discard the fix, and these
documents have nothing to do with the dependency bump.

Validated 81 passed / 0 failed under BOTH 1.6.0 (what main has today) and 1.9.0
(what #38 brings), so it is safe now and unblocks the bump later.
ullrich deleted branch fix/openspec-modified-blocks-drop-scenarios 2026-08-19 14:34:48 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
dependencies
Dependency updates raised by Renovate (renovate.json5 labels)

  
major
Major version bump — always needs human review, never auto-merged

  
security-pin
Touches a pnpm.overrides CVE remediation pin, not an ordinary dependency

No labels
dependencies
major
security-pin
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
trails-cool/trails!80
No description provided.