From 841717842fcfc0d8b4b2fad1cfa9cbcf8408d9d6 Mon Sep 17 00:00:00 2001 From: Vlad Mocanu Date: Fri, 27 Feb 2026 13:34:19 +0200 Subject: [PATCH] fix: improve upstream-sync Claude prompt and permissions - Add command execution rules to prompt (no env prefixes, no 2>&1) - Document pre-existing test failures so Claude doesn't waste turns - Add Bash(echo:*) and Bash(cat:*) to allowed_tools - Delete stale remote branches before creating new ones Addresses 24 permission denials in last run where Claude spent ~15 of 47 turns on blocked commands. Co-Authored-By: Claude Opus 4.6 --- .forgejo/kcl-ci/main.k | 30 ++++++++++++++++++++++++---- .forgejo/workflows/upstream-sync.yml | 30 ++++++++++++++++++++++++---- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/.forgejo/kcl-ci/main.k b/.forgejo/kcl-ci/main.k index e7cef80..29420ba 100644 --- a/.forgejo/kcl-ci/main.k +++ b/.forgejo/kcl-ci/main.k @@ -274,10 +274,25 @@ When fixing incompatibilities, minimize fork divergence to keep future merges ea Prefer adapter patterns (separate Forgejo files) over inline modifications to upstream files. The fewer upstream files we modify, the fewer conflicts in the next sync. +## IMPORTANT: Command execution rules +- GITHUB_SERVER_URL=https://github.com is ALREADY exported in your environment. Run commands directly WITHOUT env var prefixes. + CORRECT: `bun test` + WRONG: `GITHUB_SERVER_URL=https://github.com bun test` (this will be blocked by permissions) +- Do NOT use `2>&1` redirections, subshells (`sh -c`), or `export` commands — they will be blocked. +- For inspecting env vars, use: `bun -e "console.log(process.env.VAR_NAME)"` +- Available commands: git, bun, bunx, npx, curl, echo, and anything in ./node_modules/.bin/ + +## Known pre-existing test failures +Some tests in comment-logic.test.ts and create-prompt.test.ts have pre-existing failures related to Forgejo URL patterns. +These are NOT caused by the upstream merge. When running `bun test`: +- If only comment-logic.test.ts and/or create-prompt.test.ts fail, that's expected — proceed. +- If OTHER test files fail that weren't failing before, investigate those as potential merge regressions. +- Do NOT spend time trying to fix pre-existing failures or trying different env var approaches. + ## After merge is resolved and reviewed -1. Run tests: bun test (GITHUB_SERVER_URL is already set in the environment) -2. Run typecheck: bun run typecheck -3. Check formatting: bun run format:check — if issues, fix with bun run format +1. Run tests: `bun test` — see "Known pre-existing test failures" above +2. Run typecheck: `bun run typecheck` +3. Check formatting: `bun run format:check` — if issues, fix with `bun run format` 4. Commit any remaining changes and push the branch 5. Push the branch, then create or update the PR: - If existing PR is "true": PR #${_merge_existing_pr_number_ref} already exists. Just push the branch — the PR updates automatically. @@ -564,6 +579,13 @@ if [ "${h.gha_steps("existing-pr", "pr_exists")}" = "true" ]; then BRANCH="${"$"}PR_BRANCH" else BRANCH="chore/upstream-sync-${"$"}(date +%Y%m%d)" + + # Delete stale remote branch if it exists (from a previous run with no PR) + if git ls-remote --heads origin "${"$"}BRANCH" | grep -q "${"$"}BRANCH"; then + echo "Deleting stale remote branch ${"$"}BRANCH (no PR references it)" + git push origin --delete "${"$"}BRANCH" || true + fi + git checkout -b "${"$"}BRANCH" fi @@ -620,7 +642,7 @@ fi""" github_token = h.gha_secret("CLAUDE_BOT_TOKEN") platform = "forgejo" show_full_output = "true" - allowed_tools = "Bash(git:*),Bash(bun:*),Bash(bunx:*),Bash(npx:*),Bash(./node_modules/.bin/*:*),Bash(curl:*),Read,Write,Edit,MultiEdit,Glob,Grep,WebFetch,WebSearch" + allowed_tools = "Bash(git:*),Bash(bun:*),Bash(bunx:*),Bash(npx:*),Bash(./node_modules/.bin/*:*),Bash(curl:*),Bash(echo:*),Bash(cat:*),Read,Write,Edit,MultiEdit,Glob,Grep,WebFetch,WebSearch" } env = { GITHUB_TOKEN = h.gha_secret("CLAUDE_BOT_TOKEN") diff --git a/.forgejo/workflows/upstream-sync.yml b/.forgejo/workflows/upstream-sync.yml index 628dcf9..b720ca9 100644 --- a/.forgejo/workflows/upstream-sync.yml +++ b/.forgejo/workflows/upstream-sync.yml @@ -248,6 +248,13 @@ jobs: BRANCH="$PR_BRANCH" else BRANCH="chore/upstream-sync-$(date +%Y%m%d)" + + # Delete stale remote branch if it exists (from a previous run with no PR) + if git ls-remote --heads origin "$BRANCH" | grep -q "$BRANCH"; then + echo "Deleting stale remote branch $BRANCH (no PR references it)" + git push origin --delete "$BRANCH" || true + fi + git checkout -b "$BRANCH" fi @@ -347,10 +354,25 @@ jobs: Prefer adapter patterns (separate Forgejo files) over inline modifications to upstream files. The fewer upstream files we modify, the fewer conflicts in the next sync. + ## IMPORTANT: Command execution rules + - GITHUB_SERVER_URL=https://github.com is ALREADY exported in your environment. Run commands directly WITHOUT env var prefixes. + CORRECT: `bun test` + WRONG: `GITHUB_SERVER_URL=https://github.com bun test` (this will be blocked by permissions) + - Do NOT use `2>&1` redirections, subshells (`sh -c`), or `export` commands — they will be blocked. + - For inspecting env vars, use: `bun -e "console.log(process.env.VAR_NAME)"` + - Available commands: git, bun, bunx, npx, curl, echo, and anything in ./node_modules/.bin/ + + ## Known pre-existing test failures + Some tests in comment-logic.test.ts and create-prompt.test.ts have pre-existing failures related to Forgejo URL patterns. + These are NOT caused by the upstream merge. When running `bun test`: + - If only comment-logic.test.ts and/or create-prompt.test.ts fail, that's expected — proceed. + - If OTHER test files fail that weren't failing before, investigate those as potential merge regressions. + - Do NOT spend time trying to fix pre-existing failures or trying different env var approaches. + ## After merge is resolved and reviewed - 1. Run tests: bun test (GITHUB_SERVER_URL is already set in the environment) - 2. Run typecheck: bun run typecheck - 3. Check formatting: bun run format:check — if issues, fix with bun run format + 1. Run tests: `bun test` — see "Known pre-existing test failures" above + 2. Run typecheck: `bun run typecheck` + 3. Check formatting: `bun run format:check` — if issues, fix with `bun run format` 4. Commit any remaining changes and push the branch 5. Push the branch, then create or update the PR: - If existing PR is "true": PR #${{ steps.existing-pr.outputs.pr_number }} already exists. Just push the branch — the PR updates automatically. @@ -368,7 +390,7 @@ jobs: github_token: ${{ secrets.CLAUDE_BOT_TOKEN }} platform: forgejo show_full_output: 'true' - allowed_tools: Bash(git:*),Bash(bun:*),Bash(bunx:*),Bash(npx:*),Bash(./node_modules/.bin/*:*),Bash(curl:*),Read,Write,Edit,MultiEdit,Glob,Grep,WebFetch,WebSearch + allowed_tools: Bash(git:*),Bash(bun:*),Bash(bunx:*),Bash(npx:*),Bash(./node_modules/.bin/*:*),Bash(curl:*),Bash(echo:*),Bash(cat:*),Read,Write,Edit,MultiEdit,Glob,Grep,WebFetch,WebSearch env: GITHUB_TOKEN: ${{ secrets.CLAUDE_BOT_TOKEN }} GITHUB_SERVER_URL: https://github.com