Skip to content

sync-skill: catch balanced -X ours seam damage, and stop typechecking the retired mobile-web project #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zpyoung wants to merge 1 commit into
base: main
Choose a base branch
from
Open

sync-skill: catch balanced -X ours seam damage, and stop typechecking the retired mobile-web project #99

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions .claude/skills/sync-upstream/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,19 @@ output lists — `checkout.txt`, `remove.txt`, `ours.txt`, `merge-review.txt`. `
no shell command: those paths took a real three-way merge and must be read by hand against the
manifest's declared lines.

`-X ours` resolved every conflicting hunk in those files toward the fork, so re-merge each seam file

@greptile-apps greptile-apps Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Seam command lacks file inputs The new check names three commits, but git merge-file needs three file paths for each seam. It also prints the result rather than updating the seam file. Without instructions to extract those files and apply the result, an operator cannot reliably complete this required check.

without it (`git merge-file --diff3 -p` with `$MERGE_HEAD_PRE`, `$PREV_TAG`, `$UPSTREAM_TARGET`) and
resolve every file that reports a conflict. Do this even where `--verify-residuals` passes: a
discarded upstream hunk the same size as the fork's keeps the budget intact. v1.4.215 had two such
files that the residual check never flagged: `pr-e2e-gate-contract.test.mjs`, still naming a job
upstream had retired, and `NativeChatToolLine.tsx`, with a hover-group rename lost.

The `ours.txt` three-way merge has a matching blind spot. A clean merge imports upstream's new
assertions into an exception-owned contract test even when they name a workflow or job the fork
deletes. v1.4.215 did this for `track-community-prs.yaml` and `package_windows`. Nothing flags it
until PR CI, so grep every merged exception test for the paths the manifest marks `deleted: true`
and for jobs missing from the fork's `pr.yml`.

The reference also carries three per-sync checklists that are part of this step, not optional
extras: **tier-2 forked-copy replay**, **tier-4 pending-upstream review**, and **upstream
feature-collision review**. Each can surface a decision the reference routes to a human. Under
Expand Down Expand Up @@ -646,19 +659,21 @@ Order: `pnpm install --frozen-lockfile` (re-run it here — the merge may have t

**Never run `pnpm typecheck` here.** `AGENTS.md` forbids it on this machine: it is
`run-typecheck-projects-in-parallel.mjs`, which spawns a `tsc --noEmit` per project concurrently,
and with several sync worktrees open that saturates every core. Run the same four projects one
after another instead — same coverage, no concurrency — clearing the build cache before each, since
and with several sync worktrees open that saturates every core. Run the projects it lists in
`TYPECHECK_PROJECTS` one after another instead — same coverage, no concurrency — clearing the build cache before each, since
composite projects cache errors across the `git checkout` swaps this step is full of:

```sh
Comment on lines 665 to 666

@greptile-apps greptile-apps Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Typecheck instructions disagree This step says to use the tag runner's TYPECHECK_PROJECTS, but shows a fixed set of three commands; the ownership reference still calls them four commands. The current fork runner uses a waves list, not TYPECHECK_PROJECTS. If a tag lists a different set of projects, following the displayed commands can leave one unchecked. Please make the gate and its reference agree on how to follow the tag's list.

find config -maxdepth 1 -name '*.tsbuildinfo' -delete && pnpm run typecheck:node
find config -maxdepth 1 -name '*.tsbuildinfo' -delete && pnpm run typecheck:web
find config -maxdepth 1 -name '*.tsbuildinfo' -delete && pnpm run typecheck:cli
find config -maxdepth 1 -name '*.tsbuildinfo' -delete && \
pnpm exec tsc --noEmit -p config/tsconfig.mobile-web.json
```

Four separate commands on purpose, not a loop: under the sandboxed shell a command in a `for` body
Read the project list from the tag's `config/scripts/run-typecheck-projects-in-parallel.mjs` rather
than from here, because upstream edits it. v1.4.215 retired `config/tsconfig.mobile-web.json` in
#22193, and a gate that still ran it failed with a missing-file error that names no type problem.

Separate commands on purpose, not a loop: under the sandboxed shell a command in a `for` body
can come back "command not found", which inverts the result silently, and a loop's `|| break` would
swallow the exit code this gate turns on. Read each one's status before running the next. `zsh` has
no `PIPESTATUS`, so do not pipe these into `tail` and expect `$?` to mean anything — redirect to a
Expand Down
Loading