Repository navigation
chore: pull ddd-core (ESM config-graph imports) - #152
Merged
Merged
Conversation
790acd0 Merge pull request #16 from dddwa/jakeginnivan/esm-extensionless-imports-nodenext-resolution 1330c7b chore: fully specify config-graph imports for native ESM resolution git-subtree-dir: core git-subtree-split: 790acd007b2c5c411b0a634771c31bf477c9a2a3
…ivan/pull-upstream-core-esm-imports # Conflicts: # core/website/vite.config.ts
The skills describing this layout (/new-conference, /pull-upstream) are authored in ddd-core and travel down with the subtree as core/.claude/skills/. But Claude Code only reads .claude/skills/ at the repo root, so the copies actually loaded were snapshots taken when the fork was created and never updated again. That gap is silent and it already bit: ddd-core added a step to /pull-upstream requiring the subtree-split trailer be verified and the PR marked do-not-squash — guidance that exists precisely because squash-merging a subtree pull corrupts every later pull. The fork's copy was 50 lines behind and had none of it, so the skill ran without those checks. Both fork copies turn out to be older revisions of the upstream files rather than local customisations, so copying is the right operation, not merging. Adds scripts/sync-core-skills.mjs (copy, --check to verify, removes skills core has dropped) and wires it into the pull itself at step 6, next to the `pnpm i` that already runs there. Deliberately not relying on `prepare` alone: pnpm skips lifecycle scripts when an install is a no-op, which is exactly what `pnpm i` does after a pull that changed no dependencies — verified, the hook does not fire there. It is still wired to `prepare` so a fresh clone gets current skills, but the pull is what guarantees it. Nothing is fetched; the subtree pull has already placed the current skills in core/.claude/skills/. This only copies them to where they are read. Claude-Session: https://claude.ai/code/session_012ZUK9WJ1TGNQzBSNwwrBx1
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
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.
Warning
Do not squash-merge this PR. Use "Create a merge commit" or "Rebase and merge".
git subtree pull --squashrecords the sync point in agit-subtree-splittrailer onthe squash commit, and the next pull scans history for the most recent one to find its
starting point. Upstream's individual commits never exist in this fork, so that trailer is
the only record of where we last synced to. A GitHub squash merge rewrites the commit
message and destroys it — after which every future pull replays the whole range from the
original subtree add, manufacturing spurious conflicts as that range grows.
Verified present on this branch (skill step 6b):
What
git subtree pullof ddd-core72ff724..790acd0— dddwa/ddd-core#16, the core half of the native-ESM import work that landed here in #151.Net effect on the fork: one line. Because #151 had already made the identical changes to the fork's
core/, the only content this pull actually brings down is core's own eslint boundary-allowlist fix:Everything else in the subtree already matched.
The one conflict, and why it resolved this way
core/website/vite.config.tsconflicted. The three-way view:'../conference-stub/build-manifest''../conference-stub/build-manifest.ts''../../conference/build-manifest.ts'Both sides independently added
.ts; only the path differs, and that difference is the documented fork override (the comment directly above it spells this out). Resolved by keeping ours. No upstream intent was dropped — the extension upstream wanted is already present.Verification
tsc -b --forcenx run-many --target=lint --allnx run-many --target=test --allnx build websiteAlso diffed the fork's
core/against upstream790acd0: the only remaining differences are the three documented fork overrides (@conference/*path aliases inwebsite/tsconfig.json, thebuild-manifestimport invite.config.ts) plus a pre-existingignoreDeprecationsline. No merge damage.Note on the skill
/pull-upstreamstep 3 previews incoming work withgit log --oneline core..ddd-core/main -- core/. There is no ref namedcore— it's a directory — so that command silently returns empty and the skill reports "already up to date" even when there are commits to pull. I read the real state from thegit-subtree-dirtrailer instead (Squashed 'core/' changes from 8ba297f..72ff724→ fork at72ff724, upstream at790acd0). Worth fixing in the skill separately; I haven't touched it here.https://claude.ai/code/session_012ZUK9WJ1TGNQzBSNwwrBx1