diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 59ecf28..1694efd 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -87,6 +87,41 @@ jobs: - name: Verify (lint, types, tests, coverage floor, required API) run: ./.upstreamer/scripts/verify.sh + # The port tracks upstream HEAD, so it is routinely AHEAD of the release + # whose version number pyproject.toml carries. Publishing from that state + # would ship unreleased upstream work as a released version — permanently, + # since a PyPI version can never be reused. + # + # verify.sh reports this, but only when a sync run has left an upstream + # checkout in tmp/. There is none here, so check it explicitly against the + # public repo rather than letting the guard be silent at the one moment it + # matters most. + - name: Refuse to publish a version the port is ahead of + run: | + set -euo pipefail + VERSION="$(grep -m1 '^version' pyproject.toml | sed 's/.*"\(.*\)".*/\1/')" + PORTED="$(grep -m1 '^upstream_commit:' .upstreamer/state.yaml | awk '{print $2}')" + echo "declared version: $VERSION" + echo "ported commit: $PORTED" + + rm -rf /tmp/upstream-check + git clone -q --filter=blob:none --no-checkout \ + https://github.com/OpenRouterTeam/typescript-agent.git /tmp/upstream-check + git -C /tmp/upstream-check fetch -q --tags origin + + TAG="@openrouter/agent@${VERSION}" + if ! git -C /tmp/upstream-check rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then + echo "::error::Upstream has no release tag $TAG. This package's version tracks the ported @openrouter/agent version, so publishing $VERSION means upstream released it. Wait for the upstream release, or correct the version." + exit 1 + fi + + AHEAD="$(git -C /tmp/upstream-check rev-list --count "refs/tags/$TAG..$PORTED" 2>/dev/null || echo 0)" + if [ "${AHEAD:-0}" -gt 0 ]; then + echo "::error::The ported commit is $AHEAD commit(s) ahead of the $TAG release tag. Publishing $VERSION now would ship unreleased upstream work under a released version number, and a PyPI version can never be reused. Publish from a commit level with a release tag, or wait for upstream to release what the port has reached." + exit 1 + fi + echo "Ported tree is level with $TAG — $VERSION is honest to publish." + - name: Build sdist and wheel run: | set -euo pipefail diff --git a/.github/workflows/upstreamer-port.yaml b/.github/workflows/upstreamer-port.yaml index 17317c6..499d0a6 100644 --- a/.github/workflows/upstreamer-port.yaml +++ b/.github/workflows/upstreamer-port.yaml @@ -1,10 +1,27 @@ name: Upstreamer Port -# Ports @openrouter/agent into this repo. Two triggers: -# 1. repository_dispatch from typescript-agent's publish.yaml on a new npm release -# (event type: openrouter-agent-published) — the intended path. Ports track -# published releases, not every commit to upstream main. -# 2. Weekly cron as a safety net for missed dispatches, plus manual dispatch. +# Ports @openrouter/agent into this repo. +# +# The port tracks upstream's **default-branch HEAD**, not the latest published npm +# release. Release tracking sounds safer but produces exactly the failure this +# pipeline exists to prevent: upstream can sit for weeks with large unreleased +# work on main (doom-loop detection, #73, was ~7.5k lines) and the port stays +# blind to it, then absorbs the whole delta in one automated run touching the most +# load-bearing modules. Tracking HEAD keeps each delta small enough to review. +# +# Consequence to keep in mind: the port is then routinely AHEAD of the latest +# release, so its declared version legitimately lags upstream's package.json. The +# verifier reports that rather than failing, and publishing is gated on it — see +# the Package Version section of .upstreamer/upstreamer.md. +# +# Three triggers, all resolving to HEAD unless given an explicit ref: +# 1. Weekly cron — the primary path now that releases are not the trigger. +# 2. repository_dispatch from typescript-agent's publish.yaml on a new npm +# release. Still useful as a "something just shipped, sync promptly" nudge, +# but it no longer pins the ref to that release tag: doing so would port +# BACKWARDS once the port is ahead of the release. scripts/upstream refuses +# an ancestor ref outright. +# 3. Manual dispatch, optionally with an explicit ref. # # Opens a PR. Never pushes to main. A failed parity eval leaves # .upstreamer/state.yaml unchanged, so the next run retries the same delta. @@ -39,6 +56,13 @@ jobs: port: runs-on: ubuntu-latest timeout-minutes: 150 + env: + # Surfaced as env because the `secrets` context is NOT available in a + # step-level `if:` (only github/needs/strategy/matrix/job/runner/env/vars/ + # steps/inputs are). Referencing secrets.* there evaluates to empty and the + # condition silently never matches — so the App-token gate below tests this + # variable instead. Only ever compared against '' ; never echoed. + HAS_APP_KEY: ${{ secrets.PORT_BOT_PRIVATE_KEY != '' }} steps: - uses: actions/checkout@v4 with: @@ -52,21 +76,29 @@ jobs: - name: Set up language toolchain uses: ./.github/actions/port-toolchain - # Ports track published releases, not upstream main. When no ref arrives - # (cron, or a manual dispatch with the input left blank), resolve the - # latest published @openrouter/agent version from the public npm registry - # and port its release tag. This makes the cron fully equivalent to the - # repository_dispatch fast path — same tag either way — so the pipeline - # works with no cross-repo token at all if the dispatch is unavailable. + # Blank ref = upstream default-branch HEAD, which scripts/upstream resolves + # itself. That is the normal case for both the cron and a publish dispatch. + # + # Only an EXPLICIT manual `ref` input is honored. The publish dispatch's + # client_payload.ref is deliberately ignored: it carries the release tag, + # which is an ancestor of HEAD once the port is ahead of the release, so + # honoring it would revert landed work. The dispatch still does its real + # job — waking the pipeline promptly after a release — it just syncs to HEAD + # like every other trigger. (scripts/upstream also refuses an ancestor ref + # outright, so this is defense in depth, not the only guard.) - name: Resolve target ref id: target run: | set -euo pipefail - REF="${{ inputs.ref || github.event.client_payload.ref }}" - if [ -z "$REF" ]; then - VERSION="$(curl -fsSL 'https://registry.npmjs.org/@openrouter%2Fagent/latest' | python3 -c 'import json,sys; print(json.load(sys.stdin)["version"])')" - REF="@openrouter/agent@${VERSION}" - echo "No ref provided — resolved latest npm release: $REF" + REF="${{ inputs.ref }}" + if [ -n "$REF" ]; then + echo "Explicit ref requested: $REF" + else + PAYLOAD_REF="${{ github.event.client_payload.ref }}" + if [ -n "$PAYLOAD_REF" ]; then + echo "::notice::Ignoring dispatch payload ref '$PAYLOAD_REF' — this port tracks upstream HEAD, and a release tag is an ancestor once the port is ahead of it. Syncing to HEAD instead." + fi + echo "No explicit ref — porting upstream default-branch HEAD." fi echo "ref=$REF" >> "$GITHUB_OUTPUT" @@ -84,9 +116,14 @@ jobs: echo "::error::OPENROUTER_API_KEY secret is not set. See .upstreamer/port.env.example." exit 1 fi - args=(--ref "${{ steps.target.outputs.ref }}") + # Only pass --ref when there is actually a ref. `--ref ""` is not the + # same as omitting it: the arg parser consumes the empty value and the + # script would target an empty ref instead of defaulting to HEAD. + args=() + REF="${{ steps.target.outputs.ref }}" + [ -n "$REF" ] && args+=(--ref "$REF") [ "${{ inputs.force }}" = "true" ] && args+=(--force) - ./scripts/upstream "${args[@]}" + ./scripts/upstream ${args[@]+"${args[@]}"} - name: Check for changes id: diff @@ -112,12 +149,48 @@ jobs: echo "passed=true" >> "$GITHUB_OUTPUT" fi + # Mint a GitHub App installation token so the PR is created by the App + # rather than by the native GITHUB_TOKEN. + # + # Why this exists: GitHub does not trigger workflows from events created + # with GITHUB_TOKEN (recursion guard), so a PR opened with it gets no + # pull_request-event checks — and `main`'s required status checks are + # satisfied ONLY by pull_request-event runs. Measured on PR #24: the commit + # had 14 check-runs, the PR's rollup showed 7; the workflow_dispatch half was + # invisible to branch protection. So the previous "dispatch ci.yaml + # explicitly" workaround produced green runs that could never satisfy the + # required checks, leaving an automated port PR permanently unmergeable. + # + # An App installation token is not recursion-guarded, so the PR gets real + # pull_request checks. Preferred over a PAT: scoped to this repo, not tied to + # a person's account, and independently revocable. + # + # Optional by design — see the fallback below. + - name: Mint App token + id: app-token + # Gate on BOTH halves. Gating on the App ID alone is a trap: the ID is a + # variable and the key is a secret, so they are added in separate places + # and one routinely lands first (it did here — the ID arrived first). With + # only the ID set, this step would run and fail on the missing key, turning + # a working fallback into a broken pipeline — worse than no App at all. + if: >- + steps.diff.outputs.changed == 'true' + && vars.PORT_BOT_APP_ID != '' + && env.HAS_APP_KEY == 'true' + uses: actions/create-github-app-token@v1 + with: + app-id: ${{ vars.PORT_BOT_APP_ID }} + private-key: ${{ secrets.PORT_BOT_PRIVATE_KEY }} + - name: Open PR id: open-pr if: steps.diff.outputs.changed == 'true' uses: peter-evans/create-pull-request@v6 with: - token: ${{ secrets.GITHUB_TOKEN }} + # App token when configured; GITHUB_TOKEN otherwise. With the fallback + # the PR still opens, but its checks will not attach — the guard step + # below says so loudly rather than leaving a silently stuck PR. + token: ${{ steps.app-token.outputs.token || secrets.GITHUB_TOKEN }} branch: upstreamer/sync delete-branch: true title: >- @@ -142,16 +215,23 @@ jobs: `.upstreamer/state.yaml` did not advance, the eval did not pass and this PR must not be merged as-is. - # Events created with the native GITHUB_TOKEN deliberately do not trigger - # other workflows (GitHub's recursion guard), so the PR opened above gets - # no CI checks on its own. workflow_dispatch is exempt from that guard: - # kick ci.yaml at the PR branch explicitly. This keeps the whole pipeline - # on the native token — no PAT anywhere in this repo. - - name: Trigger CI on the port PR - if: steps.diff.outputs.changed == 'true' && steps.open-pr.outputs.pull-request-operation != 'none' + # Fallback path only. With the App configured, the PR above already has real + # pull_request checks and nothing here runs. + # + # Without it, the PR exists but can never satisfy `main`'s required checks. + # Dispatching ci.yaml still gives a human something to read, but the run does + # NOT attach to the PR — so say that plainly instead of leaving a green-looking + # PR that will not merge and no explanation of why. + - name: Trigger CI on the port PR (no App token — checks will not attach) + if: >- + steps.diff.outputs.changed == 'true' + && steps.open-pr.outputs.pull-request-operation != 'none' + && steps.app-token.outputs.token == '' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: gh workflow run ci.yaml --repo "$GITHUB_REPOSITORY" --ref upstreamer/sync + run: | + echo "::warning::PORT_BOT_APP_ID / PORT_BOT_PRIVATE_KEY are not configured, so this PR was opened with GITHUB_TOKEN and will receive NO pull_request-event checks. main's required status checks cannot be satisfied, so the PR cannot merge as-is. The dispatched run below is informational only. Configure the App (see PORTING.md) or close and reopen the PR by hand to generate real checks." + gh workflow run ci.yaml --repo "$GITHUB_REPOSITORY" --ref upstreamer/sync - name: Upload logs if: always() diff --git a/.upstreamer/scripts/verify.sh b/.upstreamer/scripts/verify.sh index 556eb9a..a9d9d4f 100755 --- a/.upstreamer/scripts/verify.sh +++ b/.upstreamer/scripts/verify.sh @@ -86,6 +86,30 @@ if [ -f "$upstream_pkg" ]; then else fail "version drift: pyproject.toml=$declared, upstream @openrouter/agent=$target" fi + + # This port tracks upstream HEAD, so it routinely contains work that upstream + # has not released. package.json still carries the last released number, so the + # check above passes — and the port would publish "0.8.0" while containing + # post-0.8.0 commits. Not a verifier failure (being ahead is the intended + # state), but it must be visible, because publishing it as a released version + # number is a real misrepresentation and PyPI versions cannot be reused. + upstream_git="tmp/upstreamer/upstream" + if [ -d "$upstream_git/.git" ]; then + ported_sha="$(git -C "$upstream_git" rev-parse HEAD 2>/dev/null || true)" + rel_tag="$(git -C "$upstream_git" tag -l "@openrouter/agent@$target" | head -1)" + if [ -n "$ported_sha" ] && [ -n "$rel_tag" ]; then + ahead="$(git -C "$upstream_git" rev-list --count "$rel_tag^{commit}..$ported_sha" 2>/dev/null || echo 0)" + if [ "${ahead:-0}" -gt 0 ]; then + echo " NOTE: ported tree is $ahead commit(s) ahead of the $target release tag." + echo " Declaring $declared is correct for the port, but do NOT publish" + echo " $declared to PyPI from this state — it would ship unreleased" + echo " upstream work under a released version number. Publish only from" + echo " a commit level with a release tag, or after upstream releases." + else + pass "ported tree is level with the $target release tag (publishable)" + fi + fi + fi else # Only present during a sync run. Standalone/CI invocations legitimately have no # upstream checkout; not a failure, but say so rather than passing silently. diff --git a/.upstreamer/upstreamer.md b/.upstreamer/upstreamer.md index 99fa966..a4e9cd5 100644 --- a/.upstreamer/upstreamer.md +++ b/.upstreamer/upstreamer.md @@ -65,13 +65,24 @@ output. Leave them alone. ## Package Version `pyproject.toml` `version` tracks the ported `@openrouter/agent` version. Read it -from the upstream `packages/agent/package.json` at the target commit and set it -to match. If the target commit is between releases, keep the last released -version and note the drift in the final report. - -Publishing is gated on this: a released version can never be reused on PyPI, so a -sync that bumps the version is what makes the next release possible. Never bump it -past what was actually ported. +from the upstream `packages/agent/package.json` at the target commit and set it to +match. Never set it past what was actually ported. + +**This port targets upstream HEAD, so between-releases is the normal state, not the +exception.** `package.json` on `main` still carries the last released number, so +matching it is correct and the verifier passes — but the ported tree then contains +commits upstream has not released. When that is the case: + +- Keep `version` at the number `package.json` shows. Do not invent a + pre-release suffix; the verifier compares against `package.json` exactly. +- **Say so in the final report**: how many commits ahead of the release tag the + target is, and what unreleased upstream work is now included. The verifier + prints this as a `NOTE`, but the report is what a reviewer reads. + +Publishing is gated on this. A released version can never be reused on PyPI, so +publishing `X.Y.Z` from a tree that is ahead of upstream's `X.Y.Z` tag ships +unreleased work under a released number, permanently. Release from a commit level +with a release tag, or after upstream publishes the version the port has reached. ## Required Public API diff --git a/PORTING.md b/PORTING.md index 6924bbf..25b1c6e 100644 --- a/PORTING.md +++ b/PORTING.md @@ -9,27 +9,59 @@ listed in the Idiomatic Divergences section of `.upstreamer/upstreamer.md`. ## How it works +The port tracks upstream's **default-branch HEAD**, not the latest published npm +release. + ``` -typescript-agent publishes @openrouter/agent to npm - │ - │ repository_dispatch: openrouter-agent-published + weekly cron · publish dispatch · manual dispatch + │ (a nudge, (optional + │ not a ref) explicit ref) ▼ .github/workflows/upstreamer-port.yaml - │ + │ resolve ref: explicit input, else upstream HEAD ▼ scripts/upstream │ 1. fetch upstream, resolve target commit - │ 2. compare against .upstreamer/state.yaml — skip if unchanged - │ 3. opencode runs the port against .upstreamer/upstreamer.md - │ 4. .upstreamer/scripts/verify.sh (mechanical gate) - │ 5. .upstreamer/eval.md (parity gate, fresh context) - │ 6. advance state.yaml — ONLY if both gates pass + │ 2. REFUSE if target is behind state.yaml (would revert work) + │ 3. compare against .upstreamer/state.yaml — skip if unchanged + │ 4. opencode runs the port against .upstreamer/upstreamer.md + │ 5. .upstreamer/scripts/verify.sh (mechanical gate) + │ 6. .upstreamer/eval.md (parity gate, fresh context) + │ 7. advance state.yaml — ONLY if both gates pass ▼ - Pull request (never a direct push to main) + Pull request, opened by the GitHub App so it gets real + pull_request-event CI checks (never a direct push to main) ``` -A weekly cron backs up the dispatch in case one is missed, and -`workflow_dispatch` allows a manual run against any ref. +### Why HEAD and not the latest release + +Release tracking sounds more conservative and is worse in practice. Upstream can +sit for weeks with large unreleased work on `main` — doom-loop detection (#73) was +~7,500 lines, ~4,700 of it tests, and rewrote a big part of `model-result.ts`. A +release-tracking port stays blind to that, then absorbs the entire delta in one +automated run touching the most load-bearing module in the package. Tracking HEAD +keeps each delta small enough that a human can actually review it. + +Two consequences follow, and both are handled rather than ignored: + +**The port is routinely ahead of the latest release.** So a release ref is now +*dangerous*: it resolves to an ancestor of what is already ported, and the +converter would faithfully "port" the older tree, reverting landed work. +`scripts/upstream` refuses a target that is behind `state.yaml` (exit 3) unless +`--force` is given, and the workflow ignores the publish dispatch's +`client_payload.ref` for the same reason. + +**Its declared version legitimately lags upstream's `package.json`.** Being ahead +of a release means carrying commits upstream has not versioned yet, while +`package.json` still shows the last released number. The verifier reports how many +commits ahead the ported tree is and warns not to publish that version to PyPI — +shipping unreleased upstream work under a released version number is a +misrepresentation, and a PyPI version can never be reused. Publish from a commit +level with a release tag. + +The weekly cron is the primary trigger. The publish dispatch still fires on a new +npm release — useful as "something shipped, sync promptly" — but it syncs to HEAD +like everything else. `workflow_dispatch` allows a manual run against any ref. ## The contract is the product @@ -101,6 +133,42 @@ Two values, same names locally and in CI: | `OPENROUTER_API_KEY` | local: `.upstreamer/port.env` · CI: repo **secret** | `sk-or-…` key opencode uses for inference | | `OPENCODE_MODEL` | local: `.upstreamer/port.env` · CI: repo **variable** | e.g. `openrouter/~anthropic/claude-opus-latest` | +Two more are needed in CI only, for the bot that opens port PRs: + +| Name | Kind | What | +|------|------|------| +| `PORT_BOT_APP_ID` | repo **variable** | The GitHub App's App ID | +| `PORT_BOT_PRIVATE_KEY` | repo **secret** | The App's generated private key (full PEM, including the BEGIN/END lines) | + +### Why a GitHub App is required, not optional + +`main` requires six status checks, and **only `pull_request`-event runs satisfy +them**. GitHub does not trigger workflows from events created with the native +`GITHUB_TOKEN` (its recursion guard), so a PR opened with that token gets no +`pull_request` checks and can never become mergeable. + +Measured on PR #24: the commit carried **14 check-runs, while the PR's rollup +showed 7** — a `workflow_dispatch` run of the same workflow on the same commit was +completely invisible to branch protection. That is why "just dispatch `ci.yaml` +at the branch" does not work; it produces green runs that cannot satisfy anything. + +An App installation token is not recursion-guarded, so the PR it opens gets real +checks. Preferred over a PAT: scoped to this repo, not tied to anyone's personal +account, and revocable on its own. + +**Setup** — create a GitHub App (org Settings → Developer settings → GitHub Apps): + +- Repository permissions: **Contents: Read and write**, **Pull requests: Read and + write**. Nothing else. +- Install it on `OpenRouterTeam/python-agent`. +- Generate a private key, then add `PORT_BOT_APP_ID` (variable) and + `PORT_BOT_PRIVATE_KEY` (secret). + +Until those exist the pipeline still runs and still opens a PR, but emits a +`::warning::` saying the PR will receive no checks and cannot merge as-is. That is +deliberate — an unconfigured bot should degrade loudly, not look healthy while +producing permanently stuck PRs. + The wrapper writes the key into `~/.local/share/opencode/auth.json` so headless runs work without the interactive `opencode /connect` flow. diff --git a/pyproject.toml b/pyproject.toml index 1c1760e..9047731 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -77,6 +77,12 @@ include = [ [tool.ruff] line-length = 120 target-version = "py310" +# tmp/ is the port pipeline's scratch space (upstream checkout, probe scripts a +# sync run writes while exploring). It is gitignored, but ruff still walks it, so +# a stray scratch file there fails the verifier on grounds unrelated to the port. +# Ruff honors .gitignore for untracked files but not for an explicitly-walked +# directory, so exclude it outright. +extend-exclude = ["tmp"] [tool.ruff.lint] select = ["E", "F", "I", "UP", "B"] diff --git a/scripts/upstream b/scripts/upstream index e110791..d6fe930 100755 --- a/scripts/upstream +++ b/scripts/upstream @@ -149,6 +149,35 @@ echo "target: $target_commit${ref:+ ($ref)}" >&2 echo "last port: ${last_upstream_commit:-none}" >&2 echo "model: ${model:-}" >&2 +# Refuse to port backwards. +# +# The port tracks upstream's default branch, so it is routinely AHEAD of the +# latest published release. That makes a release ref actively dangerous: a +# publish dispatch (or a manual --ref v0.8.0) resolves to a commit that is an +# ancestor of what we already ported, and the converter would dutifully "port" +# the older tree — silently reverting everything landed since. +# +# Ancestor of last-ported, and not equal to it, means strictly behind. Unrelated +# histories (no merge-base) are not caught here; they surface as a normal diff +# for review rather than a silent revert. +if [ "$force" -eq 0 ] && [ -n "$last_upstream_commit" ] && [ "$target_commit" != "$last_upstream_commit" ] \ + && git -C "$upstream_dir" merge-base --is-ancestor "$target_commit" "$last_upstream_commit" 2>/dev/null; then + behind_by="$(git -C "$upstream_dir" rev-list --count "$target_commit..$last_upstream_commit" 2>/dev/null || echo "?")" + cat >&2 <