From dfdd41879c6c8a0a52601c06d33bd5ed8ee7cbf0 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Fri, 3 Jul 2026 11:46:00 -0400 Subject: [PATCH 01/17] feat(infra): upstream-sync triage ledger, cherry-pick hooks, and git cp (#118) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs(upstream-sync): add triage ledger + cherry-pick hook plan Seeds the upstream triage ledger (52 diverged commits from langchain-ai/open-swe categorized: landed/won't-merge/deferred/untriaged) and the design plan for a git-hook mechanism to keep it in sync during cherry-picks. * feat(upstream-sync): jsonl-backed triage ledger + generator CLI triage.jsonl is now the source of truth (52 rows migrated from triage.md); triage.md is generated with a do-not-edit banner. scripts/triage.py provides migrate/generate/reconcile/lookup/check-reject/set; make triage-render/check/reconcile added (triage-check is CI-safe staleness gate). Stdlib-only so git hooks can call it. * feat(upstream-sync): cherry-pick triage git hooks + git cp wrapper post-commit journals each -x pick to an untracked .git-local journal; prepare-commit-msg hard-blocks known-reject picks (commit-msg is a secondary backstop — clean picks skip it on git 2.50.1), overridable via git cp --force / SH_CHERRYPICK_ALLOW_REJECT=1 / sh.cherrypick.blockRejects=false. git-cp is the pre-apply guard + auto-reconcile. pre-push is a SHIM that re-execs the global Sea Haven security pre-push so core.hooksPath=.githooks does not shadow it; install-hooks.sh verifies that shim FIRST and refuses if it is missing. * docs(upstream-sync): correct hook plan + git cp runbook Record the verified git 2.50.1 finding that clean cherry-picks skip commit-msg, so the block lives in prepare-commit-msg; note the locked HARD-BLOCK-by-default reject policy. CHERRYPICK.md now leads with make install-hooks + git cp and explains the generated-md ledger. * chore(upstream-sync): mark #1651 landed (Bedrock family fix on gateway-routing) * docs(upstream-sync): rewrite CHERRYPICK.md as a repo-specific runbook * feat(upstream-sync): add triage.py sync + make triage-sync Discovers commits on dev..upstream/main not yet in the ledger and appends them as untriaged (PR # and subject parsed from each commit), then bumps _meta 'last synced' to the upstream tip and regenerates triage.md. Closes the discovery side of the workflow: triage-sync to pull in new work, git cp to land it. * docs(upstream-sync): move cherry-pick runbook to PR1 branch as cherry-pick-runbook.md --- .githooks/_reject_guard.sh | 56 ++ .githooks/commit-msg | 17 + .githooks/post-commit | 28 + .githooks/pre-push | 15 + .githooks/prepare-commit-msg | 16 + Makefile | 27 +- docs/upstream-sync/cherry-pick-hook-plan.md | 436 ++++++++++++++ docs/upstream-sync/triage.jsonl | 53 ++ docs/upstream-sync/triage.md | 66 +++ scripts/git-cp | 124 ++++ scripts/install-hooks.sh | 56 ++ scripts/triage.py | 615 ++++++++++++++++++++ 12 files changed, 1508 insertions(+), 1 deletion(-) create mode 100644 .githooks/_reject_guard.sh create mode 100755 .githooks/commit-msg create mode 100755 .githooks/post-commit create mode 100755 .githooks/pre-push create mode 100755 .githooks/prepare-commit-msg create mode 100644 docs/upstream-sync/cherry-pick-hook-plan.md create mode 100644 docs/upstream-sync/triage.jsonl create mode 100644 docs/upstream-sync/triage.md create mode 100755 scripts/git-cp create mode 100755 scripts/install-hooks.sh create mode 100755 scripts/triage.py diff --git a/.githooks/_reject_guard.sh b/.githooks/_reject_guard.sh new file mode 100644 index 00000000..b46a814e --- /dev/null +++ b/.githooks/_reject_guard.sh @@ -0,0 +1,56 @@ +# shellcheck shell=bash +# Shared cherry-pick reject guard. SOURCED (never executed directly) by both +# .githooks/prepare-commit-msg and .githooks/commit-msg. Git only executes files whose +# names exactly match a hook name, so this underscore-prefixed helper is ignored by git. +# +# Recovers the upstream SHA of an in-progress cherry-pick and HARD-BLOCKS (returns 1) when +# the triage ledger marks it "Won't merge". No-op for anything that is not a cherry-pick. +# Fail-safe: only a confirmed, non-overridden reject returns 1; every other path returns 0. +# +# Override an intentional re-pick with either: +# SH_CHERRYPICK_ALLOW_REJECT=1 git cherry-pick -x +# git config sh.cherrypick.blockRejects false # warn-only for this repo + +sh_cherrypick_reject_guard() { + local msgfile="${1:-}" + local gd up_sha root triage py reason rc + + gd="$(git rev-parse --absolute-git-dir 2>/dev/null)" || return 0 + + up_sha="" + if [ -f "$gd/CHERRY_PICK_HEAD" ]; then + up_sha="$(tr -d '[:space:]' <"$gd/CHERRY_PICK_HEAD" 2>/dev/null)" + fi + if [ -z "$up_sha" ] && [ -n "$msgfile" ] && [ -f "$msgfile" ]; then + up_sha="$(sed -n 's/.*cherry picked from commit \([0-9a-f]\{7,40\}\).*/\1/p' "$msgfile" | tail -1)" + fi + [ -z "$up_sha" ] && return 0 # not a cherry-pick -> no-op (normal commits untouched) + + root="$(git rev-parse --show-toplevel 2>/dev/null)" || return 0 + triage="$root/scripts/triage.py" + [ -f "$triage" ] || return 0 + py="$(command -v python3 || command -v python 2>/dev/null)" + [ -n "$py" ] || return 0 + + reason="$("$py" "$triage" check-reject "$up_sha" 2>/dev/null)" + rc=$? + [ "$rc" -eq 3 ] || return 0 # rc 0 = ok; rc 2 = ledger error -> fail-safe; only rc 3 blocks + + echo "" >&2 + echo "sh-cherrypick: BLOCKED — ${up_sha:0:12} is marked \"Won't merge\" in the triage ledger." >&2 + echo " reason: ${reason:-}" >&2 + + if [ "${SH_CHERRYPICK_ALLOW_REJECT:-}" = "1" ]; then + echo " override: SH_CHERRYPICK_ALLOW_REJECT=1 — allowing this pick." >&2 + return 0 + fi + if [ "$(git config --bool sh.cherrypick.blockRejects 2>/dev/null || echo true)" = "false" ]; then + echo " override: sh.cherrypick.blockRejects=false — warn-only, allowing." >&2 + return 0 + fi + + echo " To re-evaluate intentionally: SH_CHERRYPICK_ALLOW_REJECT=1 git cherry-pick -x " >&2 + echo " or repo-wide warn-only: git config sh.cherrypick.blockRejects false" >&2 + echo " Recover this pick: git cherry-pick --abort (or --skip)" >&2 + return 1 +} diff --git a/.githooks/commit-msg b/.githooks/commit-msg new file mode 100755 index 00000000..9d01427d --- /dev/null +++ b/.githooks/commit-msg @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +# sh-cherrypick commit-msg: SECONDARY reject backstop. +# +# commit-msg does NOT fire on a clean `git cherry-pick` auto-commit (that path is caught by +# prepare-commit-msg + post-commit); it does fire on the `git commit`-backed path such as +# `git cherry-pick --continue`. On that path prepare-commit-msg already runs first and blocks, +# so this hook is defense-in-depth against git versions/config where prepare-commit-msg is +# bypassed. Shares the exact same guard logic. +# +# Fail-safe: the only non-zero exit is the deliberate reject block; normal commits are no-ops. +set -uo pipefail + +# shellcheck source=_reject_guard.sh +. "$(dirname "$0")/_reject_guard.sh" + +sh_cherrypick_reject_guard "${1:-}" || exit 1 +exit 0 diff --git a/.githooks/post-commit b/.githooks/post-commit new file mode 100755 index 00000000..bc45e2ee --- /dev/null +++ b/.githooks/post-commit @@ -0,0 +1,28 @@ +#!/usr/bin/env bash +# sh-cherrypick post-commit: after a `git cherry-pick -x` lands, record the upstream SHA +# (recovered from the `(cherry picked from commit )` trailer) + the new local SHA into +# an UNTRACKED .git-local journal. Never edits tracked files. No-op for normal commits. +# Fail-safe: any internal error exits 0 so a commit is never aborted here. +set -uo pipefail + +{ + msg="$(git log -1 --format=%B 2>/dev/null)" || exit 0 + up_sha="$(printf '%s\n' "$msg" \ + | sed -n 's/.*cherry picked from commit \([0-9a-f]\{7,40\}\).*/\1/p' | tail -1)" + [ -z "$up_sha" ] && exit 0 # not a cherry-pick (or no -x) -> leave normal commits untouched + + local_sha="$(git rev-parse HEAD 2>/dev/null)" || exit 0 + gd="$(git rev-parse --absolute-git-dir 2>/dev/null)" || exit 0 + journal="$gd/sh-cherrypick-journal" + ts="$(date -u +%Y-%m-%dT%H:%M:%SZ)" + tab="$(printf '\t')" + + if ! { [ -f "$journal" ] && grep -q "^${up_sha}${tab}" "$journal" 2>/dev/null; }; then + printf '%s\t%s\t%s\n' "$up_sha" "$local_sha" "$ts" >> "$journal" + fi + + n="$(grep -c . "$journal" 2>/dev/null || echo '?')" + echo "sh-cherrypick: journaled ${up_sha:0:12} (${n} pending) — run: make triage-reconcile" >&2 +} || true + +exit 0 diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 00000000..9eb24917 --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +# sh-cherrypick pre-push SHIM. +# +# core.hooksPath is a SINGLE directory, not a search path. Pointing this repo at .githooks/ +# SHADOWS the machine-global hook dir (~/.config/git/hooks), which holds the MANDATORY Sea +# Haven security pre-push gate. To avoid silently disabling that gate, this shim re-execs the +# global pre-push, passing through args + stdin and preserving its exit code. +GLOBAL="${SH_GLOBAL_HOOKS:-$HOME/.config/git/hooks}/pre-push" + +if [ -x "$GLOBAL" ]; then + exec "$GLOBAL" "$@" +fi + +echo "sh-cherrypick: no global pre-push at $GLOBAL — nothing to delegate to." >&2 +exit 0 diff --git a/.githooks/prepare-commit-msg b/.githooks/prepare-commit-msg new file mode 100755 index 00000000..98622bf7 --- /dev/null +++ b/.githooks/prepare-commit-msg @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# sh-cherrypick prepare-commit-msg: PRIMARY reject backstop for a raw `git cherry-pick`. +# +# This (not commit-msg) is the hook that fires on a CLEAN cherry-pick auto-commit. Git runs +# prepare-commit-msg + post-commit on a clean pick but SKIPS pre-commit/commit-msg; commit-msg +# only fires on the `git commit`-backed path (`--continue`, normal commits). So the hard-block +# for known-reject picks lives here to cover clean picks too. +# +# Fail-safe: the only non-zero exit is the deliberate reject block; normal commits are no-ops. +set -uo pipefail + +# shellcheck source=_reject_guard.sh +. "$(dirname "$0")/_reject_guard.sh" + +sh_cherrypick_reject_guard "${1:-}" || exit 1 +exit 0 diff --git a/Makefile b/Makefile index d1268071..57ca0e49 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ -.PHONY: all format format-check lint test tests integration_tests help run dev +.PHONY: all format format-check lint test tests integration_tests help run dev \ + install-hooks triage-sync triage-reconcile triage-render triage-check # Default target executed when no arguments are given to make. all: help @@ -53,6 +54,25 @@ format: format-check: uv run ruff format $(PYTHON_FILES) --check +###################### +# UPSTREAM SYNC / CHERRY-PICK TRIAGE +###################### + +install-hooks: + bash scripts/install-hooks.sh + +triage-sync: + python3 scripts/triage.py sync + +triage-reconcile: + python3 scripts/triage.py reconcile + +triage-render: + python3 scripts/triage.py generate + +triage-check: + python3 scripts/triage.py generate --check + ###################### # HELP ###################### @@ -66,3 +86,8 @@ help: @echo 'lint - run linters' @echo 'test - run unit tests' @echo 'integration_tests - run integration tests' + @echo 'install-hooks - install cherry-pick triage git hooks (per clone)' + @echo 'triage-sync - fetch upstream + add new dev..upstream/main commits as untriaged' + @echo 'triage-reconcile - drain cherry-pick journal into the triage ledger' + @echo 'triage-render - regenerate docs/upstream-sync/triage.md' + @echo 'triage-check - fail if triage.md is stale vs triage.jsonl (CI)' diff --git a/docs/upstream-sync/cherry-pick-hook-plan.md b/docs/upstream-sync/cherry-pick-hook-plan.md new file mode 100644 index 00000000..d9357403 --- /dev/null +++ b/docs/upstream-sync/cherry-pick-hook-plan.md @@ -0,0 +1,436 @@ +# Cherry-pick triage-ledger sync — design plan + +Status: **design only** (nothing here is installed or wired yet). This document is the +build spec for a git-hook mechanism that keeps the upstream-sync triage ledger in sync +during `git cherry-pick -x`, identically for a human at the terminal and for Claude Code +driving git. Companion runbook: `../../CHERRYPICK.md`. + +--- + +## 1. Problem statement and the "no cherry-pick hook exists" reality + +This is a long-lived fork of `langchain-ai/open-swe` (remote `upstream`). We pull upstream +commits one at a time via `git cherry-pick -x `. A human keeps a triage ledger at +`docs/upstream-sync/triage.md` recording, **per upstream SHA**, a disposition: + +- **Landed** — cherry-picked into the fork. +- **Won't-merge** — already-in-dev / regression / tooling-rejected (a decided *no*). +- **Deferred→\** — parked for later on a named branch. +- **Untriaged** — seen but not yet decided. + +The ledger keys every row on the **upstream SHA** because it is stable; cherry-pick rewrites +the SHA locally, so the local SHA is not a durable key. + +We want two behaviors during a cherry-pick: + +1. **Auto-land:** when a pick succeeds, move that upstream SHA into the *Landed* section + from wherever it currently sits. +2. **Reject-warning:** when someone cherry-picks a SHA the ledger marks *Won't-merge*, warn + them as early as possible (ideally before the change is applied). + +### The hard reality + +**Git has no `pre-cherry-pick` or `post-cherry-pick` hook.** The only hooks that fire during +a cherry-pick are, per successfully-applied commit: + +``` +prepare-commit-msg → commit-msg → post-commit +``` + +There is **no** native hook at the *start* of a cherry-pick and **no** hook that sees the +list of SHAs about to be picked. Everything below is designed around that fact — we do not +invent a hook that does not exist. + +> **Build correction (verified on git 2.50.1).** A *clean* cherry-pick auto-commit runs only +> `prepare-commit-msg` **and** `post-commit` — it **skips** `pre-commit` and `commit-msg`. +> `commit-msg` fires only on the `git commit`-backed path (a normal commit, or +> `git cherry-pick --continue` after a conflict). So the reject **hard-block must live in +> `prepare-commit-msg`** (fires on every pick, clean or resolved), with `commit-msg` kept only +> as a secondary backstop. The original plan named `commit-msg` as the primary gate; that would +> silently miss every clean pick. `prepare-commit-msg` returning non-zero aborts the commit +> cleanly and leaves `CHERRY_PICK_HEAD` in place, so `--abort/--skip/--continue` still recover. + +Two signals are load-bearing: + +- **`.git/CHERRY_PICK_HEAD`** exists *while a pick is in progress* and contains the **full + upstream SHA** being applied. It is present at `prepare-commit-msg` and `commit-msg` time + (before the commit object is finalized) and is **gone** by `post-commit`. +- **The `-x` trailer** `(cherry picked from commit )` is written into the commit + message by `git cherry-pick -x`. It is present in the message file at `commit-msg` time and + is recoverable from `git log -1 --format=%B` at `post-commit` time. + +So `commit-msg` is the **earliest gate that can act with knowledge of the upstream SHA**, and +`post-commit` is the **only reliable "the pick actually landed" signal**. + +--- + +## 2. Chosen architecture + +Two hooks plus a thin wrapper and a small Python CLI. Division of labour: + +| Component | Fires / runs | Job | Touches tracked files? | +|---|---|---|---| +| `.githooks/prepare-commit-msg` | per pick (incl. clean auto-commit), before commit object is finalized | Behavior #2 **primary** hard-block: recover upstream SHA, block if ledger says *Won't-merge* (override to allow) | No | +| `.githooks/commit-msg` | `git commit`-backed path only (`--continue`, normal commit) | Behavior #2 **secondary** backstop (same guard); clean picks skip this hook | No | +| `.githooks/_reject_guard.sh` | sourced by both hooks above | Shared reject-guard function (not a hook — git ignores non-hook-named files) | No | +| `.githooks/post-commit` | per pick, after commit object exists | Behavior #1: recover upstream SHA + new local SHA, append to an **untracked** `.git/` journal | No | +| `scripts/triage.py reconcile` | end of a pick run (auto under wrapper, one command otherwise) | Drain journal → set those SHAs to *Landed* in `triage.jsonl` → regenerate `triage.md` → stage both | Yes (once, as a follow-up commit) | +| `scripts/git-cp` (wrapper / `git cp` alias) | user-invoked instead of raw cherry-pick | True pre-warning: check ledger **before** any tree change; then `cherry-pick -x`; then auto-`reconcile` | via reconcile | +| `.githooks/pre-push` | on push | **Shim** that re-invokes the global Sea Haven security `pre-push` (see §6) | No | + +**Why hooks never edit the tracked ledger directly:** editing `triage.md`/`triage.jsonl` +inside a hook during a multi-pick sequence leaves the tracked file dirty *between* picks +(see §4). We avoid that entirely — hooks only ever append to an untracked `.git/`-local +journal; the tracked ledger is mutated exactly once, by an explicit `reconcile`, as its own +commit. + +### End-to-end: single pick + +``` +$ git cp -x A # wrapper (recommended). Raw `git cherry-pick -x A` also works. + │ + │ (wrapper) pre-check A against triage.jsonl + │ └─ A is Won't-merge → print reason, require --force to proceed ◄─ true pre-warning + │ + ├─ git cherry-pick -x A + │ │ applies A's diff to index/worktree + │ ├─ prepare-commit-msg (unused) + │ ├─ commit-msg CHERRY_PICK_HEAD=A present → look up A + │ │ └─ Won't-merge? loud stderr warning (exit 0 by default) + │ │ finalize commit object A' with -x trailer + │ └─ post-commit CHERRY_PICK_HEAD gone; parse -x trailer → A + │ append "AA'landed" to .git/sh-cherrypick-journal + │ + └─ (wrapper) scripts/triage.py reconcile + drain journal → triage.jsonl: A→landed (local_sha=A') + regenerate triage.md → git add both → report "1 landed; commit the ledger" +``` + +Raw `git cherry-pick -x A` runs everything except the wrapper's pre-check and the auto-reconcile; +`post-commit` still journals, and it prints `N pick(s) journaled — run: make triage-reconcile`. + +### End-to-end: multi-pick sequence `git cp -x A B C` + +``` +wrapper pre-check A,B,C ── any Won't-merge? → list them, require --force + │ +git cherry-pick -x A B C (git sequencer) + A → commit-msg(warn?) → post-commit → journal: A A' + B → commit-msg(warn?) → post-commit → journal: B B' + C → commit-msg(warn?) → post-commit → journal: C C' + │ (tracked ledger NEVER touched mid-sequence → no dirty-tree hazard, §4) + │ +wrapper → scripts/triage.py reconcile + drain {A,B,C} → triage.jsonl all→landed → regenerate triage.md → stage → one report +``` + +If the sequence stops on a conflict at B: A is already journaled. The user resolves and +`git cherry-pick --continue` (B and C journal as they land). Because reconcile is journal-driven +and idempotent, running it after the sequence finally completes lands exactly A, B, C once. +Under raw cherry-pick the user runs `make triage-reconcile` at the end; the journal survived the +conflict pause because it lives in `.git/`, untouched by the sequencer. + +--- + +## 3. Ledger data model — machine source of truth + generated markdown (**recommended**) + +**Decision: `triage.jsonl` is the source of truth; `triage.md` is generated from it.** +Do *not* have hooks parse/edit the human markdown table. + +- **`docs/upstream-sync/triage.jsonl`** — one JSON object per line, the canonical record. +- **`docs/upstream-sync/triage.md`** — generated view with a `` + banner, rendered by `scripts/triage.py render`. Grouped into the same sections/columns the + human ledger uses today (`| sha | #pr | subject | reason |`). + +Record schema (one line): + +```json +{"sha":"","pr":123,"subject":"...","disposition":"landed", + "reason":"...","deferred_branch":null,"local_sha":"", + "updated":"2026-07-02T00:00:00Z"} +``` + +`disposition ∈ {landed, wont-merge, deferred, untriaged}`; `deferred_branch` set only when +`deferred`. Render maps `deferred` rows into a `Deferred→` subsection. + +### Why not edit the markdown directly + +- Editing a human-formatted markdown table from a shell hook is the fragile path the brief + warns about: alignment, escaped pipes in subjects, multi-line reasons, section boundaries, + and hand-edits all break naive `sed`/`awk`. One malformed edit corrupts the ledger. +- JSONL is append/patch-friendly, trivially greppable (`grep '"sha":""'` for the + reject-check), has clean line-oriented diffs, and is mutated safely by a tiny Python with + real JSON parsing. The repo already has a Python `scripts/` dir and `uv`, so a + `scripts/triage.py` CLI is idiomatic here and far more robust than shell string-surgery. +- **JSONL over TSV:** dispositions carry structure (`deferred_branch`, `local_sha`, `pr`) and + `reason`/`subject` are free text that can contain tabs — TSV would need escaping rules JSONL + gives for free. +- Humans still get a readable, reviewable `triage.md` in PRs; they just edit it through + `triage.jsonl` (directly, or via `scripts/triage.py set …`). A `make triage-check` in + CI fails if `triage.md` is stale vs `triage.jsonl`, so the generated view can never drift. + +**Migration from today's markdown ledger:** a one-shot `scripts/triage.py import triage.md` +parses the current hand-written table into `triage.jsonl`, after which `triage.md` becomes a +generated artifact. This is a build task, not a runtime dependency. + +--- + +## 4. Behavior #1 — auto-move to Landed, step by step + +**SHA recovery.** `post-commit` runs after the commit object exists and `CHERRY_PICK_HEAD` is +already gone, so recover the upstream SHA from the `-x` trailer: + +```bash +msg="$(git log -1 --format=%B)" +up_sha="$(printf '%s\n' "$msg" | sed -n 's/.*cherry picked from commit \([0-9a-f]\{40\}\).*/\1/p' | tail -1)" +[ -z "$up_sha" ] && exit 0 # not a cherry-pick (or no -x) → no-op, normal commits are untouched +local_sha="$(git rev-parse HEAD)" +``` + +**Journal, don't edit.** Append to an **untracked** journal and dedupe: + +``` +.git/sh-cherrypick-journal # \t\t, one line per pick +``` + +The journal lives under `.git/` — outside version control and outside the working tree — so +it is invisible to `git status`, never conflicts with an incoming pick, and survives conflict +pauses in a sequence. `post-commit` does nothing else. + +**Multi-pick dirty-tree handling (the crux).** If the hook instead edited the tracked ledger +in place, every intermediate pick would leave `docs/upstream-sync/triage.*` modified and +unstaged. Analysis: + +- It would **not** hard-break the sequence: cherry-pick applies the *next* commit's diff to + the index, and upstream commits never touch our fork-only `docs/upstream-sync/` files, so a + dirty ledger is a file the incoming pick doesn't care about — git allows that. +- But it is still the wrong design: `git status` is polluted mid-run, the ledger edits get + interleaved with pick state, and — worst case — folding a ledger edit into a cherry-picked + commit would pollute the pristine `-x` provenance we depend on. There is also no reliable + in-hook signal for "this is the last pick" (`.git/sequencer/` is torn down racily, and a + single `git cherry-pick A` may never create a sequencer dir at all), so a hook cannot know + when to do the "final" reconcile. + +So the tracked ledger is mutated **once**, outside any hook, by `reconcile`: + +``` +scripts/triage.py reconcile + read .git/sh-cherrypick-journal + for each (upstream_sha, local_sha): triage.jsonl[sha].disposition = landed + triage.jsonl[sha].local_sha = local_sha + render triage.md from triage.jsonl + git add docs/upstream-sync/triage.jsonl docs/upstream-sync/triage.md + truncate the journal + print summary (does NOT commit — the human/agent commits the ledger separately) +``` + +`reconcile` runs automatically as the last step of the `git cp` wrapper (fully hands-off for +wrapper users and for Claude Code when it calls the wrapper). For raw `git cherry-pick`, +`post-commit` prints `N pick(s) journaled — run: make triage-reconcile`, and the user runs it +once at the end. Reconcile is idempotent: a drained journal reconciles to a no-op, and +re-landing an already-landed SHA is a no-op, so double-running is safe. + +The ledger update lands as its **own** commit, keeping cherry-picked commits pristine. + +--- + +## 5. Behavior #2 — block on known-reject: honest verdict + +> **Locked decision (overrides the recommendation below): HARD-BLOCK by default.** Picking a +> *Won't-merge* SHA is blocked by both the `git cp` pre-apply check and the +> `prepare-commit-msg` hook (the plan text below still discusses warn-only as an option; the +> shipped default is block). Documented overrides: `git cp --force`, env +> `SH_CHERRYPICK_ALLOW_REJECT=1`, or repo-wide `git config sh.cherrypick.blockRejects false`. + + +**Constraint:** by `commit-msg` the pick's diff is already staged in the index/worktree; by +`post-commit` the commit exists. **No hook can warn *before* the change is applied to the +tree** — the earliest a hook sees the SHA is `commit-msg`, and by then the diff is staged (the +commit just isn't finalized). A *true* pre-application warning is impossible with hooks alone. + +Three honest options: + +- **(a) `commit-msg` hard-block (`exit 1`).** Aborts the commit cleanly: the commit object is + not created, `CHERRY_PICK_HEAD` stays, the index holds the applied diff, and the user + recovers with `git cherry-pick --abort` / `--skip` / `--continue`. In a sequence it stops at + that commit. Downsides: it's *post-apply* (tree already changed), and hard-blocking a decided + SHA that the maintainer legitimately wants to re-pick is annoying and, if the hook ever + misfires, wedges a pick. +- **(b) `commit-msg` warn-only (`exit 0`).** Loud stderr warning with the recorded reason, but + the commit proceeds. Never wedges anything. Downside: also post-apply, and easy to miss in a + multi-pick scroll. +- **(c) Wrapper pre-check.** `git cp` reads `triage.jsonl` **before** calling cherry-pick and + refuses (or prompts) on a *Won't-merge* SHA — a **genuine pre-apply** warning, before any + tree change. Downside: only fires when people use the wrapper; raw `git cherry-pick` bypasses + it. + +**Recommendation: hook + wrapper — do both, with these defaults.** + +1. **`scripts/git-cp` wrapper (primary, true pre-warning).** Before invoking cherry-pick, look + up every requested SHA in `triage.jsonl`. If any is `wont-merge`, print the SHA, subject, + and reason and **abort** unless `--force` is passed. This is the real "stop before you apply + a known-no" guard, and it is the path we point both humans (`CHERRYPICK.md`) and Claude Code + at. Expose it as `git cp` via `git config alias.cp '!bash scripts/git-cp'`. +2. **`.githooks/commit-msg` backstop (catches raw `git cherry-pick`).** Recover the upstream + SHA from `CHERRY_PICK_HEAD` (fallback: the `-x` trailer in message file `$1`); if + `triage.jsonl` marks it `wont-merge`, print a loud stderr warning with the reason. + **Default: warn and `exit 0` (non-blocking).** Opt-in hard-block via + `git config sh.cherrypick.blockRejects true` for anyone who wants option (a). Blocking is + off by default so the hook can never wedge a legitimate pick or a normal commit. + +Rationale: the wrapper gives the *real* pre-warning we actually want; the hook guarantees that +even a raw `git cherry-pick` (or Claude Code shelling straight to git) still gets a visible +signal, without ever risking a wedged pick by default. + +--- + +## 6. Installation / bootstrapping via `core.hooksPath` + +Hooks live in an in-repo, version-controlled **`.githooks/`** so they're shared. Activation is +per-clone: + +```bash +git config core.hooksPath .githooks +``` + +Git does **not** auto-adopt a repo's `core.hooksPath` (that would let a clone run arbitrary +code on checkout), so this one line is unavoidable per clone. Automate/ship it via: + +- **`scripts/install-hooks.sh`** — sets `core.hooksPath .githooks`, `chmod +x .githooks/*`, + and prints the global-hook note below. +- **`make hooks`** target calling that script; optionally invoke it from `make install` so a + standard setup wires hooks too. Document the one line in `CHERRYPICK.md` / `README`. + +### CRITICAL: collision with the global Sea Haven security `pre-push` + +This machine has a **global** hook path already configured: + +``` +core.hooksPath = ~/.config/git/hooks # holds the mandatory Sea Haven security pre-push gate +``` + +`core.hooksPath` is **a single directory, not a search path** — a repo-level +`core.hooksPath=.githooks` **overrides** the global one for this repo and would **silently +disable the security `pre-push` gate** here. That is a compliance violation, not a cosmetic +issue. + +**Mitigation (required): ship a `pre-push` shim in `.githooks/` that re-invokes the global +hook.** + +```bash +# .githooks/pre-push +#!/usr/bin/env bash +GLOBAL="${SH_GLOBAL_HOOKS:-$HOME/.config/git/hooks}/pre-push" +[ -x "$GLOBAL" ] && exec "$GLOBAL" "$@" +exit 0 # no global hook present → succeed, don't block the push +``` + +`install-hooks.sh` must detect a pre-existing global `core.hooksPath`, confirm the shim is in +place, and warn loudly if the global security hook exists but the shim is missing. If Sea Haven +ever adds more global hooks, add a matching shim for each (or a generic dispatcher that execs +every same-named hook under the global dir). This keeps the security gate intact while the +cherry-pick hooks run. + +--- + +## 7. Failure modes and safety guarantees + +- **Never abort/corrupt a normal commit.** Every hook first checks the cherry-pick signal + (`CHERRY_PICK_HEAD` for `commit-msg`, the `-x` trailer for `post-commit`) and no-ops + instantly otherwise. A plain `git commit` never reaches ledger logic. +- **Fail safe.** All hooks run `set -uo pipefail` and wrap their body so any internal error + (missing/mangled `triage.jsonl`, no Python, unreadable journal) results in `exit 0` — a hook + failure must never abort the user's git operation. The only path that can exit non-zero is + the *opt-in* `blockRejects` block, and that is a clean, recoverable cherry-pick abort. +- **No half-written tracked ledger.** Hooks only append to the untracked `.git/` journal; the + tracked ledger changes solely inside `reconcile`, which writes `triage.jsonl` + + regenerated `triage.md` atomically (write temp → `os.replace`) and stages them. A crash + mid-reconcile leaves the journal intact (source of truth for a re-run) and the tracked files + either fully old or fully new. +- **No mid-sequence dirty-tree hazard** (see §4): the tracked ledger is never touched during a + multi-pick run. +- **Idempotent + crash-tolerant.** Journal lines are deduped by upstream SHA; reconcile on a + drained journal is a no-op; re-landing an already-landed SHA is a no-op. Safe to run twice, + and safe across a conflict pause (journal survives in `.git/`). +- **Malformed source of truth.** If `triage.jsonl` fails to parse, `reconcile` aborts *without + writing* and leaves the journal intact; `commit-msg`/`post-commit` degrade to a stderr note + and `exit 0`. +- **Unknown SHA.** A picked SHA absent from the ledger is journaled and, on reconcile, inserted + as a new `landed` row (subject/PR backfilled from `git show`), so the ledger self-heals + instead of silently dropping the pick. +- **Missing Python / hooks not installed.** If `core.hooksPath` isn't set, nothing runs and + cherry-pick behaves normally (ledger just goes stale until someone reconciles) — no breakage. + +--- + +## 8. File/directory layout and implementation checklist + +### Files to create + +``` +.githooks/ + prepare-commit-msg # behavior #2 PRIMARY hard-block (fires on clean picks too) + commit-msg # behavior #2 secondary backstop (git commit / --continue path) + _reject_guard.sh # shared guard sourced by the two hooks above (not a hook itself) + post-commit # behavior #1: recover SHA from -x trailer, append to .git journal + pre-push # SHIM → global Sea Haven security pre-push (§6) +docs/upstream-sync/ + cherry-pick-hook-plan.md # this document + triage.jsonl # SOURCE OF TRUTH (created by the import step) + triage.md # GENERATED view (do-not-edit banner) +scripts/ + triage.py # CLI: import | set | check-reject | reconcile | render | lint + git-cp # wrapper: pre-check ledger → cherry-pick -x → reconcile + install-hooks.sh # sets core.hooksPath=.githooks, verifies pre-push shim vs global +# runtime, untracked (add to .gitignore is unnecessary — it lives under .git/): +.git/sh-cherrypick-journal +``` + +### `scripts/triage.py` subcommands + +- `import ` — one-shot migration of the current hand-written ledger → `triage.jsonl`. +- `set --disposition … [--pr … --subject … --reason … --branch …]` — human/agent edit path. +- `check-reject ` — exit non-zero + print reason iff `wont-merge` (used by hook + wrapper). +- `reconcile` — drain `.git/sh-cherrypick-journal` → mark landed → render → stage → summarize. +- `render [--check]` — regenerate `triage.md`; `--check` fails if stale (for CI). +- `lint` — validate `triage.jsonl` schema/dispositions. + +### Makefile targets + +- `hooks` → `bash scripts/install-hooks.sh` +- `triage-reconcile` → `uv run scripts/triage.py reconcile` +- `triage-render` → `uv run scripts/triage.py render` +- `triage-check` → `uv run scripts/triage.py render --check` (wire into CI) + +### Build checklist (ordered, no re-deciding required) + +1. **Ledger model.** Write `scripts/triage.py` with the schema in §3; implement `render` + (grouped sections + `| sha | #pr | subject | reason |`, do-not-edit banner) and `lint`. +2. **Migrate.** `triage.py import docs/upstream-sync/triage.md` → commit `triage.jsonl` + + regenerated `triage.md`; from here `triage.md` is generated. +3. **post-commit hook.** `-x` trailer recovery (§4), append `\t\t` to + `.git/sh-cherrypick-journal` (deduped), no-op on non-cherry-pick, `exit 0` on any error, + print the "run reconcile" reminder. +4. **reconcile.** Implement `triage.py reconcile` (drain → land → atomic render → stage → + truncate journal → summary; idempotent; self-heal unknown SHAs). +5. **commit-msg hook.** Recover SHA from `CHERRY_PICK_HEAD` (fallback `-x` trailer in `$1`); + `check-reject`; default warn + `exit 0`; opt-in `sh.cherrypick.blockRejects` → `exit 1`. +6. **git-cp wrapper + alias.** Pre-check all SHAs (abort on `wont-merge` unless `--force`) → + `git cherry-pick -x "$@"` → `triage.py reconcile`. Ship `alias.cp` setup in install script. +7. **pre-push shim (§6).** `.githooks/pre-push` execs the global security hook; make it the + first thing `install-hooks.sh` verifies. +8. **install-hooks.sh + make hooks.** Set `core.hooksPath=.githooks`, `chmod +x`, set `git cp` + alias, detect/reconcile the global-hooksPath collision, warn if the security shim is missing. +9. **Docs.** Update `CHERRYPICK.md` to lead with `git cp`, note the one-time `make hooks`, and + explain the warn/block behavior. Add `make triage-check` to CI so `triage.md` never drifts. +10. **Tests.** Cover: single pick lands; multi-pick sequence lands all once; conflict-pause then + `--continue` still lands correctly; reject warning fires (warn and block modes); normal + non-cherry-pick commit is untouched; hook errors never abort git; reconcile is idempotent; + `pre-push` shim delegates to the global security hook. + +### Open items for the human to confirm before build + +- **Default reject policy:** warn-only (recommended) vs block-by-default. Plan assumes warn-only + with opt-in block. +- **Branch target for the reconcile commit:** picks land on `chore/cherry-pick-*` off `dev` + (per `CHERRYPICK.md`); confirm the ledger commit rides the same branch/PR. diff --git a/docs/upstream-sync/triage.jsonl b/docs/upstream-sync/triage.jsonl new file mode 100644 index 00000000..79ea4071 --- /dev/null +++ b/docs/upstream-sync/triage.jsonl @@ -0,0 +1,53 @@ +{"_meta": {"last_synced": "73b7d1c0", "last_synced_date": "2026-07-02"}} +{"sha": "0b76afdc", "pr": 1653, "subject": "reviews block agenda, sticky headers, diff scroll", "disposition": "landed", "reason": "", "branch": "cherry-pick-upstream", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "7530653b", "pr": 1655, "subject": "ResizeObserver settle for review scroll-to", "disposition": "landed", "reason": "", "branch": "cherry-pick-upstream", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "23bd4a63", "pr": 1660, "subject": "top padding to sticky review block header", "disposition": "landed", "reason": "", "branch": "cherry-pick-upstream", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "9e5a1924", "pr": 1656, "subject": "purge expired thread_wakeup crons", "disposition": "landed", "reason": "", "branch": "cherry-pick-upstream", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "63eb9a08", "pr": 1661, "subject": "sidebar filter popover border tokens", "disposition": "landed", "reason": "", "branch": "cherry-pick-upstream", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "bc7ce591", "pr": 1668, "subject": "preserve dashboard redirect after login", "disposition": "landed", "reason": "", "branch": "cherry-pick-upstream", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "f32e492a", "pr": 1637, "subject": "return to thread after plan approval", "disposition": "landed", "reason": "", "branch": "cherry-pick-upstream", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "7ee3e057", "pr": 1636, "subject": "make plan view mobile friendly", "disposition": "landed", "reason": "", "branch": "cherry-pick-upstream", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "6575c327", "pr": 1654, "subject": "disable React StrictMode", "disposition": "landed", "reason": "kept fork's `PwaUpdateProvider`", "branch": "cherry-pick-upstream", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "c3292d82", "pr": 1611, "subject": "bake sfw binary into sandbox image", "disposition": "wont-merge", "reason": "already in dev", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "48bf712b", "pr": 1609, "subject": "show message timestamps", "disposition": "wont-merge", "reason": "already in dev", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "85c0f63e", "pr": 1620, "subject": "clickable shared PR header", "disposition": "wont-merge", "reason": "already in dev", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "db2ae58e", "pr": 1643, "subject": "pre-bundle shiki/@pierre deps", "disposition": "wont-merge", "reason": "already in dev", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "1d9da064", "pr": 1662, "subject": "bump astral-sh/setup-uv", "disposition": "wont-merge", "reason": "dev ahead (v8.2.0, `checkout@v7`)", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "83cb40a0", "pr": 1616, "subject": "update langsmith sdk to 0.9.3", "disposition": "wont-merge", "reason": "regression — dev has 0.9.6", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "00906401", "pr": 1610, "subject": "editable plan mode", "disposition": "wont-merge", "reason": "regression — dev plan-mode supersedes", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "e5a29eca", "pr": 1613, "subject": "plan links in PR descriptions", "disposition": "wont-merge", "reason": "regression — dev has async plan-ref", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "e1d85526", "pr": 1645, "subject": "switch ui to pnpm", "disposition": "wont-merge", "reason": "tooling — fork keeps bun", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "f5670f24", "pr": 1639, "subject": "require bun for ui agent work", "disposition": "deferred", "reason": "clean new file, aligned", "branch": "PR1", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "209132d3", "pr": 1621, "subject": "durable interrupt dispatch + completion webhook", "disposition": "deferred", "reason": "investigate first — may be applied", "branch": "durable-dispatch", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "02bb4dfd", "pr": 1658, "subject": "don't attach loopback run-complete webhooks", "disposition": "deferred", "reason": "", "branch": "durable-dispatch", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "29015fad", "pr": 1614, "subject": "gate workflow pushes with approval", "disposition": "deferred", "reason": "", "branch": "durable-dispatch", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "546042a4", "pr": 1652, "subject": "add workflow approval UI", "disposition": "deferred", "reason": "", "branch": "plan-approval", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "ae04b72b", "pr": 1635, "subject": "publish plans from sandbox files", "disposition": "deferred", "reason": "", "branch": "plan-approval", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "c03a6be7", "pr": 1634, "subject": "keep plan guidance high-level", "disposition": "deferred", "reason": "", "branch": "plan-approval", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "96cceb74", "pr": 1632, "subject": "notify Slack on plan approval", "disposition": "deferred", "reason": "", "branch": "plan-approval", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "2f56d754", "pr": 1618, "subject": "omit plan link when no plan exists", "disposition": "deferred", "reason": "likely regression", "branch": "plan-approval", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "ee224d3e", "pr": 1650, "subject": "add Slack reaction tool", "disposition": "deferred", "reason": "", "branch": "slack-tooling", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "747ce4bb", "pr": 1638, "subject": "add Slack breakout thread tool", "disposition": "deferred", "reason": "", "branch": "slack-tooling", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "27d90ef1", "pr": 1633, "subject": "include Slack channel context in prompts", "disposition": "deferred", "reason": "", "branch": "slack-tooling", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "4cd5fa5c", "pr": 1629, "subject": "avoid recapping Slack replies", "disposition": "deferred", "reason": "", "branch": "slack-tooling", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "92dbf6f9", "pr": 1630, "subject": "update Slack trace reply on web handoff", "disposition": "deferred", "reason": "", "branch": "slack-tooling", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "bb36448b", "pr": 1627, "subject": "surface Slack thread errors", "disposition": "deferred", "reason": "", "branch": "slack-tooling", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "73b7d1c0", "pr": 1678, "subject": "fix OpenAI Responses reasoning replay", "disposition": "deferred", "reason": "", "branch": "gateway-routing", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "5f7c2f46", "pr": 1674, "subject": "fix Fireworks Gateway base URL", "disposition": "deferred", "reason": "", "branch": "gateway-routing", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "702ef908", "pr": 1673, "subject": "dedicated LangSmith gateway API key", "disposition": "deferred", "reason": "", "branch": "gateway-routing", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "e9dc6e01", "pr": 1671, "subject": "opt-in LangSmith LLM Gateway routing", "disposition": "deferred", "reason": "", "branch": "gateway-routing", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "289f5e3a", "pr": 1651, "subject": "add Sonnet 5 to model picker", "disposition": "landed", "reason": "already in dev; added Bedrock family fallback fix (c16fb915)", "branch": "gateway-routing", "local_sha": null, "updated": "2026-07-03T00:19:50Z"} +{"sha": "5da3d0c6", "pr": 1624, "subject": "post reviewer resolution notes verbatim", "disposition": "deferred", "reason": "", "branch": "reviewer-misc", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "69148f54", "pr": 1612, "subject": "add PR trace resolution", "disposition": "deferred", "reason": "", "branch": "reviewer-misc", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "6d125526", "pr": 1625, "subject": "stop wrapping installs in sfw", "disposition": "deferred", "reason": "", "branch": "reviewer-misc", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "320bb39a", "pr": 1657, "subject": "opt-in tracemalloc for aiohttp sessions", "disposition": "deferred", "reason": "", "branch": "reviewer-misc", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "baf0c248", "pr": 1617, "subject": "filter & grouping menu in threads sidebar", "disposition": "deferred", "reason": "~998 LOC", "branch": "own branch", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "f29868ff", "pr": 1615, "subject": "recover thread work as patch", "disposition": "deferred", "reason": "~495 LOC", "branch": "own branch", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "8e0788dc", "pr": 1631, "subject": "show queued dashboard follow-ups", "disposition": "deferred", "reason": "", "branch": "own branch", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "9c601ca1", "pr": 1648, "subject": "add Stagehand-powered browser subagent", "disposition": "deferred", "reason": "", "branch": "own branch", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "8c944381", "pr": 1622, "subject": "restore forced tool call", "disposition": "deferred", "reason": "", "branch": "own branch", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "5dc360d8", "pr": 1619, "subject": "bump langgraph-checkpoint 4.1.0→4.1.1", "disposition": "untriaged", "reason": "", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "4f913198", "pr": 1647, "subject": "widen split review diffs", "disposition": "untriaged", "reason": "", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "20f63e8c", "pr": 1646, "subject": "install missing deps before verification", "disposition": "untriaged", "reason": "", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "89f886e2", "pr": 1642, "subject": "request actions read for sandbox logs", "disposition": "untriaged", "reason": "", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} +{"sha": "2f237b53", "pr": 1626, "subject": "fall back to vision model for image threads", "disposition": "untriaged", "reason": "", "branch": "", "local_sha": null, "updated": "2026-07-02T00:00:00Z"} diff --git a/docs/upstream-sync/triage.md b/docs/upstream-sync/triage.md new file mode 100644 index 00000000..8c353489 --- /dev/null +++ b/docs/upstream-sync/triage.md @@ -0,0 +1,66 @@ + + +# Upstream triage ledger + +Commits on `upstream/main` (langchain-ai/open-swe) not yet in `dev`, and the decision on each. +Rows key on the **upstream SHA** (stable across local cherry-picks). Deferred rows are provisional +— re-inspect before picking. See the fork-maintenance runbook in `CLAUDE.md`. + +**Last synced `upstream/main`:** `73b7d1c0` (2026-07-02) + +| sha | pr | subject | decision | why | branch | +|---|---|---|---|---|---| +| `0b76afdc` | #1653 | reviews block agenda, sticky headers, diff scroll | Landed | | cherry-pick-upstream | +| `7530653b` | #1655 | ResizeObserver settle for review scroll-to | Landed | | cherry-pick-upstream | +| `23bd4a63` | #1660 | top padding to sticky review block header | Landed | | cherry-pick-upstream | +| `9e5a1924` | #1656 | purge expired thread_wakeup crons | Landed | | cherry-pick-upstream | +| `63eb9a08` | #1661 | sidebar filter popover border tokens | Landed | | cherry-pick-upstream | +| `bc7ce591` | #1668 | preserve dashboard redirect after login | Landed | | cherry-pick-upstream | +| `f32e492a` | #1637 | return to thread after plan approval | Landed | | cherry-pick-upstream | +| `7ee3e057` | #1636 | make plan view mobile friendly | Landed | | cherry-pick-upstream | +| `6575c327` | #1654 | disable React StrictMode | Landed | kept fork's `PwaUpdateProvider` | cherry-pick-upstream | +| `289f5e3a` | #1651 | add Sonnet 5 to model picker | Landed | already in dev; added Bedrock family fallback fix (c16fb915) | gateway-routing | +| `c3292d82` | #1611 | bake sfw binary into sandbox image | Won't merge | already in dev | | +| `48bf712b` | #1609 | show message timestamps | Won't merge | already in dev | | +| `85c0f63e` | #1620 | clickable shared PR header | Won't merge | already in dev | | +| `db2ae58e` | #1643 | pre-bundle shiki/@pierre deps | Won't merge | already in dev | | +| `1d9da064` | #1662 | bump astral-sh/setup-uv | Won't merge | dev ahead (v8.2.0, `checkout@v7`) | | +| `83cb40a0` | #1616 | update langsmith sdk to 0.9.3 | Won't merge | regression — dev has 0.9.6 | | +| `00906401` | #1610 | editable plan mode | Won't merge | regression — dev plan-mode supersedes | | +| `e5a29eca` | #1613 | plan links in PR descriptions | Won't merge | regression — dev has async plan-ref | | +| `e1d85526` | #1645 | switch ui to pnpm | Won't merge | tooling — fork keeps bun | | +| `f5670f24` | #1639 | require bun for ui agent work | Deferred | clean new file, aligned | PR1 | +| `209132d3` | #1621 | durable interrupt dispatch + completion webhook | Deferred | investigate first — may be applied | durable-dispatch | +| `02bb4dfd` | #1658 | don't attach loopback run-complete webhooks | Deferred | | durable-dispatch | +| `29015fad` | #1614 | gate workflow pushes with approval | Deferred | | durable-dispatch | +| `546042a4` | #1652 | add workflow approval UI | Deferred | | plan-approval | +| `ae04b72b` | #1635 | publish plans from sandbox files | Deferred | | plan-approval | +| `c03a6be7` | #1634 | keep plan guidance high-level | Deferred | | plan-approval | +| `96cceb74` | #1632 | notify Slack on plan approval | Deferred | | plan-approval | +| `2f56d754` | #1618 | omit plan link when no plan exists | Deferred | likely regression | plan-approval | +| `ee224d3e` | #1650 | add Slack reaction tool | Deferred | | slack-tooling | +| `747ce4bb` | #1638 | add Slack breakout thread tool | Deferred | | slack-tooling | +| `27d90ef1` | #1633 | include Slack channel context in prompts | Deferred | | slack-tooling | +| `4cd5fa5c` | #1629 | avoid recapping Slack replies | Deferred | | slack-tooling | +| `92dbf6f9` | #1630 | update Slack trace reply on web handoff | Deferred | | slack-tooling | +| `bb36448b` | #1627 | surface Slack thread errors | Deferred | | slack-tooling | +| `73b7d1c0` | #1678 | fix OpenAI Responses reasoning replay | Deferred | | gateway-routing | +| `5f7c2f46` | #1674 | fix Fireworks Gateway base URL | Deferred | | gateway-routing | +| `702ef908` | #1673 | dedicated LangSmith gateway API key | Deferred | | gateway-routing | +| `e9dc6e01` | #1671 | opt-in LangSmith LLM Gateway routing | Deferred | | gateway-routing | +| `5da3d0c6` | #1624 | post reviewer resolution notes verbatim | Deferred | | reviewer-misc | +| `69148f54` | #1612 | add PR trace resolution | Deferred | | reviewer-misc | +| `6d125526` | #1625 | stop wrapping installs in sfw | Deferred | | reviewer-misc | +| `320bb39a` | #1657 | opt-in tracemalloc for aiohttp sessions | Deferred | | reviewer-misc | +| `baf0c248` | #1617 | filter & grouping menu in threads sidebar | Deferred | ~998 LOC | own branch | +| `f29868ff` | #1615 | recover thread work as patch | Deferred | ~495 LOC | own branch | +| `8e0788dc` | #1631 | show queued dashboard follow-ups | Deferred | | own branch | +| `9c601ca1` | #1648 | add Stagehand-powered browser subagent | Deferred | | own branch | +| `8c944381` | #1622 | restore forced tool call | Deferred | | own branch | +| `5dc360d8` | #1619 | bump langgraph-checkpoint 4.1.0→4.1.1 | Untriaged | | | +| `4f913198` | #1647 | widen split review diffs | Untriaged | | | +| `20f63e8c` | #1646 | install missing deps before verification | Untriaged | | | +| `89f886e2` | #1642 | request actions read for sandbox logs | Untriaged | | | +| `2f237b53` | #1626 | fall back to vision model for image threads | Untriaged | | | + +_Maintenance: after a `git sync`, add new `dev..upstream/main` SHAs as **Untriaged** (edit `triage.jsonl`) and bump "Last synced". A successful `git cherry-pick -x` auto-moves the row to **Landed** via the `post-commit` journal + `make triage-reconcile`._ diff --git a/scripts/git-cp b/scripts/git-cp new file mode 100755 index 00000000..823b93a5 --- /dev/null +++ b/scripts/git-cp @@ -0,0 +1,124 @@ +#!/usr/bin/env bash +# `git cp` wrapper — the real pre-apply guard the hooks can't be. +# +# git cp ... pre-check ledger, cherry-pick -x, auto-reconcile +# git cp --force ... pick even known "Won't merge" SHAs (documented override) +# git cp --continue|--abort|--skip|--quit forwarded to git cherry-pick +# +# Behaviour: +# 1. Resolve requested SHAs (single, list, or A^..B range) BEFORE touching the tree. +# 2. If any is disposition "wont-merge", print SHA + reason and ABORT unless --force. +# 3. git cherry-pick -x (ensures -x is present exactly once). +# 4. On success, run `scripts/triage.py reconcile` to land the picks in the ledger. +set -uo pipefail + +root="$(git rev-parse --show-toplevel 2>/dev/null)" || { + echo "git cp: not a git repository" >&2 + exit 1 +} +triage="$root/scripts/triage.py" +py="$(command -v python3 || command -v python 2>/dev/null)" + +force=0 +control=0 +passthru=() +picks=() + +for arg in "$@"; do + case "$arg" in + --force | --allow-reject) + force=1 + ;; + --continue | --abort | --skip | --quit) + control=1 + passthru+=("$arg") + ;; + -*) + passthru+=("$arg") + ;; + *) + passthru+=("$arg") + picks+=("$arg") + ;; + esac +done + +reconcile() { + if [ -n "$py" ] && [ -f "$triage" ]; then + "$py" "$triage" reconcile + else + echo "git cp: python/triage.py unavailable — skipping auto-reconcile" >&2 + fi +} + +ensure_x() { + # Prepend -x unless the caller already passed it. + for a in "${passthru[@]}"; do + [ "$a" = "-x" ] && return 0 + done + passthru=("-x" "${passthru[@]}") +} + +# Sequencer control verbs: forward and (for --continue) reconcile whatever landed. +if [ "$control" -eq 1 ]; then + git cherry-pick "${passthru[@]}" + rc=$? + if [ "$rc" -eq 0 ]; then + reconcile + fi + exit "$rc" +fi + +# Pre-apply reject check. +if [ -n "$py" ] && [ -f "$triage" ] && [ "${#picks[@]}" -gt 0 ]; then + resolved=() + for tok in "${picks[@]}"; do + if [[ "$tok" == *..* ]]; then + while IFS= read -r s; do + [ -n "$s" ] && resolved+=("$s") + done < <(git rev-list --reverse "$tok" 2>/dev/null) + else + s="$(git rev-parse --verify --quiet "${tok}^{commit}" 2>/dev/null)" && resolved+=("$s") + fi + done + + rejects=() + for s in ${resolved[@]+"${resolved[@]}"}; do + reason="$("$py" "$triage" check-reject "$s" 2>/dev/null)" + if [ "$?" -eq 3 ]; then + rejects+=("${s:0:12} $reason") + fi + done + + if [ "${#rejects[@]}" -gt 0 ]; then + echo "git cp: the following SHA(s) are marked \"Won't merge\" in the triage ledger:" >&2 + for r in "${rejects[@]}"; do + echo " ⛔ $r" >&2 + done + if [ "$force" -eq 0 ]; then + echo "" >&2 + echo "Refusing to cherry-pick a known-reject. To override intentionally: git cp --force ..." >&2 + exit 1 + fi + echo " --force set — proceeding anyway." >&2 + fi +fi + +ensure_x + +# --force must also satisfy the commit-msg backstop, so allow rejects through the hook too. +if [ "$force" -eq 1 ]; then + export SH_CHERRYPICK_ALLOW_REJECT=1 +fi + +git cherry-pick "${passthru[@]}" +rc=$? + +if [ "$rc" -eq 0 ]; then + reconcile +else + echo "" >&2 + echo "git cp: cherry-pick stopped (rc=$rc). Resolve, then: git cp --continue (or --abort/--skip)." >&2 + echo " Landed picks are journaled; reconcile runs on --continue or via make triage-reconcile." >&2 +fi +exit "$rc" diff --git a/scripts/install-hooks.sh b/scripts/install-hooks.sh new file mode 100755 index 00000000..e84077b2 --- /dev/null +++ b/scripts/install-hooks.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env bash +# Install the cherry-pick triage hooks for THIS clone. +# +# What it does (per-clone; git never auto-adopts a repo's core.hooksPath): +# 0. FIRST verify the Sea Haven global security pre-push still fires through our shim. +# 1. chmod +x the hooks + scripts. +# 2. git config core.hooksPath .githooks +# 3. git config alias.cp -> scripts/git-cp +# +# Safe to run repeatedly. +set -euo pipefail + +root="$(git rev-parse --show-toplevel)" +cd "$root" + +global_dir="${SH_GLOBAL_HOOKS:-$HOME/.config/git/hooks}" +global_pp="$global_dir/pre-push" +shim="$root/.githooks/pre-push" + +echo "==> [1/4] Verifying the Sea Haven global security pre-push will still fire..." +if [ -x "$global_pp" ]; then + if [ ! -f "$shim" ]; then + echo "FATAL: global security pre-push exists ($global_pp) but the shim ($shim) is MISSING." >&2 + echo " Setting core.hooksPath=.githooks would SHADOW and silently disable the security" >&2 + echo " gate. Refusing to install. Restore .githooks/pre-push first." >&2 + exit 1 + fi + if ! grep -q "$global_dir" "$shim" && ! grep -q 'SH_GLOBAL_HOOKS' "$shim"; then + echo "FATAL: shim ($shim) does not appear to delegate to the global hook dir. Refusing." >&2 + exit 1 + fi + if ! grep -q 'exec "\$GLOBAL"' "$shim"; then + echo "FATAL: shim ($shim) does not exec the global pre-push. Refusing." >&2 + exit 1 + fi + echo " OK: .githooks/pre-push shim re-execs $global_pp — security gate preserved." +else + echo " WARN: no global security pre-push found at $global_pp." + echo " If you expected the Sea Haven security gate, investigate BEFORE pushing." +fi + +echo "==> [2/4] Marking hooks + scripts executable..." +chmod +x "$root/.githooks/"* 2>/dev/null || true +chmod +x "$root/scripts/git-cp" "$root/scripts/install-hooks.sh" 2>/dev/null || true + +echo "==> [3/4] Pointing core.hooksPath at .githooks..." +git config core.hooksPath .githooks + +echo "==> [4/4] Installing the 'git cp' alias..." +git config alias.cp '!bash "$(git rev-parse --show-toplevel)/scripts/git-cp"' + +echo "" +echo "Done. This clone now:" +echo " - journals cherry-picks (post-commit) and blocks known-rejects (commit-msg)" +echo " - runs 'git cp' as the guarded cherry-pick wrapper" +echo " - STILL runs the global security pre-push via the .githooks/pre-push shim" diff --git a/scripts/triage.py b/scripts/triage.py new file mode 100755 index 00000000..c5d56838 --- /dev/null +++ b/scripts/triage.py @@ -0,0 +1,615 @@ +#!/usr/bin/env python3 +"""Upstream cherry-pick triage ledger tool. + +Source of truth is ``docs/upstream-sync/triage.jsonl`` (one JSON object per line); +``docs/upstream-sync/triage.md`` is a GENERATED, human-readable view of it. + +Subcommands: + migrate one-shot: parse the hand-written markdown ledger -> triage.jsonl + generate [--check] render triage.jsonl -> triage.md (--check: fail if md is stale) + reconcile drain .git journal -> mark SHAs landed -> render -> git add + sync add new base..ref commits (default dev..upstream/main) as untriaged + lookup print a SHA's disposition (+ reason) + check-reject exit 3 iff the SHA is disposition "wont-merge" (used by hooks) + set ... edit a row (disposition / pr / subject / reason / branch) + +Stdlib only. Hooks call this without uv, so keep it dependency-free. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import sys +from datetime import UTC, datetime +from pathlib import Path + +DISPOSITIONS = ("landed", "wont-merge", "deferred", "untriaged") + +DISPOSITION_LABELS = { + "landed": "Landed", + "wont-merge": "Won't merge", + "deferred": "Deferred", + "untriaged": "Untriaged", +} + +# Normalized label text -> disposition. Keys are lowercased with apostrophes stripped. +_LABEL_TO_DISPOSITION = { + "landed": "landed", + "wont merge": "wont-merge", + "wontmerge": "wont-merge", + "wont-merge": "wont-merge", + "deferred": "deferred", + "untriaged": "untriaged", +} + +GENERATED_BANNER = ( + "" +) + +PREAMBLE = ( + "# Upstream triage ledger\n" + "\n" + "Commits on `upstream/main` (langchain-ai/open-swe) not yet in `dev`, and the decision on each.\n" + "Rows key on the **upstream SHA** (stable across local cherry-picks). Deferred rows are provisional\n" + "— re-inspect before picking. See the fork-maintenance runbook in `CLAUDE.md`.\n" +) + +MAINTENANCE_NOTE = ( + "_Maintenance: after a `git sync`, add new `dev..upstream/main` SHAs as **Untriaged** " + '(edit `triage.jsonl`) and bump "Last synced". A successful `git cherry-pick -x` auto-moves ' + "the row to **Landed** via the `post-commit` journal + `make triage-reconcile`._\n" +) + +JOURNAL_NAME = "sh-cherrypick-journal" + + +# --------------------------------------------------------------------------- paths + + +def repo_root() -> Path: + try: + out = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + check=True, + ) + return Path(out.stdout.strip()) + except Exception: + return Path(__file__).resolve().parents[1] + + +def git_dir() -> Path | None: + try: + out = subprocess.run( + ["git", "rev-parse", "--absolute-git-dir"], + capture_output=True, + text=True, + check=True, + ) + return Path(out.stdout.strip()) + except Exception: + return None + + +def jsonl_path(root: Path) -> Path: + return root / "docs" / "upstream-sync" / "triage.jsonl" + + +def md_path(root: Path) -> Path: + return root / "docs" / "upstream-sync" / "triage.md" + + +# ------------------------------------------------------------------------- records + + +def _now_iso() -> str: + return datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%SZ") + + +def load_ledger(path: Path) -> tuple[dict, list[dict]]: + """Return (meta, records). First line may be a ``{"_meta": {...}}`` object.""" + meta: dict = {} + records: list[dict] = [] + if not path.exists(): + return meta, records + for lineno, raw in enumerate(path.read_text().splitlines(), start=1): + line = raw.strip() + if not line: + continue + try: + obj = json.loads(line) + except json.JSONDecodeError as exc: + raise ValueError(f"{path}:{lineno}: invalid JSON: {exc}") from exc + if "_meta" in obj: + meta = obj["_meta"] + continue + records.append(obj) + return meta, records + + +def write_ledger(path: Path, meta: dict, records: list[dict]) -> None: + lines = [json.dumps({"_meta": meta}, ensure_ascii=False)] + for rec in records: + lines.append(json.dumps(rec, ensure_ascii=False)) + tmp = path.with_suffix(path.suffix + ".tmp") + tmp.write_text("\n".join(lines) + "\n") + os.replace(tmp, path) + + +def sha_match(a: str, b: str) -> bool: + """Prefix match either direction — ledger holds short SHAs, journal holds full SHAs.""" + a = a.lower() + b = b.lower() + return a.startswith(b) or b.startswith(a) + + +def find_record(records: list[dict], sha: str) -> dict | None: + for rec in records: + if sha_match(rec.get("sha", ""), sha): + return rec + return None + + +# ------------------------------------------------------------------------- render + + +def _disposition_rank(disp: str) -> int: + try: + return DISPOSITIONS.index(disp) + except ValueError: + return len(DISPOSITIONS) + + +def render_md(meta: dict, records: list[dict]) -> str: + last_synced = meta.get("last_synced", "") + last_synced_date = meta.get("last_synced_date", "") + synced_line = "" + if last_synced: + synced_line = f"**Last synced `upstream/main`:** `{last_synced}`" + if last_synced_date: + synced_line += f" ({last_synced_date})" + synced_line += "\n" + + ordered = sorted( + enumerate(records), + key=lambda item: (_disposition_rank(item[1].get("disposition", "")), item[0]), + ) + + rows = [ + "| sha | pr | subject | decision | why | branch |", + "|---|---|---|---|---|---|", + ] + for _, rec in ordered: + sha = rec.get("sha", "") + pr = rec.get("pr") + pr_cell = f"#{pr}" if pr not in (None, "") else "" + subject = rec.get("subject", "") or "" + decision = DISPOSITION_LABELS.get(rec.get("disposition", ""), rec.get("disposition", "")) + why = rec.get("reason", "") or "" + branch = rec.get("branch", "") or "" + cells = [f"`{sha}`", pr_cell, subject, decision, why, branch] + rows.append("| " + " | ".join(cells) + " |") + + parts = [GENERATED_BANNER, "", PREAMBLE.rstrip("\n"), ""] + if synced_line: + parts.append(synced_line.rstrip("\n")) + parts.append("") + parts.append("\n".join(rows)) + parts.append("") + parts.append(MAINTENANCE_NOTE.rstrip("\n")) + return "\n".join(parts) + "\n" + + +# ------------------------------------------------------------------------ migrate + + +_ROW_RE = re.compile(r"^\|(.+)\|\s*$") + + +def _norm_label(text: str) -> str: + return re.sub(r"[^a-z\- ]", "", text.strip().lower()).strip() + + +def parse_md_ledger(text: str) -> tuple[dict, list[dict]]: + meta: dict = {} + records: list[dict] = [] + + m = re.search(r"Last synced.*?`([0-9a-f]{6,40})`\s*(?:\(([^)]*)\))?", text) + if m: + meta["last_synced"] = m.group(1) + if m.group(2): + meta["last_synced_date"] = m.group(2) + + for raw in text.splitlines(): + m = _ROW_RE.match(raw) + if not m: + continue + cells = [c.strip() for c in m.group(1).split("|")] + if len(cells) < 6: + continue + first = cells[0].strip("`").strip().lower() + # skip header + separator rows + if first in ("sha", "") or set(cells[0]) <= {"-", " "}: + continue + if not re.fullmatch(r"[0-9a-f]{7,40}", first): + continue + sha = first + pr_raw = cells[1].lstrip("#").strip() + pr = int(pr_raw) if pr_raw.isdigit() else None + subject = cells[2].strip() + disposition = _LABEL_TO_DISPOSITION.get(_norm_label(cells[3])) + if disposition is None: + raise ValueError(f"unrecognized decision {cells[3]!r} for {sha}") + reason = cells[4].strip() + branch = cells[5].strip() + updated = ( + f"{meta.get('last_synced_date')}T00:00:00Z" + if meta.get("last_synced_date") + else _now_iso() + ) + records.append( + { + "sha": sha, + "pr": pr, + "subject": subject, + "disposition": disposition, + "reason": reason, + "branch": branch, + "local_sha": None, + "updated": updated, + } + ) + return meta, records + + +# ------------------------------------------------------------------- subcommands + + +def cmd_migrate(args: argparse.Namespace) -> int: + root = repo_root() + src = Path(args.source) + if not src.exists(): + print(f"error: {src} not found", file=sys.stderr) + return 2 + meta, records = parse_md_ledger(src.read_text()) + out = jsonl_path(root) + out.parent.mkdir(parents=True, exist_ok=True) + write_ledger(out, meta, records) + print(f"migrated {len(records)} rows -> {out}") + md_path(root).write_text(render_md(meta, records)) + print(f"rendered {md_path(root)}") + return 0 + + +def cmd_generate(args: argparse.Namespace) -> int: + root = repo_root() + jl = jsonl_path(root) + try: + meta, records = load_ledger(jl) + except ValueError as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + rendered = render_md(meta, records) + md = md_path(root) + if args.check: + current = md.read_text() if md.exists() else "" + if current != rendered: + print( + f"error: {md} is stale vs {jl}. Run `make triage-render` and commit.", + file=sys.stderr, + ) + return 1 + print(f"ok: {md.name} is up to date with {jl.name}") + return 0 + md.write_text(rendered) + print(f"rendered {md}") + return 0 + + +def _read_journal(journal: Path) -> list[tuple[str, str, str]]: + entries: list[tuple[str, str, str]] = [] + seen: set[str] = set() + if not journal.exists(): + return entries + for raw in journal.read_text().splitlines(): + line = raw.strip() + if not line: + continue + parts = line.split("\t") + up = parts[0] + local = parts[1] if len(parts) > 1 else "" + ts = parts[2] if len(parts) > 2 else _now_iso() + if up in seen: + # keep the latest entry for a given upstream sha + entries = [e for e in entries if e[0] != up] + seen.add(up) + entries.append((up, local, ts)) + return entries + + +def cmd_reconcile(args: argparse.Namespace) -> int: + root = repo_root() + gd = git_dir() + if gd is None: + print("error: not a git repository", file=sys.stderr) + return 2 + journal = gd / JOURNAL_NAME + entries = _read_journal(journal) + if not entries: + print("reconcile: journal empty — nothing to do") + return 0 + + jl = jsonl_path(root) + try: + meta, records = load_ledger(jl) + except ValueError as exc: + print(f"error: {exc} — aborting without writing; journal preserved", file=sys.stderr) + return 2 + + landed = 0 + inserted = 0 + for up_sha, local_sha, ts in entries: + rec = find_record(records, up_sha) + if rec is None: + subject = "" + if local_sha: + try: + subject = subprocess.run( + ["git", "log", "-1", "--format=%s", local_sha], + capture_output=True, + text=True, + check=True, + ).stdout.strip() + except Exception: + subject = "" + records.append( + { + "sha": up_sha[:8], + "pr": None, + "subject": subject, + "disposition": "landed", + "reason": "", + "branch": "", + "local_sha": local_sha or None, + "updated": ts, + } + ) + inserted += 1 + landed += 1 + continue + already = rec.get("disposition") == "landed" and rec.get("local_sha") == (local_sha or None) + if already: + continue + rec["disposition"] = "landed" + rec["local_sha"] = local_sha or None + rec["updated"] = ts + landed += 1 + + write_ledger(jl, meta, records) + md = md_path(root) + md.write_text(render_md(meta, records)) + + # stage the tracked ledger; drain the journal + try: + subprocess.run(["git", "add", str(jl), str(md)], check=True) + except Exception as exc: # noqa: BLE001 + print(f"warning: `git add` failed ({exc}); files updated but not staged", file=sys.stderr) + journal.write_text("") + + print( + f"reconcile: {landed} landed ({inserted} new), staged {jl.name} + {md.name}, " + "journal drained. Commit the ledger separately." + ) + return 0 + + +def cmd_lookup(args: argparse.Namespace) -> int: + root = repo_root() + try: + _meta, records = load_ledger(jsonl_path(root)) + except ValueError as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + rec = find_record(records, args.sha) + if rec is None: + print("unknown") + return 0 + disp = rec.get("disposition", "") + reason = rec.get("reason", "") + print(f"{disp}\t{reason}") + return 0 + + +def cmd_check_reject(args: argparse.Namespace) -> int: + """Exit 3 iff SHA is disposition 'wont-merge'. Exit 0 otherwise. Exit 2 on ledger error.""" + root = repo_root() + try: + _meta, records = load_ledger(jsonl_path(root)) + except ValueError as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + rec = find_record(records, args.sha) + if rec is not None and rec.get("disposition") == "wont-merge": + print(rec.get("reason", "") or "(no reason recorded)") + return 3 + return 0 + + +def cmd_set(args: argparse.Namespace) -> int: + root = repo_root() + jl = jsonl_path(root) + try: + meta, records = load_ledger(jl) + except ValueError as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + rec = find_record(records, args.sha) + if rec is None: + rec = { + "sha": args.sha[:8], + "pr": None, + "subject": "", + "disposition": "untriaged", + "reason": "", + "branch": "", + "local_sha": None, + "updated": _now_iso(), + } + records.append(rec) + if args.disposition: + if args.disposition not in DISPOSITIONS: + print(f"error: disposition must be one of {DISPOSITIONS}", file=sys.stderr) + return 2 + rec["disposition"] = args.disposition + if args.pr is not None: + rec["pr"] = args.pr + if args.subject is not None: + rec["subject"] = args.subject + if args.reason is not None: + rec["reason"] = args.reason + if args.branch is not None: + rec["branch"] = args.branch + rec["updated"] = _now_iso() + write_ledger(jl, meta, records) + md_path(root).write_text(render_md(meta, records)) + print(f"updated {rec['sha']} -> {rec['disposition']}") + return 0 + + +def _diverged_commits(base: str, ref: str) -> list[tuple[str, int | None, str]]: + """Return ``[(short_sha, pr, subject)]`` for non-merge commits in ``base..ref``. + + Newest first, matching ``git log`` default order. ``pr`` is parsed from a + trailing ``(#1234)`` in the subject, else ``None``. + """ + out = subprocess.run( + ["git", "log", "--no-merges", "--format=%h%x09%s", f"{base}..{ref}"], + capture_output=True, + text=True, + check=True, + ).stdout + commits: list[tuple[str, int | None, str]] = [] + for line in out.splitlines(): + if "\t" not in line: + continue + sha, subject = line.split("\t", 1) + prs = re.findall(r"\(#(\d+)\)", subject) + pr = int(prs[-1]) if prs else None + commits.append((sha.strip(), pr, subject.strip())) + return commits + + +def cmd_sync(args: argparse.Namespace) -> int: + root = repo_root() + ref = args.ref + if not args.no_fetch: + remote = ref.split("/", 1)[0] if "/" in ref else "upstream" + try: + subprocess.run(["git", "fetch", remote], check=True) + except Exception as exc: # noqa: BLE001 + print(f"error: `git fetch {remote}` failed: {exc}", file=sys.stderr) + return 2 + try: + commits = _diverged_commits(args.base, ref) + except subprocess.CalledProcessError as exc: + print(f"error: `git log {args.base}..{ref}` failed: {exc}", file=sys.stderr) + return 2 + + jl = jsonl_path(root) + try: + meta, records = load_ledger(jl) + except ValueError as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + + added = 0 + # Append oldest-first so freshly-discovered rows read chronologically. + for sha, pr, subject in reversed(commits): + if find_record(records, sha) is not None: + continue + records.append( + { + "sha": sha[:8], + "pr": pr, + "subject": subject, + "disposition": "untriaged", + "reason": "", + "branch": "", + "local_sha": None, + "updated": _now_iso(), + } + ) + added += 1 + + try: + tip = subprocess.run( + ["git", "rev-parse", "--short", ref], + capture_output=True, + text=True, + check=True, + ).stdout.strip() + meta["last_synced"] = tip + meta["last_synced_date"] = datetime.now(UTC).strftime("%Y-%m-%d") + except Exception: # noqa: BLE001 + pass + + write_ledger(jl, meta, records) + md_path(root).write_text(render_md(meta, records)) + print( + f"sync: {added} new untriaged from {args.base}..{ref}; " + f"last synced -> {meta.get('last_synced', '?')} ({len(records)} rows total). " + "Triage the new rows, then commit triage.jsonl + triage.md." + ) + return 0 + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + sub = parser.add_subparsers(dest="command", required=True) + + p = sub.add_parser("migrate", help="parse hand-written triage.md -> triage.jsonl (one-time)") + p.add_argument("source", help="path to the current triage.md") + p.set_defaults(func=cmd_migrate) + + p = sub.add_parser("generate", help="render triage.jsonl -> triage.md") + p.add_argument("--check", action="store_true", help="fail if triage.md is stale (for CI)") + p.set_defaults(func=cmd_generate) + + p = sub.add_parser("reconcile", help="drain journal, mark landed, render, stage") + p.set_defaults(func=cmd_reconcile) + + p = sub.add_parser("sync", help="add new base..ref commits as untriaged") + p.add_argument("--base", default="dev", help="base branch already in the fork (default: dev)") + p.add_argument("--ref", default="upstream/main", help="upstream ref (default: upstream/main)") + p.add_argument("--no-fetch", action="store_true", help="skip `git fetch` of the remote") + p.set_defaults(func=cmd_sync) + + p = sub.add_parser("lookup", help="print a SHA's disposition") + p.add_argument("sha") + p.set_defaults(func=cmd_lookup) + + p = sub.add_parser("check-reject", help="exit 3 iff SHA is 'wont-merge'") + p.add_argument("sha") + p.set_defaults(func=cmd_check_reject) + + p = sub.add_parser("set", help="edit or add a ledger row") + p.add_argument("sha") + p.add_argument("--disposition", choices=DISPOSITIONS) + p.add_argument("--pr", type=int) + p.add_argument("--subject") + p.add_argument("--reason") + p.add_argument("--branch") + p.set_defaults(func=cmd_set) + + args = parser.parse_args(argv) + return args.func(args) + + +if __name__ == "__main__": + raise SystemExit(main()) From 589cd236c68f630987043224eae75450f6179260 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Fri, 3 Jul 2026 11:48:40 -0400 Subject: [PATCH 02/17] chore: cherry-pick clean upstream fixes + cherry-pick runbook (#117) * fix: make plan view mobile friendly (#1636) Co-authored-by: open-swe[bot] (cherry picked from commit 7ee3e0572406359632502b0d65450709a03645ea) * fix: return to thread after plan approval (#1637) Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] (cherry picked from commit f32e492ab46c120130e0c63eeca3eb353f10d83e) * feat: reviews block agenda, sticky headers, accurate diff scroll (#1653) Rework the AI-sorted blocks experience on the PR reviews page into a Google-Docs-style outline: the left sidebar is now a clean number+title agenda with scroll-spy highlighting of the active block; each block shows its title + description (sticky) above its diff; and diff rows are pinned to a uniform height so scroll-to lands precisely via the virtualizer's own geometry instead of an estimate-driven correction loop. Co-authored-by: open-swe[bot] (cherry picked from commit 0b76afdc955e33805c7623d1502a75a9c7c9c1b7) * fix: jump + ResizeObserver settle for review scroll-to (#1655) Replace smooth-scroll plus frame-count correction loops on the PR reviews page with an instant jump that re-asserts its target via a ResizeObserver (the real "layout settled" signal). Block/file navigation and finding/comment centering now land deterministically as off-screen cards mount, files expand, and annotation cards measure, instead of racing a smooth-scroll animation against height reconciliation. Holds bail on user wheel/touch input and after a short ceiling, and a new navigation cancels the previous hold. Co-authored-by: open-swe[bot] Co-authored-by: Johannes du Plessis (cherry picked from commit 7530653bba7774d66a54b8bef0d2bbc25f519942) * fix: purge expired thread_wakeup crons (#1656) * fix: purge expired thread_wakeup crons One-shot wakeup crons set an end_time that stops re-firing but the cron row is never deleted, so dead rows accumulate (86 in prod). Add a purge that deletes thread_wakeup crons past their end_time, called opportunistically before scheduling a new wakeup, plus a one-time backfill script. Conservative: matches only kind=thread_wakeup with a past end_time. * chore: retrigger Open SWE review --------- Co-authored-by: open-swe[bot] (cherry picked from commit 9e5a1924ef306269322c31342a1831e57831cfee) * fix: add top padding to sticky review block header (#1660) * fix: add top padding to sticky review block header The sticky per-block header on the reviews page had padding below but none above, so the block number badge sat glued against the top edge when pinned. Add matching top padding for breathing room. Co-authored-by: open-swe[bot] * chore: use py-2 shorthand for review block header padding Co-authored-by: open-swe[bot] --------- Co-authored-by: open-swe[bot] (cherry picked from commit 23bd4a63fc5ba0fe853babf79ed33feb866cc8b2) * fix: use global tokens for sidebar filter popover border (#1661) The filter popover renders via base-ui Menu.Portal into document.body, outside the .agents-ui container where the --ui-* CSS variables are scoped. As a result border-[var(--ui-border)] resolved to an undefined variable and border-color fell back to currentColor, producing a strong near-black border (separators/hover/labels were similarly off). Switch the portaled popup styling to the same global shadcn tokens the theme/settings popover (SidebarUserMenu) already uses (border-border, bg-border, bg-muted, text-muted-foreground). These are defined at :root so they resolve inside portals too, and match the settings popover. Co-authored-by: open-swe[bot] (cherry picked from commit 63eb9a08209f683016abf01cdcc548bc5905f158) * fix: preserve dashboard redirect after login (#1668) * fix: preserve dashboard redirect after login Co-authored-by: open-swe[bot] * test: cover plan login redirect in e2e Co-authored-by: open-swe[bot] --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] (cherry picked from commit bc7ce59169b5350da7286164afb83a7b037b528d) * Disable React StrictMode (#1654) Co-authored-by: open-swe[bot] (cherry picked from commit 6575c327a3ac2b107a6e79a04fa61168d779dbf0) * docs(upstream-sync): add cherry-pick runbook Repo-specific runbook for bringing upstream (langchain-ai/open-swe) commits into the fork: triage-sync discovery, the git cp workflow, the triage ledger, themed-branch layout, and conflict/regression handling. --------- Co-authored-by: Johannes du Plessis Co-authored-by: open-swe[bot] Co-authored-by: Ramon Nogueira Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: Caroline di Vittorio <43390382+carolinedivittorio@users.noreply.github.com> --- agent/tools/schedule_thread_wakeup.py | 70 +++++ docs/upstream-sync/cherry-pick-runbook.md | 95 ++++++ scripts/purge_wakeup_crons.py | 88 ++++++ tests/e2e/README.md | 22 +- tests/e2e/harness.py | 48 ++- tests/e2e/tests/plan_review.spec.ts | 92 ++++-- tests/test_dashboard_oauth_redirect.py | 30 ++ tests/test_schedule_thread_wakeup.py | 124 +++++++- ui/src/client.tsx | 6 +- ui/src/components/agents/PlanReview.tsx | 37 ++- ui/src/components/agents/ReviewMainBody.tsx | 295 ++++++++++++++---- ui/src/components/agents/ReviewSidebar.tsx | 156 +++------ .../components/agents/SidebarFilterMenu.tsx | 10 +- ui/src/components/agents/utils/diffUtils.ts | 14 + ui/src/lib/api.ts | 2 +- ui/src/lib/auth-redirect-core.ts | 120 +++++++ ui/src/lib/auth-redirect.test.ts | 76 +++++ ui/src/lib/auth-redirect.tsx | 13 + ui/src/routes/admin.tsx | 3 +- ui/src/routes/admin_.evals.tsx | 3 +- ui/src/routes/agents.tsx | 10 +- ui/src/routes/agents/$threadId_.plan.tsx | 20 +- .../agents/reviews/$owner.$repo.$number.tsx | 5 +- ui/src/routes/agents_.instructions.tsx | 5 +- ui/src/routes/agents_.snapshots.tsx | 3 +- ui/src/routes/cloud-agents.tsx | 5 +- ui/src/routes/login.tsx | 53 +++- ui/src/routes/my-settings.tsx | 5 +- ui/src/routes/review.tsx | 5 +- ui/src/routes/review_.repositories.$owner.tsx | 5 +- ui/src/routes/review_.styles.tsx | 5 +- ui/src/routes/usage.tsx | 5 +- 32 files changed, 1139 insertions(+), 291 deletions(-) create mode 100644 docs/upstream-sync/cherry-pick-runbook.md create mode 100644 scripts/purge_wakeup_crons.py create mode 100644 tests/test_dashboard_oauth_redirect.py create mode 100644 ui/src/lib/auth-redirect-core.ts create mode 100644 ui/src/lib/auth-redirect.test.ts create mode 100644 ui/src/lib/auth-redirect.tsx diff --git a/agent/tools/schedule_thread_wakeup.py b/agent/tools/schedule_thread_wakeup.py index 88b558c5..78d42e06 100644 --- a/agent/tools/schedule_thread_wakeup.py +++ b/agent/tools/schedule_thread_wakeup.py @@ -18,6 +18,9 @@ _MIN_DELAY_SECONDS = 60 _MAX_DELAY_SECONDS = 86_400 _END_TIME_PADDING_SECONDS = 90 +_WAKEUP_KIND = "thread_wakeup" +_PURGE_PAGE_SIZE = 100 + _DEFAULT_WAKEUP_PROMPT = ( "This is an automated re-trigger of this thread. The agent scheduled this " "wakeup to poll for updates. Check the current state of whatever you were " @@ -46,6 +49,71 @@ def _build_one_shot_cron(fire_time: datetime) -> str: ) +def _parse_iso(value: Any) -> datetime | None: + if not isinstance(value, str) or not value: + return None + try: + return datetime.fromisoformat(value.replace("Z", "+00:00")) + except ValueError: + return None + + +async def find_expired_wakeup_cron_ids(client: Any, *, now: datetime) -> list[str]: + """Return the ids of ``thread_wakeup`` crons whose ``end_time`` has passed. + + Conservative: matches solely on ``metadata.kind == "thread_wakeup"`` AND a + past ``end_time``, so analyzer/dashboard crons are never selected. Paginates + fully before returning so the result is stable to delete afterwards. + """ + expired_ids: list[str] = [] + offset = 0 + while True: + page = await client.crons.search( + metadata={"kind": _WAKEUP_KIND}, + limit=_PURGE_PAGE_SIZE, + offset=offset, + ) + if not page: + break + for cron in page: + if not isinstance(cron, dict): + continue + end_time = _parse_iso(cron.get("end_time")) + cron_id = cron.get("cron_id") + if end_time is not None and end_time < now and isinstance(cron_id, str) and cron_id: + expired_ids.append(cron_id) + if len(page) < _PURGE_PAGE_SIZE: + break + offset += len(page) + return expired_ids + + +async def purge_expired_wakeup_crons(client: Any, *, now: datetime) -> int: + """Delete ``thread_wakeup`` crons whose ``end_time`` has already passed. + + Each wakeup is a thread-bound cron with an ``end_time`` (~90s past its fire) + that stops it re-firing, but the cron row itself is never removed, so dead + rows accumulate. This deletes only those dead rows. Returns the count deleted. + """ + expired_ids = await find_expired_wakeup_cron_ids(client, now=now) + deleted = 0 + for cron_id in expired_ids: + await client.crons.delete(cron_id) + deleted += 1 + return deleted + + +async def _purge_expired_wakeups_best_effort() -> None: + """Opportunistically purge expired wakeup crons; never raises.""" + try: + client = get_client(url=langgraph_url()) + deleted = await purge_expired_wakeup_crons(client, now=datetime.now(UTC)) + if deleted: + logger.info("Purged %d expired thread_wakeup cron(s)", deleted) + except Exception: + logger.warning("Failed to purge expired thread_wakeup crons", exc_info=True) + + async def _create_wakeup_cron( *, thread_id: str, @@ -130,6 +198,8 @@ async def schedule_thread_wakeup(delay_minutes: int, prompt: str | None = None) if value is not None: wakeup_configurable[key] = value + await _purge_expired_wakeups_best_effort() + try: return await _create_wakeup_cron( thread_id=thread_id, diff --git a/docs/upstream-sync/cherry-pick-runbook.md b/docs/upstream-sync/cherry-pick-runbook.md new file mode 100644 index 00000000..bd5312bc --- /dev/null +++ b/docs/upstream-sync/cherry-pick-runbook.md @@ -0,0 +1,95 @@ +# Cherry-picking upstream into the fork + +This repository is a long-lived fork of `langchain-ai/open-swe` (git remote `upstream`). +Upstream changes are brought in one commit at a time with `git cherry-pick`, and every +diverged commit is tracked in a triage ledger so a decision is made once and not revisited. +`dev` is the integration branch; the broader strategy lives in the fork-maintenance section +of `CLAUDE.md`. + +## Setup (once per clone) + + make install-hooks + +Installs the triage hooks and the `git cp` alias by pointing `core.hooksPath` at `.githooks/`. +Because that shadows the machine-global hook directory (`~/.config/git/hooks`, which holds the +mandatory security `pre-push`), `.githooks/pre-push` is a shim that re-execs the global hook, +and the installer verifies that delegation before it changes anything. `git` never auto-adopts +a repository's `core.hooksPath`, so this step cannot be skipped. + +Note: `core.hooksPath` applies repo-wide, but `.githooks/` is a tracked directory. The hooks +(and the security shim) only run on branches that actually contain `.githooks/`. Keep it present +on `dev` and `main` so no branch loses the security `pre-push`. + +## Finding what to pick + + make triage-sync # git fetch upstream, then append new dev..upstream/main commits + # to the ledger as `untriaged` (PR # + subject parsed from each) + +`triage-sync` is the discovery step: it records every diverged commit as `untriaged` and bumps +"Last synced" to the new `upstream/main` tip. Triage those rows (decide `deferred` / `wont-merge` +and which branch), then pick the ones you want. The underlying views if you prefer raw git: + + git fetch upstream + git log --oneline --no-merges dev..upstream/main # everything diverged + git show # inspect before deciding + +Cross-check candidates against the ledger first — most diverged commits already carry a +decision (already-in-dev, regression, deferred, or landed) and should not be re-examined. + +## Bringing in commits: `git cp` + + git cp -x # pre-check the ledger, cherry-pick -x, auto-reconcile + git cp -x ... # several, applied in the given order + git cp --continue # after resolving a conflict; also reconciles + git cp --force # override a SHA the ledger marks "Won't merge" + +`git cp` reads `docs/upstream-sync/triage.jsonl` before touching the tree and refuses a +known-reject SHA (override with `--force`). On success it runs `make triage-reconcile`, which +moves each applied SHA to Landed in the ledger and stages `triage.jsonl` + `triage.md` for you +to commit. + +Apply commits in upstream chronological order (oldest first), not the order you happen to list +them — a later commit often depends on an earlier one, and out-of-order picks conflict +needlessly: + + git log --reverse --topo-order --format=%h dev..upstream/main + +## The triage ledger + +`docs/upstream-sync/triage.jsonl` is the source of truth: one JSON row per upstream SHA, keyed +on the SHA (stable, unlike the local SHAs cherry-pick rewrites). `docs/upstream-sync/triage.md` +is generated from it and must not be hand-edited. Dispositions are `landed`, `wont-merge`, +`deferred`, `untriaged`. + + scripts/triage.py set --disposition deferred --branch slack-tooling --reason "..." + make triage-render # regenerate triage.md from the jsonl + make triage-check # CI gate: fail if triage.md is stale + +A SHA marked `wont-merge` is hard-blocked by both `git cp` and the `prepare-commit-msg` hook. +Override for a one-off re-evaluation with `git cp --force`, `SH_CHERRYPICK_ALLOW_REJECT=1`, or +`git config sh.cherrypick.blockRejects false`. + +## Branch layout + +Never cherry-pick onto `dev` directly. Work on a themed branch off `dev` and open a PR into +`dev`; the ledger's `branch` column records where each deferred commit is meant to land +(for example `slack-tooling`, `gateway-routing`, `plan-approval`, `durable-dispatch`). Keep +each PR to one theme so conflict resolution stays within one subsystem. + +## Raw `git cherry-pick` + +The hooks fire on a plain `git cherry-pick -x ` too: `post-commit` journals each applied +pick and `prepare-commit-msg` blocks known-rejects. Run `make triage-reconcile` once at the end +to land the picks in the ledger, then commit `triage.jsonl` + `triage.md`. + +## Conflicts + + # resolve the files, then: + git add + git cherry-pick --continue # or: git cp --continue + git cherry-pick --abort # bail out of the whole pick + git cherry-pick --skip # drop just this commit and continue the batch + +A commit that conflicts because `dev` already carries a newer version of the same code is a +regression, not a merge — skip it and record the decision as `wont-merge` in the ledger rather +than forcing it in. diff --git a/scripts/purge_wakeup_crons.py b/scripts/purge_wakeup_crons.py new file mode 100644 index 00000000..ad556593 --- /dev/null +++ b/scripts/purge_wakeup_crons.py @@ -0,0 +1,88 @@ +"""One-time backfill: delete expired ``thread_wakeup`` crons from a deployment. + +One-shot wakeup crons set an ``end_time`` that stops them re-firing, but the +cron row is never removed, so dead rows accumulate. The ``schedule_thread_wakeup`` +tool now purges these opportunistically; this script clears the backlog. + +Usage: + uv run python scripts/purge_wakeup_crons.py --dry-run + uv run python scripts/purge_wakeup_crons.py + +Resolves the deployment URL from ``--url`` or ``LANGGRAPH_URL`` / ``LANGGRAPH_URL_PROD``, +and the API key from ``LANGGRAPH_API_KEY`` / ``LANGSMITH_API_KEY`` / ``LANGSMITH_API_KEY_PROD``. +""" + +from __future__ import annotations + +import argparse +import asyncio +import logging +import os +from datetime import UTC, datetime + +from langgraph_sdk import get_client + +from agent.tools.schedule_thread_wakeup import ( + find_expired_wakeup_cron_ids, + purge_expired_wakeup_crons, +) + +logger = logging.getLogger(__name__) + + +def _load_dotenv_if_available() -> None: + try: + from dotenv import load_dotenv + except ImportError: + return + load_dotenv() + + +def _resolve_url(arg_url: str | None) -> str: + url = arg_url or os.environ.get("LANGGRAPH_URL") or os.environ.get("LANGGRAPH_URL_PROD") + if not url: + raise RuntimeError("Set --url or LANGGRAPH_URL / LANGGRAPH_URL_PROD") + return url + + +def _resolve_api_key() -> str | None: + return ( + os.environ.get("LANGGRAPH_API_KEY") + or os.environ.get("LANGSMITH_API_KEY") + or os.environ.get("LANGSMITH_API_KEY_PROD") + ) + + +async def _run(url: str, api_key: str | None, dry_run: bool) -> None: + client = get_client(url=url, api_key=api_key) + now = datetime.now(UTC) + if dry_run: + expired = await find_expired_wakeup_cron_ids(client, now=now) + logger.info("[dry-run] %d expired thread_wakeup cron(s) would be deleted", len(expired)) + for cron_id in expired: + logger.info(" %s", cron_id) + return + deleted = await purge_expired_wakeup_crons(client, now=now) + logger.info("Deleted %d expired thread_wakeup cron(s)", deleted) + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser(description="Purge expired thread_wakeup crons.") + parser.add_argument("--url", default=None, help="Deployment URL (defaults to env).") + parser.add_argument( + "--dry-run", + action="store_true", + help="List the crons that would be deleted without deleting them.", + ) + return parser.parse_args() + + +def main() -> None: + _load_dotenv_if_available() + logging.basicConfig(level=logging.INFO, format="%(message)s") + args = parse_args() + asyncio.run(_run(_resolve_url(args.url), _resolve_api_key(), args.dry_run)) + + +if __name__ == "__main__": + main() diff --git a/tests/e2e/README.md b/tests/e2e/README.md index 990ef3cd..1469b129 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -12,17 +12,17 @@ This drives the **whole happy path** through two mock UIs: Only the **LLM** and the **external SaaS HTTP boundaries** are faked. All agent code runs for real. -| Piece | Real or fake | -|---|---| -| Slack webhook → `process_slack_mention` → run dispatch | **real** (`agent.webapp`) | -| `get_agent`, deepagents loop, tools, middleware, prompt | **real** | -| `open_pull_request`, `slack_thread_reply` tools | **real** | -| Sandbox | **real** `local` provider, rooted in a throwaway temp dir | -| Git remote ("GitHub") | **real git**, a local bare repo the agent clones/pushes | -| The LLM | **fake** — a scripted model (`fake_llm.py`) emitting a fixed tool sequence | -| `api.github.com` REST (PR create) | **fake** (`/fake-gh/...`), state rendered at `/mock/github` | -| `slack.com/api` (post message, etc.) | **fake** (`/fake-slack/...`), thread rendered at `/mock/slack` | -| GitHub App token mint, `api.github.com/user` identity | stubbed (offline) | +| Piece | Real or fake | +| ---------------------------------------------------------------- | -------------------------------------------------------------------------- | +| Slack webhook → `process_slack_mention` → run dispatch | **real** (`agent.webapp`) | +| `get_agent`, deepagents loop, tools, middleware, prompt | **real** | +| `open_pull_request`, `slack_thread_reply` tools | **real** | +| Sandbox | **real** `local` provider, rooted in a throwaway temp dir | +| Git remote ("GitHub") | **real git**, a local bare repo the agent clones/pushes | +| The LLM | **fake** — a scripted model (`fake_llm.py`) emitting a fixed tool sequence | +| `api.github.com` REST (PR create) + dashboard GitHub OAuth login | **fake** (`/fake-gh/...`), state rendered at `/mock/github` | +| `slack.com/api` (post message, etc.) | **fake** (`/fake-slack/...`), thread rendered at `/mock/slack` | +| GitHub App token mint, `api.github.com/user` identity | stubbed (offline) | The fake GitHub/Slack stores are the single source of truth the mock UIs render, so what Playwright asserts on is exactly what the real agent produced. diff --git a/tests/e2e/harness.py b/tests/e2e/harness.py index 1147300b..f774d546 100644 --- a/tests/e2e/harness.py +++ b/tests/e2e/harness.py @@ -18,8 +18,10 @@ import json import os import sys import time +from html import escape from pathlib import Path from typing import Any +from urllib.parse import quote sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) @@ -185,29 +187,43 @@ async def control_login_get(login: str = "", email: str = "", next_url: str = "" @app.get("/dashboard/api/auth/login") -async def mock_github_login(redirect_to: str = "", login: str = "") -> Response: - """Mock stand-in for GitHub OAuth: the dashboard's "Continue with GitHub" - button lands here. With no ``login``, render a picker of the fake GitHub - test users; once one is chosen, mint the real session cookie and redirect - back into the dashboard (``redirect_to``).""" +async def mock_github_login(redirect_to: str = "") -> Response: + """E2E stand-in for the dashboard OAuth start route. + + The real route would redirect to github.com. Keep the dashboard-facing URL + intact, then hand off to the fake GitHub simulator so Playwright exercises a + browser login flow instead of test code pre-minting a session cookie. + """ + ui = os.environ.get("DASHBOARD_BASE_URL", "").rstrip("/") + dest = redirect_to or (f"{ui}/agents" if ui else "/agents") + return RedirectResponse(f"/fake-gh/login/oauth/authorize?redirect_to={quote(dest)}", 302) + + +@app.get("/fake-gh/login/oauth/authorize") +async def fake_github_authorize(redirect_to: str = "", login: str = "") -> Response: + """Fake GitHub OAuth consent/login page for dashboard e2e tests.""" ui = os.environ.get("DASHBOARD_BASE_URL", "").rstrip("/") dest = redirect_to or (f"{ui}/agents" if ui else "/agents") if not login: options = "".join( - f'' for u in TEST_USERS + f'" + for u in TEST_USERS ) return HTMLResponse( - f"""Continue with GitHub (mock) + f"""GitHub · Authorize open-swe -

Continue with GitHub (mock)

-

Pick a fake GitHub account to sign in as.

-
- - - -
-

Tip: use a separate browser or profile per - user so their sessions don't overwrite each other.

+
+

Authorize open-swe

+

Pick a fake GitHub account to continue.

+
+ + + +
+
""" ) match = next((u for u in TEST_USERS if u["login"] == login), None) diff --git a/tests/e2e/tests/plan_review.spec.ts b/tests/e2e/tests/plan_review.spec.ts index e7637a7b..e7f92199 100644 --- a/tests/e2e/tests/plan_review.spec.ts +++ b/tests/e2e/tests/plan_review.spec.ts @@ -38,9 +38,14 @@ test.describe("Plan review (HTTP comments)", () => { // 1. A user asks the bot to PLAN something in Slack. await request.post("/control/reset"); const send = await request.post("/mock/slack/send", { - data: { text: "<@U0BOT> plan how to add a greet() helper", mention_bot: true }, + data: { + text: "<@U0BOT> plan how to add a greet() helper", + mention_bot: true, + }, }); - const { thread_id: threadId } = (await send.json()) as { thread_id: string }; + const { thread_id: threadId } = (await send.json()) as { + thread_id: string; + }; expect(threadId).toBeTruthy(); const planPath = `/agents/${threadId}/plan`; @@ -57,7 +62,9 @@ test.describe("Plan review (HTTP comments)", () => { }; return (state.values?.messages ?? []) .map((m) => - typeof m.content === "string" ? m.content : JSON.stringify(m.content), + typeof m.content === "string" + ? m.content + : JSON.stringify(m.content), ) .some((c) => c.includes("Plan mode is active")); }, @@ -67,13 +74,45 @@ test.describe("Plan review (HTTP comments)", () => { // 2. The agent shares the plan-review link, then announces the plan is ready. await expect - .poll(async () => (await botMessages(request)).join("\n"), { timeout: 60_000 }) + .poll(async () => (await botMessages(request)).join("\n"), { + timeout: 60_000, + }) .toMatch(/\/agents\/[^/]+\/plan\b/); await expect - .poll(async () => (await botMessages(request)).join("\n"), { timeout: 60_000 }) + .poll(async () => (await botMessages(request)).join("\n"), { + timeout: 60_000, + }) .toMatch(/ready for review/i); - // 3. The OWNER opens the conversation, follows the "Review plan" banner, and + // 3. A logged-out user follows the plan deep link, signs in through the fake + // GitHub OAuth simulator, and lands back on the same plan page. + const loggedOutCtx = await browser.newContext(); + const loggedOut = await loggedOutCtx.newPage(); + await loggedOut.goto(planPath); + await expect(loggedOut).toHaveURL( + new RegExp(`/login\\?redirect=.*${threadId}.*plan`), + ); + await expect(loggedOut.getByText("Sign in to open-swe")).toBeVisible({ + timeout: 30_000, + }); + await loggedOut.getByRole("link", { name: "Continue with GitHub" }).click(); + await expect(loggedOut).toHaveURL(/\/fake-gh\/login\/oauth\/authorize/); + await expect(loggedOut.getByTestId("fake-github-login")).toBeVisible(); + await loggedOut.getByLabel("GitHub user").selectOption(OWNER.login); + await loggedOut.getByRole("button", { name: "Authorize open-swe" }).click(); + await expect(loggedOut).toHaveURL(new RegExp(`/agents/${threadId}/plan$`)); + await expect(loggedOut.getByTestId("plan-review")).toBeVisible({ + timeout: 30_000, + }); + await expect(loggedOut.getByTestId("plan-document")).toContainText( + "greet", + { + timeout: 30_000, + }, + ); + await loggedOutCtx.close(); + + // 4. The OWNER opens the conversation, follows the "Review plan" banner, and // sees the rendered plan. const ownerCtx = await browser.newContext({ permissions: ["clipboard-read", "clipboard-write"], @@ -85,7 +124,9 @@ test.describe("Plan review (HTTP comments)", () => { await expect(reviewLink).toBeVisible({ timeout: 30_000 }); await reviewLink.click(); await expect(owner).toHaveURL(new RegExp(`/agents/${threadId}/plan$`)); - await expect(owner.getByTestId("plan-review")).toBeVisible({ timeout: 30_000 }); + await expect(owner.getByTestId("plan-review")).toBeVisible({ + timeout: 30_000, + }); await expect(owner.getByText("Back to conversation")).toBeVisible(); await expect(owner.getByTestId("plan-document")).toContainText("greet", { timeout: 30_000, @@ -98,7 +139,9 @@ test.describe("Plan review (HTTP comments)", () => { // Copy the whole plan as markdown. await owner.getByTestId("copy-plan").click(); await expect(owner.getByTestId("copy-plan")).toContainText("Copied!"); - const clipboard = await owner.evaluate(() => navigator.clipboard.readText()); + const clipboard = await owner.evaluate(() => + navigator.clipboard.readText(), + ); expect(clipboard).toContain("## Plan: Add greet() helper"); expect(clipboard).toContain("### Verification"); @@ -107,18 +150,24 @@ test.describe("Plan review (HTTP comments)", () => { await expect(owner.getByTestId("plan-comment")).toHaveCount(1); await expect(owner.getByTestId("reject-plan")).toBeEnabled(); - // 4. A COLLABORATOR opens the same plan: sees it AND the owner's comment + // 5. A COLLABORATOR opens the same plan: sees it AND the owner's comment // (fetched over HTTP), but has NO approve button. const collabCtx = await browser.newContext(); await collabCtx.request.post("/control/login", { data: COLLABORATOR }); const collab = await collabCtx.newPage(); await collab.goto(planPath); - await expect(collab.getByTestId("plan-review")).toBeVisible({ timeout: 30_000 }); + await expect(collab.getByTestId("plan-review")).toBeVisible({ + timeout: 30_000, + }); await expect(collab.getByTestId("plan-document")).toContainText("greet", { timeout: 30_000, }); - await expect(collab.getByTestId("plan-comment")).toHaveCount(1, { timeout: 30_000 }); - await expect(collab.getByTestId("plan-comment")).toContainText("looks solid"); + await expect(collab.getByTestId("plan-comment")).toHaveCount(1, { + timeout: 30_000, + }); + await expect(collab.getByTestId("plan-comment")).toContainText( + "looks solid", + ); await expect(collab.getByTestId("approve-plan")).toHaveCount(0); await expect(collab.getByTestId("reject-plan")).toBeVisible(); @@ -126,20 +175,27 @@ test.describe("Plan review (HTTP comments)", () => { await addComment(collab, "Reviewer: please also add a docstring."); await expect(collab.getByTestId("plan-comment")).toHaveCount(2); - // 5. The owner sees the collaborator's comment (polled), then approves. - await expect(owner.getByTestId("plan-comment")).toHaveCount(2, { timeout: 30_000 }); + // 6. The owner sees the collaborator's comment (polled), then approves and + // returns to the main conversation while implementation starts. + await expect(owner.getByTestId("plan-comment")).toHaveCount(2, { + timeout: 30_000, + }); await owner.getByTestId("approve-plan").click(); - await expect(owner.getByTestId("plan-decision")).toContainText(/implementing/i); + await expect(owner).toHaveURL(new RegExp(`/agents/${threadId}$`)); - // 6. The agent implements, opens a PR, and links it back in the Slack thread, + // 7. The agent implements, opens a PR, and links it back in the Slack thread, // echoing the reviewers' feedback — which proves the comments were stored // and harvested server-side on approve. await expect - .poll(async () => (await botMessages(request)).join("\n"), { timeout: 90_000 }) + .poll(async () => (await botMessages(request)).join("\n"), { + timeout: 90_000, + }) .toMatch(/\/pull\//); expect((await botMessages(request)).join("\n")).toMatch(/docstring/); - const prs = (await (await request.get("/mock/github/data")).json()) as Array; + const prs = (await ( + await request.get("/mock/github/data") + ).json()) as Array; expect(prs.length).toBeGreaterThan(0); await ownerCtx.close(); diff --git a/tests/test_dashboard_oauth_redirect.py b/tests/test_dashboard_oauth_redirect.py new file mode 100644 index 00000000..c19287ec --- /dev/null +++ b/tests/test_dashboard_oauth_redirect.py @@ -0,0 +1,30 @@ +from __future__ import annotations + +from agent.dashboard.oauth import sanitize_redirect_to + + +def test_sanitize_redirect_to_preserves_allowed_dashboard_target(monkeypatch) -> None: + monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example") + monkeypatch.setenv("DASHBOARD_ALLOWED_ORIGINS", "https://preview.example") + + target = "https://dashboard.example/agents/thread-1/plan?from=slack#review" + + assert sanitize_redirect_to(target) == target + + +def test_sanitize_redirect_to_preserves_allowed_preview_target(monkeypatch) -> None: + monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example") + monkeypatch.setenv("DASHBOARD_ALLOWED_ORIGINS", "https://preview.example") + + target = "https://preview.example/agents/thread-1/plan?from=slack#review" + + assert sanitize_redirect_to(target) == target + + +def test_sanitize_redirect_to_rejects_external_target(monkeypatch) -> None: + monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example") + monkeypatch.setenv("DASHBOARD_ALLOWED_ORIGINS", "https://preview.example") + + assert sanitize_redirect_to("https://evil.example/agents/thread-1/plan") == ( + "https://dashboard.example" + ) diff --git a/tests/test_schedule_thread_wakeup.py b/tests/test_schedule_thread_wakeup.py index 55b0c05e..50c4b1ad 100644 --- a/tests/test_schedule_thread_wakeup.py +++ b/tests/test_schedule_thread_wakeup.py @@ -1,13 +1,67 @@ from __future__ import annotations import importlib -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from typing import Any import pytest wakeup_tool = importlib.import_module("agent.tools.schedule_thread_wakeup") +# Captured before the autouse stub replaces it, for the one test that needs the real wrapper. +_real_purge_best_effort = wakeup_tool._purge_expired_wakeups_best_effort + + +@pytest.fixture(autouse=True) +def _stub_purge(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the opportunistic purge from touching the network in every test.""" + + async def _noop() -> None: + return None + + monkeypatch.setattr(wakeup_tool, "_purge_expired_wakeups_best_effort", _noop) + + +class _FakeCrons: + def __init__(self, crons: list[dict[str, Any]]) -> None: + self._crons = list(crons) + self.deleted: list[str] = [] + self.search_calls: list[dict[str, Any]] = [] + + async def search( + self, + *, + metadata: dict[str, Any] | None = None, + limit: int = 10, + offset: int = 0, + **_: Any, + ) -> list[dict[str, Any]]: + self.search_calls.append({"metadata": metadata, "limit": limit, "offset": offset}) + items = [ + c + for c in self._crons + if not metadata + or all((c.get("metadata") or {}).get(k) == v for k, v in metadata.items()) + ] + return items[offset : offset + limit] + + async def delete(self, cron_id: str) -> None: + self.deleted.append(cron_id) + self._crons = [c for c in self._crons if c.get("cron_id") != cron_id] + + +class _FakeClient: + def __init__(self, crons: list[dict[str, Any]]) -> None: + self.crons = _FakeCrons(crons) + + +def _wakeup_cron(cron_id: str, end_time: datetime | None) -> dict[str, Any]: + return { + "cron_id": cron_id, + "end_time": end_time.isoformat() if end_time else None, + "metadata": {"kind": "thread_wakeup"}, + } + def _config(**overrides: Any) -> dict[str, Any]: base: dict[str, Any] = { @@ -241,3 +295,71 @@ def test_build_one_shot_cron_handles_month_boundary() -> None: assert parts[1] == "23" assert parts[2] == "31" assert parts[3] == "12" + + +async def test_purge_deletes_only_expired_wakeups() -> None: + now = datetime(2026, 6, 30, 22, 0, tzinfo=UTC) + client = _FakeClient( + [ + _wakeup_cron("expired-1", now - timedelta(hours=1)), + _wakeup_cron("expired-2", now - timedelta(days=1)), + _wakeup_cron("future-1", now + timedelta(hours=1)), + _wakeup_cron("no-end", None), + ] + ) + + deleted = await wakeup_tool.purge_expired_wakeup_crons(client, now=now) + + assert deleted == 2 + assert client.crons.deleted == ["expired-1", "expired-2"] + # Search is scoped to the thread_wakeup kind so other crons are never seen. + assert client.crons.search_calls[0]["metadata"] == {"kind": "thread_wakeup"} + + +async def test_purge_paginates(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(wakeup_tool, "_PURGE_PAGE_SIZE", 2) + now = datetime(2026, 6, 30, 22, 0, tzinfo=UTC) + client = _FakeClient([_wakeup_cron(f"expired-{i}", now - timedelta(hours=1)) for i in range(3)]) + + deleted = await wakeup_tool.purge_expired_wakeup_crons(client, now=now) + + assert deleted == 3 + assert sorted(client.crons.deleted) == ["expired-0", "expired-1", "expired-2"] + # Two pages fetched (offset 0 and 2), then a short final page ends the loop. + assert [c["offset"] for c in client.crons.search_calls] == [0, 2] + + +async def test_best_effort_purge_swallows_errors(monkeypatch: pytest.MonkeyPatch) -> None: + async def boom(*_: Any, **__: Any) -> int: + raise RuntimeError("search failed") + + monkeypatch.setattr(wakeup_tool, "purge_expired_wakeup_crons", boom) + monkeypatch.setattr(wakeup_tool, "get_client", lambda url: object()) + + # The real wrapper must never propagate — a purge failure can't block wakeups. + await _real_purge_best_effort() + + +async def test_schedule_purges_before_creating(monkeypatch: pytest.MonkeyPatch) -> None: + calls: list[str] = [] + + async def spy_purge() -> None: + calls.append("purge") + + async def fake_create_wakeup_cron(**kwargs: Any) -> dict[str, Any]: + calls.append("create") + return { + "success": True, + "cron_id": "cron-1", + "scheduled_for": "", + "thread_id": kwargs["thread_id"], + } + + monkeypatch.setattr(wakeup_tool, "get_config", _config) + monkeypatch.setattr(wakeup_tool, "_purge_expired_wakeups_best_effort", spy_purge) + monkeypatch.setattr(wakeup_tool, "_create_wakeup_cron", fake_create_wakeup_cron) + + result = await wakeup_tool.schedule_thread_wakeup(5) + + assert result["success"] is True + assert calls == ["purge", "create"] diff --git a/ui/src/client.tsx b/ui/src/client.tsx index c6783110..5fe66e09 100644 --- a/ui/src/client.tsx +++ b/ui/src/client.tsx @@ -1,5 +1,5 @@ import { StartClient } from "@tanstack/react-start/client" -import { StrictMode, useEffect, useState } from "react" +import { useEffect, useState } from "react" import { hydrateRoot } from "react-dom/client" import { registerSW } from "virtual:pwa-register" @@ -35,8 +35,8 @@ function PwaUpdateProvider() { hydrateRoot( document, - + <> - + ) diff --git a/ui/src/components/agents/PlanReview.tsx b/ui/src/components/agents/PlanReview.tsx index 1220c4a3..c8ec2815 100644 --- a/ui/src/components/agents/PlanReview.tsx +++ b/ui/src/components/agents/PlanReview.tsx @@ -1,4 +1,5 @@ import { useCallback, useEffect, useState } from "react" +import { useNavigate } from "@tanstack/react-router" import type { PlanComment, PlanData } from "@/lib/plan" import { @@ -47,6 +48,7 @@ async function copyToClipboard(text: string): Promise { } export function PlanReview({ plan }: { plan: PlanData }) { + const navigate = useNavigate() const resolvedTheme = useResolvedTheme() const [comments, setComments] = useState>([]) const [draft, setDraft] = useState("") @@ -108,20 +110,23 @@ export function PlanReview({ plan }: { plan: PlanData }) { setBusy(kind) setError(null) try { - if (kind === "approve") await approvePlan(plan.threadId) - else await rejectPlan(plan.threadId) - setDecision( - kind === "approve" - ? "Plan approved — the agent is implementing it." - : "Changes requested — the agent is revising the plan." - ) + if (kind === "approve") { + await approvePlan(plan.threadId) + await navigate({ + to: "/agents/$threadId", + params: { threadId: plan.threadId }, + }) + return + } + await rejectPlan(plan.threadId) + setDecision("Changes requested — the agent is revising the plan.") } catch (e) { setError((e as Error).message) } finally { setBusy(null) } }, - [plan.threadId] + [navigate, plan.threadId] ) const copyPlan = useCallback(async () => { @@ -139,8 +144,8 @@ export function PlanReview({ plan }: { plan: PlanData }) { data-testid="plan-review" className="flex min-h-0 flex-1 flex-col bg-[var(--ui-bg)] text-[var(--ui-text)]" > -
-
+
+

Implementation plan

@@ -150,11 +155,11 @@ export function PlanReview({ plan }: { plan: PlanData }) { {plan.status}

-
+
{decision && ( {decision} @@ -194,9 +199,9 @@ export function PlanReview({ plan }: { plan: PlanData }) {
-
+
@@ -209,14 +214,14 @@ export function PlanReview({ plan }: { plan: PlanData }) { )}
-