Skip to content

fix(ci): watch mount-rules.ts in the spec-changes path filter - #120

Merged
gjtorikian merged 2 commits into
mainfrom
chore/spec-changes-watch-mount-rules
Aug 26, 2026
Merged

fix(ci): watch mount-rules.ts in the spec-changes path filter#120
gjtorikian merged 2 commits into
mainfrom
chore/spec-changes-watch-mount-rules

Conversation

@gjtorikian

@gjtorikian gjtorikian commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Record spec changes only fires on paths: spec/**. But the producer resolves every changed symbol to its post-mount service via mountRules (scripts/build-spec-changes.mjs), so editing the rules re-partitions already-published endpoints — a service rename, or moving endpoints between services — and changes the SDK surface without touching spec/**. Today such a merge doesn't even start a run.

Scope — this is the prerequisite, not the whole fix

Worth being explicit, since the title reads like a complete fix and isn't:

changedServices is built by iterating report.changes from an old-spec-vs-new-spec oagen diff. When only the rules moved, both sides of that diff are the same spec, the report is empty, and the run falls through to the No service changes; nothing to push check at line ~133 and exits 0.

So this PR makes the run happen, which is necessary for any future detection, but on its own it will not surface a re-partition. Doing that needs a partition diff — resolving the same spec under the old rules and the new rules and comparing the resulting service→endpoint mapping. That's a real change to build-spec-changes.mjs, so I've left it out rather than smuggling it into a path-filter edit. Happy to open a follow-up if you want it.

How it surfaced

#112 added a FeatureFlagsTargetTypes: 'FlagTargetTypes' rule, and no run exists for its merge commit (882f139, merged 08-24 19:14Z; that day's runs were 15:22, 17:20, 23:20). That PR was harmless either way — its endpoints are still @ApiExcludeController(), so the spec diff would have been empty — but a rule touching live endpoints would have skipped the bot identically.

Why only mount-rules.ts and not src/policy/**

build-spec-changes.mjs reads mountRules and nothing else out of dist/policy.mjs (see loadPolicy()), so naming.ts / transforms.ts / operation-hints.ts don't affect this manifest.

Test plan

  • paths parses as ["spec/**", "src/policy/mount-rules.ts"].
  • No behavior change for spec/** merges — this only widens what starts a run; the job body is untouched.

The producer resolves every changed symbol to its post-mount service via
`mountRules`, so a mount-rules edit re-partitions already-published
endpoints — renaming a service, or moving endpoints between services —
and changes the SDK surface without touching spec/**. Under the
`spec/**`-only filter such a merge did not even start a run.

Surfaced by #112, which added a FlagTargetTypes rule and never triggered
one. That PR was harmless on its own (its endpoints are still
ApiExcludeController, so the diff would have been empty anyway), but a
rule touching live endpoints would have skipped the bot the same way.

This is the prerequisite, not the whole fix: `changedServices` is derived
from an old-spec-vs-new-spec `oagen diff`, which is empty when only the
rules moved, so the run still ends at the "No service changes" check.
Surfacing a re-partition additionally needs a partition diff (old rules
vs new rules over the same spec), which this job does not compute — left
for a follow-up rather than smuggled into a path-filter change.

Scoped to just this file rather than src/policy/**: build-spec-changes.mjs
reads `mountRules` and nothing else out of dist/policy.mjs.
@gjtorikian
gjtorikian force-pushed the chore/spec-changes-watch-mount-rules branch from 78908ea to a7d9c36 Compare August 26, 2026 16:34
@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR expands the Record spec changes workflow’s push-path filter so changes to mount rules start the existing producer.

  • Adds src/policy/mount-rules.ts to the workflow’s watched paths.
  • Generalizes the nearby manual-run comment to reflect the expanded filter.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
.github/workflows/spec-changes.yml Adds the intended mount-rules trigger and updates its explanatory comment; no eligible follow-up defect was identified.

Reviews (2): Last reviewed commit: "Update spec-changes.yml" | Re-trigger Greptile

@gjtorikian
gjtorikian merged commit e834104 into main Aug 26, 2026
2 checks passed
@gjtorikian
gjtorikian deleted the chore/spec-changes-watch-mount-rules branch August 26, 2026 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant