Harden Skia-sync conflict policy, allow durable gn args, and split skill vs workflow

[mattleibow]

Follow-up to the mode=main (tip) work (#4254) and release-line targeting (#4239). The first real tip run surfaced two rough edges in the upstream-sync automation and exposed that a chunk of general Skia-update knowledge lived only in the workflow prompt (so the update-skia skill couldn't drive it locally). Then, watching real runs, a third rough edge appeared: the conclusion job mislabeled successful syncs as "no-op".

Sections 1–3 are docs/prompt-only (no product code; prompt-body edits, so frontmatter_hash is unchanged for those). Section 4 is the one workflow-config change — it edits the workflow frontmatter (safe-outputs), so unlike the prompt-body edits it does regenerate auto-skia-sync.lock.yml.

1. Verify-upstream-or-reapply conflict resolution (do it right, don't pick a side)

A tip merge is ~291 commits ahead of our submodule base, so it conflicts on every file carrying an [M1xx] fork cherry-pick. The old guidance ("keep ours" / "resolve what you reasonably can") risks two failure modes:

• --theirs → silently drops a not-yet-upstreamed fork patch.
• --ours → freezes a stale form of a patch upstream has since refined (e.g. our SDF-LCD patch used getMaxScale(); upstream relanded it as sk_ieee_float_divide(1.f, getMinScale())).

Replaced with an explicit, auditable policy: for each conflicted file, classify every fork patch as upstreamed (take upstream's refined form, record the SHA) or not upstreamed (re-apply our patch on top of upstream) — never a blanket --theirs/--ours, never a silent drop. A mandatory before-merge snapshot of fork patches is cross-referenced so every fork patch on a conflicted file lands in the "Conflicts resolved" table as upstreamed or re-applied. A patch that is neither is a lost patch — stop and fix it.

Applied consistently in known-gotchas.md (gotcha #15, rewritten), SKILL.md (Phase 5 conflict step + audit), and auto-skia-sync.md (tip-mode note).

Why nothing was actually lost in #4254: the merge is a real two-parent git merge (a2658b7 has parents b16789e and upstream e793ca2), so all fork commits remain in history. Every [M150] patch had genuinely been upstreamed, each with a verified SHA — the "dropped" patches were only redundant diff content converging with upstream. This change makes that safety guaranteed rather than lucky.

2. Allow a required new gn arg through build.cake

The tip build needed skia_use_partition_alloc=false because upstream now defaults skia_use_partition_alloc=is_clang, while our DEPS deliberately disables third_party/externals/partition_alloc. Skia ships an official zero-overhead noop raw_ptr for exactly this case (MiraclePtr fallback "because partition_alloc dependency is missing"), so =false is the supported embedder path — not a hack. The agent had to pass it as a one-off --gnArgs CLI flag purely because the rule banned all build.cake edits, leaving durable config non-durable.

Relaxed the rule into two tiers:

• ❌ Still forbidden: changing compiler/linker flags (or scripts/infra/native/**) to silence a host build error — those break the platforms the Linux-only sync never builds.
• ✅ Now allowed: adding a genuinely required new upstream gn arg to the affected platforms' native/**/build.cake gn-args lists (the single source of truth, next to the existing skia_use_* toggles), flagged in both PR summaries for cross-platform human review.

Sequencing matters: skia_use_partition_alloc does not exist in m150, so baking it in now would break the current build with "unknown gn arg." The relaxed rule lets the next tip/milestone merge add the toggle in-context — shipping it alongside the milestone-bearing submodule, never before.

partition_alloc recommendation (for the eventual m151 bump): keep it disabled via the noop (=false) baked into build.cake, rather than vendoring the Chromium allocator. The noop is Google's own supported path; our threat model ≠ Chrome's (UAF mitigation has marginal value for a library in trusted .NET apps); vendoring adds a large Chromium dep + binary bloat + ongoing DEPS-roll maintenance across our full platform matrix. Even upstream has flip-flopped on the default. Revisit only if we ever want raw_ptr hardening and accept the cross-platform dependency.

3. Make the skill standalone; reduce the workflow to CI-specific overrides

The workflow prompt had accumulated general Skia-update knowledge that belongs in the update-skia skill, not in CI prose. Two concepts were documented only in the workflow, so the skill couldn't drive a local/manual run of them. Moved both into the skill (so it works standalone), and trimmed the workflow to minimal overrides + pointers — keeping the remaining duplicated reminders as short safety one-liners, since an unattended CI agent benefits from repeated guardrails.

Into the skill (now works locally):

• main/tip mode is a first-class third option in Phase 1 (alongside main-target and release-line-target): the variable table gains a {UPSTREAM_REF} row + a tip column, plus a paragraph on the not-a-version-bump semantics and how Phase 2/5 milestone diffs become merge-base..upstream/main for the tip. Phase 5's merge command is generalized to upstream/{UPSTREAM_REF}.
• Required-new-gn-arg rule in Phase 7 (error-table row + note), backed by a new gotcha Text measurement support #23 with the full partition_alloc worked example and the milestone-sequencing caveat.

Out of the workflow (body-only edits → lock byte-identical): the mode/tip note and the FORBIDDEN/ALLOWED build.cake block collapse to short pointers into the skill, keeping only the genuinely CI-specific parts — can't apt-install in the sandbox, a missing host dependency is a workflow bug (fix the install step, don't hack shared flags), and the Linux-only-build cross-platform-review reminder.

4. Stop mislabeling a real sync as a "no-op" (honest completion signal)

Watching the first live runs, a sync that actually merged upstream, built, tested, and opened both PRs was reported by the gh-aw conclusion job as a no-op — which then tried (and failed, the workflow has no issues: write) to file a "no-op runs" issue. Misleading on two counts: a PR-producing run is not a no-op, and the failed issue step added red noise.

The cause is the two-subsystem split this workflow uses on purpose. safe-outputs: staged: true blocks the agent from creating anything directly; the real GitHub writes (push both repos, open both PRs) are done by the host post-step skia-sync-push-prs.sh with SKIASHARP_AUTOBUMP_TOKEN — gh-aw can't open the mono/skia PR itself because the submodule's merge commits live in a nested repo it only sees as a gitlink. With noop as the agent's only available completion signal, every run — including real syncs — signalled noop, and the conclusion classified on that alone.

Give the agent an honest signal instead:

• Declare create-pull-request in safe-outputs, kept staged (preview-only: no real PR, no branch push, no patch — the safe_outputs job runs on ubuntu-slim with permissions: {} and never touches the workspace). The agent calls it as its completion signal when it did work, so the run registers as a pull-request output rather than a no-op. Real PRs are still opened only by the post-step.
• Set noop: report-as-issue: false (the gh-aw default is true). noop is now reserved for genuine no-work runs and no longer tries to file an issue the workflow can't create.
• create_issue is no longer offered to the agent (it was never used — the prompt already forbade it).
• The prompt body gains a short "Completion signal" section: call create_pull_request when skia-sync-env.sh was written, noop only when it wasn't.

Unlike sections 1–3, this edits the workflow frontmatter, so auto-skia-sync.lock.yml is regenerated via gh aw compile (safe-outputs config + the #4260 name; never hand-edited).

Rebased on main

Picked up #4260 (regroup/rename GitHub Actions workflows), which renamed this workflow Skia Upstream Sync → Sync - Skia Upstream. The merge was clean and gh aw compile reproduced the lock with a 0-diff, so the rename and the Section-4 safe-outputs change coexist correctly. Also confirmed #4267's new support-status model (versions.json: stable 4.148, preview 4.150; the SkiaSharp minor number is the Skia milestone) is consistent with the skill's release/<major>.<milestone>.x mapping — no change needed.

Validation

• gh aw compile auto-skia-sync → 0 errors (4 pre-existing warnings).
• Sections 1–3 are prompt-body edits → no lock impact. Section 4 edits the frontmatter, so auto-skia-sync.lock.yml is regenerated and committed (this supersedes the earlier "no lock change / 3 files" note).
• End-to-end run #28269474099 (mode=main, real 2-commit merge): the agent emitted create_pull_request (staged, title [skia-sync] Update Skia to upstream main HEAD (e47bb0549c)); the safe_outputs job logged 0 noop message(s) and wrote a staged PR preview; the conclusion logged No noop items found with no failed issue step; and the host post-step still pushed skia-sync/main to both repos and updated the real PRs ([skia-sync] Merge upstream Skia main (tip) #4263 + Making the Forms views more extendable #269). All jobs green.
• Net PR diff = 4 files: SKILL.md, references/known-gotchas.md, auto-skia-sync.md, auto-skia-sync.lock.yml.

[github-actions]

📦 Try the packages from this PR

Warning

Do not run these scripts without first reviewing the code in this PR.

Step 1 — Download the packages

bash / macOS / Linux:

curl -fsSL https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.sh | bash -s -- 4258

PowerShell / Windows:

iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4258"

Step 2 — Add the local NuGet source

dotnet nuget add source ~/.skiasharp/hives/pr-4258/packages --name skiasharp-pr-4258

More options

Option	Description
--successful-only / -SuccessfulOnly	Only use successful builds
--force / -Force	Overwrite previously downloaded packages
--list / -List	List available artifacts without downloading
--build-id ID / -BuildId ID	Download from a specific build

Or download manually from Azure Pipelines — look for the nuget artifact on the build for this PR.

Remove the source when you're done:

dotnet nuget remove source skiasharp-pr-4258

[github-actions]

📖 Documentation Preview

The documentation for this PR has been deployed and is available at:

🔗 View Staging Site
🔗 View Staging Docs
🔗 View Staging Gallery (Blazor)
🔗 View Staging Gallery (Uno Platform)
🔗 View Staging SkiaFiddle

This preview will be updated automatically when you push new commits to this PR.

---

This comment is automatically updated by the documentation staging workflow.