chore: add porting-code-prs skill for the desktop migration - #76335
Conversation
charlesvien
added
the
skip-agent-review
|
😎 This pull request was merged. |
Prompt To Fix All With AI### Issue 1
.agents/skills/porting-code-prs/SKILL.md:49-54
**Excluded-only commits halt porting**
When a source commit changes only an excluded workflow or lockfile, filtering removes its entire patch, but `--empty=drop` does not handle commits emptied by path exclusion, causing `git am` to stop mid-port without the documented recovery step.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "chore: add porting-code-prs skill for th..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| gh pr diff <N> --repo PostHog/code --patch > "$PATCH" | ||
| git fetch https://github.com/PostHog/code.git refs/pull/<N>/head | ||
| ``` | ||
|
|
||
| 2. Apply with authorship preserved: | ||
|
|
There was a problem hiding this comment.
When a source commit changes only an excluded workflow or lockfile, filtering removes its entire patch, but --empty=drop does not handle commits emptied by path exclusion, causing git am to stop mid-port without the documented recovery step.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .agents/skills/porting-code-prs/SKILL.md
Line: 49-54
Comment:
**Excluded-only commits halt porting**
When a source commit changes only an excluded workflow or lockfile, filtering removes its entire patch, but `--empty=drop` does not handle commits emptied by path exclusion, causing `git am` to stop mid-port without the documented recovery step.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
|
|
||
| 4. Treat conflicts in drift-listed files (local security patches, `pnpm-workspace.yaml` overrides) as intentional monorepo divergence: keep the monorepo side and re-apply the PR's intent on top of it. | ||
|
|
||
| 5. Verify from `products/desktop/`: `pnpm install --frozen-lockfile`, `pnpm typecheck` and `pnpm --filter <pkg> test` for the packages the PR touches. The desktop CI suite runs on the monorepo PR itself. |
There was a problem hiding this comment.
Medium: Untrusted source PR code is executed with trusted credentials
An attacker can add commands to a package lifecycle or test script in their source PR, and this step executes them on the maintainer's workstation. Once pushed as a same-repository monorepo PR, desktop CI also supplies secrets that the ported code can read; require an explicit trust/review gate before running code or pushing the port to a trusted branch, and keep untrusted ports on a secretless CI path.
PR overviewThis pull request adds a porting-code-prs skill that documents the workflow for moving desktop migration changes into the target repository and validating them. One security issue remains open: the workflow can execute code from an untrusted source pull request on a maintainer’s workstation and later in CI with trusted credentials. A malicious contributor could use lifecycle or test scripts to execute commands or access secrets unless a trust gate and secretless CI path are added. No issues have yet been addressed. Open issues (1)
Fixed/addressed: 0 · PR risk: 8/10 |
1 participant
Problem
PostHog/code's main is frozen after the desktop import (#72483), so every open PR there has to be remade by hand against
products/desktop/. There was no documented procedure, and the non-obvious parts (path remapping, the 3-way fallback, the paths that must not be blind-applied) get rediscovered per PR.Changes
Adds a repo-internal skill,
.agents/skills/porting-code-prs, that walks an agent through porting one PR or sweeping all of an author's open PostHog/code PRs onto the monorepo:git am -3 --directory=products/desktop/to remap paths while preserving each commit's original author and message, with a fetch of the source PR head so the 3-way fallback has the blobs it needs.github/**(transformed workflows perMIGRATION.md's mapping table), and the lockfiles that carry local security override pinsproducts/desktop/MIGRATION.mdas the authoritative contract for conflicts in drift-listed filesPostHog/code#Nreference rewriting, draft state and assignee carried overHow did you test this code?
hogli lint:skillspasses. Verified the core mechanics against a live PR (PostHog/code#4063):gh pr diff --patch+git apply --check --directory=products/desktopapplies cleanly, and confirmed--excludematches after the--directoryprefix is prepended (the patterns in the skill carry the prefix for that reason).Automatic notifications
Docs update
N/A (the skill is the doc)
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored with Claude Code. Invoked /writing-skills for conventions and placed the skill in
.agents/skills/(repo-internal engineering skill) rather thanproducts/desktop/skills/(customer-distributed) orproducts/desktop/.claude/skills/(overwritten on resync per MIGRATION.md). Chose patch-basedgit am --directoryover a subtree/cherry-pick approach because the import was an archive copy, not a git merge, so source commits don't graft directly.