fix(ci): auto-normalize exports in pre-commit hook [OS-608]#811
Conversation
github-actions
Bot
added
docs
Greptile SummaryThis PR eliminates export drift as a recurring DX pain point by adding an unconditional Confidence Score: 4/5Safe to merge — core logic is sound, Bun-native APIs are used consistently, and the selective-staging design correctly avoids polluting commits with unrelated package changes. The approach is well-designed and addresses a real recurring problem. The
|
| Filename | Overview |
|---|---|
| scripts/normalize-exports.ts | Adds --stage <pkg...> flag — normalizes all workspace packages on disk, then selectively git adds only listed packages. Exit-code check on Bun.spawnSync is present; --check + --stage combination emits a warning and safely ignores staging. Minor UX gap: empty --stage list is a silent no-op. |
| apps/outfitter/src/commands/check-orchestrator.ts | Restructures pre-commit plan to push steps in natural order (tooling-sync-exports → normalize-exports → exports). affectedPackages computed via depth-2 path slicing and passed to --stage; exact-match filter in normalize-exports avoids the startsWith sibling-staging footgun noted in earlier reviews. |
| apps/outfitter/src/tests/check-orchestrator.test.ts | Three tests updated to assert the full tooling-sync-exports → normalize-exports → exports ordering chain. One test (JS tooling file scenario, line 362) still omits the normalize-exports < exports assertion — but this was flagged in a prior review thread. |
| .changeset/auto-normalize-exports.md | Correct patch bump for the outfitter package with an accurate description of the DX improvement. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([pre-commit triggered]) --> B{JS/TS files staged?}
B -- Yes --> C[ultracite-fix\nscoped to staged files]
B -- No staged files --> D[ultracite-fix .]
B -- Non-JS/TS only --> E{TS files staged?}
C --> E
D --> F[typecheck --only]
E -- Yes --> G[typecheck\nscoped to staged .ts files]
E -- No --> SKIP_TC[ ]:::invisible
G --> H{packages/tooling/\nfiles staged?}
SKIP_TC --> H
F --> H
H -- Yes --> I[tooling sync:exports]
H -- No --> J
I --> J[normalize-exports\nall packages written to disk]
J --> K{affectedPackages\nnon-empty?}
K -- Yes --> L["git add pkg/package.json\nfor each affected package"]
K -- No --> M
L --> M[check-exports\nvalidation]
M --> N{All steps passed?}
N -- Yes --> O([commit proceeds])
N -- No --> P([commit aborted])
classDef invisible fill:none,stroke:none
Prompt To Fix All With AI
This is a comment left during a code review.
Path: scripts/normalize-exports.ts
Line: 161-169
Comment:
**`--stage` with no packages silently no-ops**
When `--stage` is provided with no package arguments (e.g. `bun scripts/normalize-exports.ts --stage`), `stagePackages` resolves to an empty array `[]`. Because the staging block filters with `stagePackages.some(...)`, an empty `stagePackages` guarantees `toStage` is always empty — all packages are normalized on disk but nothing is staged, and the caller receives no feedback that the flag had no effect.
The orchestrator guards against this at the call site (via the `affectedPackages.length > 0` ternary), but a direct invocation during development or testing has no such safeguard. Adding an early warning when `--stage` is present but resolves to an empty list would make the CLI easier to reason about:
```typescript
const stagePackages =
stageIdx !== -1
? args.slice(stageIdx + 1).filter((a) => !a.startsWith("--"))
: undefined;
if (stagePackages !== undefined && stagePackages.length === 0) {
console.warn(
"[normalize-exports] --stage was provided with no packages; nothing will be staged"
);
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (8): Last reviewed commit: "chore: add changeset" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa26d2ce58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
galligan
force-pushed
the
fix/ci/auto-normalize-exports
branch
from
fa26d2c to
3f0d040
Compare
|
@greptileai review — addressed the scoping concern: normalize-exports now uses --stage to selectively git-add only package.json files for packages with staged changes |
|
@greptileai review |
galligan
force-pushed
the
fix/ci/auto-normalize-exports
branch
from
3f0d040 to
7497dcc
Compare
galligan
force-pushed
the
fix/ci/auto-normalize-exports
branch
from
7497dcc to
9b52227
Compare
galligan
force-pushed
the
fix/ci/auto-normalize-exports
branch
from
9b52227 to
b1026a9
Compare
galligan
force-pushed
the
fix/ci/auto-normalize-exports
branch
from
b1026a9 to
d407523
Compare
galligan
force-pushed
the
fix/ci/auto-normalize-exports
branch
from
d407523 to
87e6c6e
Compare
| const stageIdx = args.indexOf("--stage"); | ||
|
|
||
| // --stage <pkg...>: normalize ALL packages (clean working tree) but only | ||
| // git-add the package.json files for listed packages. This keeps commits | ||
| // scoped while preventing drift from showing up as a dirty working tree. | ||
| const stagePackages = | ||
| stageIdx !== -1 | ||
| ? args.slice(stageIdx + 1).filter((a) => !a.startsWith("--")) | ||
| : undefined; |
There was a problem hiding this comment.
--stage with no packages silently no-ops
When --stage is provided with no package arguments (e.g. bun scripts/normalize-exports.ts --stage), stagePackages resolves to an empty array []. Because the staging block filters with stagePackages.some(...), an empty stagePackages guarantees toStage is always empty — all packages are normalized on disk but nothing is staged, and the caller receives no feedback that the flag had no effect.
The orchestrator guards against this at the call site (via the affectedPackages.length > 0 ternary), but a direct invocation during development or testing has no such safeguard. Adding an early warning when --stage is present but resolves to an empty list would make the CLI easier to reason about:
const stagePackages =
stageIdx !== -1
? args.slice(stageIdx + 1).filter((a) => !a.startsWith("--"))
: undefined;
if (stagePackages !== undefined && stagePackages.length === 0) {
console.warn(
"[normalize-exports] --stage was provided with no packages; nothing will be staged"
);
}Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/normalize-exports.ts
Line: 161-169
Comment:
**`--stage` with no packages silently no-ops**
When `--stage` is provided with no package arguments (e.g. `bun scripts/normalize-exports.ts --stage`), `stagePackages` resolves to an empty array `[]`. Because the staging block filters with `stagePackages.some(...)`, an empty `stagePackages` guarantees `toStage` is always empty — all packages are normalized on disk but nothing is staged, and the caller receives no feedback that the flag had no effect.
The orchestrator guards against this at the call site (via the `affectedPackages.length > 0` ternary), but a direct invocation during development or testing has no such safeguard. Adding an early warning when `--stage` is present but resolves to an empty list would make the CLI easier to reason about:
```typescript
const stagePackages =
stageIdx !== -1
? args.slice(stageIdx + 1).filter((a) => !a.startsWith("--"))
: undefined;
if (stagePackages !== undefined && stagePackages.length === 0) {
console.warn(
"[normalize-exports] --stage was provided with no packages; nothing will be staged"
);
}
```
How can I resolve this? If you propose a fix, please make it concise.…6] (#813) ## Summary CI Summary job fails intermittently even when all individual jobs pass. Observed 3-4 times in a single session across PRs #806, #810, #811, #812. **Root cause:** When `gt submit` force-pushes a branch, GitHub Actions cancels the in-flight run. The `ci-summary` job checks `contains(needs.*.result, 'cancelled')`, which catches cancelled jobs from the superseded run — not actual failures. Fixes https://linear.app/outfitter/issue/OS-616/ci-summary-job-fails-spuriously-after-force-push-due-to-cancelled-job ## What changed `.github/workflows/ci.yml`: 1. **Dropped `cancelled` from the failure check** — only `failure` triggers the exit. Cancelled jobs from superseded runs aren't real failures. 2. **Added `concurrency` group** — `ci-${{ github.ref }}` with `cancel-in-progress: true` ensures only one CI run per branch exists at a time, preventing the race structurally. ## Test plan - [x] Workflow YAML is syntactically valid - [x] The concurrency group scopes to the branch ref, so main and PR branches don't cancel each other - [x] `cancel-in-progress: true` only cancels runs on the same ref (safe for stacked PRs on different branches) 🤘🏻 In-collaboration-with: [Claude Code](https://claude.com/claude-code)
1 participant
Summary
Export drift in
package.jsonhas been the most recurring DX issue in the repo (OS-120, OS-374, OS-403, OS-523, OS-530, and this session'spackages/tooling/package.jsonmutation). The root cause: adding/renaming source files without runningbun run buildleaves exports stale.This PR adds an unconditional
normalize-exportsstep to the pre-commit hook with scoped staging, so exports are always correct and the working tree is always clean — without pulling unrelated changes into commits.Fixes https://linear.app/outfitter/issue/OS-608/auto-regenerate-and-auto-stage-exports-in-pre-commit-hook-to-eliminate
How it works
The pre-commit plan now includes:
Key design: normalize all, stage selectively. The normalize step rewrites exports in ALL packages (so the working tree is always clean — no confusing dirty files for agents or developers). But it only
git adds thepackage.jsonfiles for packages that have staged changes in the commit. This means:package.jsondiffs)package.jsonfiles enter the commitThe
--stage <pkg...>flag onnormalize-exports.tshandles the selective staging internally viaexecFileSync("git", ["add", ...]), so the orchestrator doesn't need to manage it.What changed
scripts/normalize-exports.ts: Added--stage <pkg...>flag — normalizes all packages, but onlygit adds listed onesapps/outfitter/src/commands/check-orchestrator.ts: Pre-commit plan computes affected packages from staged files and passes them to--stageTest plan
normalize-exports.tsworks with no flags (backward compatible)normalize-exports.ts --checkworks (backward compatible)normalize-exports.ts --stage packages/docsnormalizes all, stages only docs🤘🏻 In-collaboration-with: Claude Code