diff --git a/.github/workflows/pr-title-sync.yml b/.github/workflows/pr-title-sync.yml index 6f5b3d3e5..4f94d4db9 100644 --- a/.github/workflows/pr-title-sync.yml +++ b/.github/workflows/pr-title-sync.yml @@ -1,7 +1,25 @@ name: PR Title Sync +# WHY pull_request_target (not pull_request): the default GITHUB_TOKEN is +# READ-ONLY on fork PRs under `pull_request`, so the title-sync backstop could +# never `gh pr edit` a fork/agent PR. `pull_request_target` runs in the base-repo +# context with a write token, which fixes fork coverage. +# +# WHY this is SAFE (pull_request_target is the most dangerous trigger): +# - We check out the BASE repo (no `ref:`), so the only code we execute is +# trusted base-repo infra (bin/gstack-pr-title-rewrite.sh). We NEVER check +# out or run PR-head/fork code. +# - Every attacker-controlled PR field (title, head repo, head sha) arrives via +# `env:` and is referenced as a shell-quoted "$VAR". We NEVER inline a +# `${{ github.event.pull_request.* }}` expression inside the run: script +# (that would execute a crafted title as shell). +# - The PR-head VERSION is read as DATA via the API (raw media type), from the +# head repo at the head sha — never by checking out the head. +# test/pr-title-sync-workflow-safety.test.ts is the static tripwire for all of +# the above and fails CI if any of it regresses. + on: - pull_request: + pull_request_target: types: [opened, synchronize, edited] paths: - 'VERSION' @@ -19,25 +37,62 @@ jobs: pull-requests: write if: github.actor != 'github-actions[bot]' steps: - - name: Checkout PR head + # Base repo only — trusted infra (the rewrite helper). No PR-head checkout. + - name: Checkout base repo (trusted) uses: actions/checkout@v4 with: fetch-depth: 1 - ref: ${{ github.event.pull_request.head.sha }} - name: Rewrite PR title to match VERSION env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUM: ${{ github.event.pull_request.number }} + # Attacker-controlled on fork PRs — env-only, never inlined into run:. OLD_TITLE: ${{ github.event.pull_request.title }} + BASE_REPO: ${{ github.repository }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | set -euo pipefail chmod +x ./bin/gstack-pr-title-rewrite.sh - VERSION=$(cat VERSION | tr -d '[:space:]') - NEW_TITLE=$(./bin/gstack-pr-title-rewrite.sh "$VERSION" "$OLD_TITLE") - if [ "$NEW_TITLE" = "$OLD_TITLE" ]; then - echo "Title already correct; no change." + + if [ "$HEAD_REPO" = "$BASE_REPO" ]; then IS_FORK=0; else IS_FORK=1; fi + + # Read the PR-head VERSION as data (raw bytes), from the head repo at + # the head sha. Guard the assignment itself: under `set -e` a bare + # `VERSION=$(...)` would abort the step before any later [ -z ] check. + if ! VERSION=$(gh api -H "Accept: application/vnd.github.raw" \ + "repos/$HEAD_REPO/contents/VERSION?ref=$HEAD_SHA" 2>/dev/null | tr -d '[:space:]'); then + VERSION="" + fi + + if [ -z "$VERSION" ]; then + # Same-repo read failure should never happen — fail loudly so we + # notice. A fork miss (public-contents quirk, private fork) is a + # convenience gap, not a gate — warn and skip so the check stays green. + if [ "$IS_FORK" = "0" ]; then + echo "::error::Could not read VERSION from same-repo PR head ($HEAD_SHA)." + exit 1 + fi + echo "::warning::Could not read VERSION from fork $HEAD_REPO ($HEAD_SHA); skipping title sync." + exit 0 + fi + + # The helper rejects a malformed VERSION (exit 2). Same policy: loud for + # same-repo, soft for forks. Never echo the raw (attacker-controlled) + # title — Actions still parses ::workflow-command:: from stdout. + if ! NEW_TITLE=$(./bin/gstack-pr-title-rewrite.sh "$VERSION" "$OLD_TITLE"); then + if [ "$IS_FORK" = "0" ]; then + echo "::error::Could not compute title for VERSION '$VERSION' on PR #$PR_NUM." + exit 1 + fi + echo "::warning::Could not compute title for fork PR #$PR_NUM; skipping." + exit 0 + fi + + if [ "$NEW_TITLE" = "$OLD_TITLE" ]; then + echo "PR #$PR_NUM title already correct; no change." exit 0 fi - echo "Rewriting: $OLD_TITLE -> $NEW_TITLE" gh pr edit "$PR_NUM" --title "$NEW_TITLE" + echo "PR #$PR_NUM title synced to VERSION." diff --git a/CHANGELOG.md b/CHANGELOG.md index bdf89641a..5109cc167 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,131 @@ # Changelog +## [1.57.3.0] - 2026-06-07 + +## **Every PR `/ship` opens gets the version stamped into its title, fork and agent PRs included.** +## **The rule rides in the always-loaded part of the skill now, and a guard keeps it there.** + +`/ship` stamps `vX.Y.Z.W` onto the title of every PR or MR it creates or updates, so +the version is the first thing you read in the PR list. That rule now lives in the +always-loaded core of the ship skill instead of an on-demand section, so the agent +applies it whether or not it opened the section that spells out the full procedure. +A CI workflow backs this up: it rewrites a title to match VERSION on every PR that +bumps the version, and it now reaches fork and agent PRs too, which a read-only token +could never touch before. Two free tests lock the behavior in so it cannot drift on +the next refactor. + +### The numbers that matter + +Reproduce with `bun test test/carve-section-ordering.test.ts test/pr-title-sync-workflow-safety.test.ts` +and `bun run eval:select`. + +| Property | Before | After | +|---|---|---| +| Where the title rule loads | on-demand section only (since v1.54.0.0) | always-loaded skeleton + on-demand detail | +| Fork / agent PR title sync | none (read-only token under `pull_request`) | covered via hardened `pull_request_target` | +| Test proving the rule stays put | none | carve-guard registry asserts it on every PR | +| CI injection guard for the title workflow | none | static tripwire fails CI on unsafe patterns | + +The title workflow now runs with a write token in the base-repo context but never +checks out or executes PR-head code, and every attacker-controlled field reaches the +script through `env:`, never inlined. A static test fails CI if either rule regresses. + +### What this means for you + +Ship a branch and the PR shows up titled `v1.57.3.0 fix: ...` without you touching it, +even when the PR came from a fork. The agent no longer needs to read the right section +at the right moment for the version to land in the title, and the next person who slims +the ship skill cannot quietly strand the rule again, because a free test on every PR +checks that it is still there. + +### Itemized changes + +#### Added +- Carve-guard coverage for the ship PR-title invariant: the registry now asserts the + `v$NEW_VERSION` rule and the title helper stay in the always-loaded skeleton, while + the full create and update procedure stays in the on-demand section. +- Static CI-safety test for the title-sync workflow that fails the build if it checks + out PR-head code or inlines an attacker-controlled PR field into a shell step. + +#### Changed +- The PR/MR title-version rule is always-loaded in `/ship` again, so the version + prefix lands on every PR the workflow creates or updates. +- The PR title-sync CI workflow now covers fork and agent PRs through a hardened + `pull_request_target` trigger (base-repo checkout only, PR fields passed via `env:`, + VERSION read as data from the PR head). + +#### Fixed +- A path token in the ship PR-body section that rendered literally instead of resolving + now uses the correct helper path, so the Linked Spec auto-detect step runs as written. + +## [1.57.2.0] - 2026-06-08 + +## **When the question picker breaks mid-skill, gstack asks in plain text instead of stalling.** +## **Every skill detects a dead AskUserQuestion and falls back to a full decision brief you answer by typing a letter.** + +AskUserQuestion is how every gstack skill asks you to decide. When the host's question +tool fails at runtime, which Conductor's MCP integration currently does intermittently, +skills used to stall or hard-block. Now each skill detects the failure, works out +whether a human is actually present, and if so re-renders the exact same decision as a +text message: a plain-English explanation of the issue, a completeness score on each +choice, and a recommendation with its reason, one paragraph per choice. You answer by +typing a single letter. Headless eval runs still block cleanly (no human to answer); +orchestrator sessions keep auto-choosing. This whole release was built and reviewed +through that fallback, because the Conductor tool was down the entire session. + +### The numbers that matter + +No production benchmark for a reliability path like this. These are the behavior and +coverage facts, verifiable with `bun test test/gstack-session-kind.test.ts +test/resolver-ask-user-format.test.ts test/auq-error-fallback-hook.test.ts`. + +| When AskUserQuestion fails | Before | After | +|---|---|---| +| Interactive session (human present) | stall / hard BLOCK | full prose decision brief, answer by letter | +| Headless eval / CI | BLOCK | BLOCK (unchanged, correct) | +| Orchestrator (OpenClaw) session | undefined | auto-choose recommended (contract kept) | +| Session kinds detected | 0 | 3 (interactive / headless / spawned) | +| New tests guarding the path | 0 | 34 | + +The text brief is not a degraded stub. It carries the same three things the picker +shows: a clear explanation of what is being decided, a `Completeness: X/10` on every +choice, and a recommendation with the reason it wins. + +### What this means for you + +If your host's question tool flakes out, a skill no longer dies on you. You get the +same decision to make, in text, and you reply with a letter. Nothing changes when the +tool works normally. If you run gstack headless, those sessions still block on a needed +question exactly as before, so eval determinism is intact. + +### Itemized changes + +#### Added +- `gstack-session-kind` classifies each session as interactive, headless, or spawned, + echoed as `SESSION_KIND` at skill start so any skill can branch on it. +- Plain-text fallback for AskUserQuestion: on a tool failure in an interactive session, + the skill renders the full decision brief (issue ELI10 + per-choice completeness + + recommendation) as markdown you answer by typing a letter, then stops and waits. +- A defensive hook that, when an AskUserQuestion call errors, reminds the agent to run + the fallback for the current session kind. + +#### Changed +- AskUserQuestion is still sent as a normal tool call; the prose path applies only when + the tool is unavailable or erroring, and never on a `[plan-tune auto-decide]` result. + +#### Fixed +- Section-loading tests use the canonical kebab test names, so the test-coverage gate + matches them. +- External-host doc-freshness checks are deterministic, no longer dependent on a prior + full regeneration. + +#### For contributors +- The eval/E2E runners set `GSTACK_HEADLESS=1` so headless runs classify correctly; + interactive-path suites opt out per-run. +- Per-skill `maxSizeRatio` override in the carve-guards registry; `document-release` + gets 1.08 headroom for the cross-cutting preamble addition while every other skill + keeps the 1.05 ceiling. + ## [1.57.0.0] - 2026-06-07 ## **Three more heavyweight skills load lighter, and every carved skill finally has a test that proves it loads.** diff --git a/SKILL.md b/SKILL.md index 60405f27d..24c498292 100644 --- a/SKILL.md +++ b/SKILL.md @@ -45,6 +45,9 @@ echo "SKILL_PREFIX: $_SKILL_PREFIX" source <(~/.claude/skills/gstack/bin/gstack-repo-mode 2>/dev/null) || true REPO_MODE=${REPO_MODE:-unknown} echo "REPO_MODE: $REPO_MODE" +_SESSION_KIND=$(~/.claude/skills/gstack/bin/gstack-session-kind 2>/dev/null || echo "interactive") +case "$_SESSION_KIND" in spawned|headless|interactive) ;; *) _SESSION_KIND="interactive" ;; esac +echo "SESSION_KIND: $_SESSION_KIND" _LAKE_SEEN=$([ -f ~/.gstack/.completeness-intro-seen ] && echo "yes" || echo "no") echo "LAKE_INTRO: $_LAKE_SEEN" _TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || true) @@ -124,7 +127,7 @@ In plan mode, allowed because they inform the plan: `$B`, `$D`, `codex exec`/`co ## Skill Invocation During Plan Mode -If the user invokes a skill in plan mode, the skill takes precedence over generic plan mode behavior. **Treat the skill file as executable instructions, not reference.** Follow it step by step starting from Step 0; the first AskUserQuestion is the workflow entering plan mode, not a violation of it. AskUserQuestion (any variant — `mcp__*__AskUserQuestion` or native; see "AskUserQuestion Format → Tool resolution") satisfies plan mode's end-of-turn requirement. If no variant is callable, the skill is BLOCKED — stop and report `BLOCKED — AskUserQuestion unavailable` per the AskUserQuestion Format rule. At a STOP point, stop immediately. Do not continue the workflow or call ExitPlanMode there. Commands marked "PLAN MODE EXCEPTION — ALWAYS RUN" execute. Call ExitPlanMode only after the skill workflow completes, or if the user tells you to cancel the skill or leave plan mode. +If the user invokes a skill in plan mode, the skill takes precedence over generic plan mode behavior. **Treat the skill file as executable instructions, not reference.** Follow it step by step starting from Step 0; the first AskUserQuestion is the workflow entering plan mode, not a violation of it. AskUserQuestion (any variant — `mcp__*__AskUserQuestion` or native; see "AskUserQuestion Format → Tool resolution") satisfies plan mode's end-of-turn requirement. If AskUserQuestion is unavailable or a call fails, follow the AskUserQuestion Format failure fallback: `headless` → BLOCKED; `interactive` → the prose fallback (also satisfies end-of-turn). At a STOP point, stop immediately. Do not continue the workflow or call ExitPlanMode there. Commands marked "PLAN MODE EXCEPTION — ALWAYS RUN" execute. Call ExitPlanMode only after the skill workflow completes, or if the user tells you to cancel the skill or leave plan mode. If `PROACTIVE` is `"false"`, do not auto-invoke or proactively suggest skills. If a skill seems useful, ask: "I think /skillname might help here — want me to run it?" diff --git a/VERSION b/VERSION index a17d4bbc0..e97e1faf0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.57.0.0 +1.57.3.0 diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index 5d5f6334c..fd6bb8a21 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -54,6 +54,9 @@ echo "SKILL_PREFIX: $_SKILL_PREFIX" source <(~/.claude/skills/gstack/bin/gstack-repo-mode 2>/dev/null) || true REPO_MODE=${REPO_MODE:-unknown} echo "REPO_MODE: $REPO_MODE" +_SESSION_KIND=$(~/.claude/skills/gstack/bin/gstack-session-kind 2>/dev/null || echo "interactive") +case "$_SESSION_KIND" in spawned|headless|interactive) ;; *) _SESSION_KIND="interactive" ;; esac +echo "SESSION_KIND: $_SESSION_KIND" _LAKE_SEEN=$([ -f ~/.gstack/.completeness-intro-seen ] && echo "yes" || echo "no") echo "LAKE_INTRO: $_LAKE_SEEN" _TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || true) @@ -133,7 +136,7 @@ In plan mode, allowed because they inform the plan: `$B`, `$D`, `codex exec`/`co ## Skill Invocation During Plan Mode -If the user invokes a skill in plan mode, the skill takes precedence over generic plan mode behavior. **Treat the skill file as executable instructions, not reference.** Follow it step by step starting from Step 0; the first AskUserQuestion is the workflow entering plan mode, not a violation of it. AskUserQuestion (any variant — `mcp__*__AskUserQuestion` or native; see "AskUserQuestion Format → Tool resolution") satisfies plan mode's end-of-turn requirement. If no variant is callable, the skill is BLOCKED — stop and report `BLOCKED — AskUserQuestion unavailable` per the AskUserQuestion Format rule. At a STOP point, stop immediately. Do not continue the workflow or call ExitPlanMode there. Commands marked "PLAN MODE EXCEPTION — ALWAYS RUN" execute. Call ExitPlanMode only after the skill workflow completes, or if the user tells you to cancel the skill or leave plan mode. +If the user invokes a skill in plan mode, the skill takes precedence over generic plan mode behavior. **Treat the skill file as executable instructions, not reference.** Follow it step by step starting from Step 0; the first AskUserQuestion is the workflow entering plan mode, not a violation of it. AskUserQuestion (any variant — `mcp__*__AskUserQuestion` or native; see "AskUserQuestion Format → Tool resolution") satisfies plan mode's end-of-turn requirement. If AskUserQuestion is unavailable or a call fails, follow the AskUserQuestion Format failure fallback: `headless` → BLOCKED; `interactive` → the prose fallback (also satisfies end-of-turn). At a STOP point, stop immediately. Do not continue the workflow or call ExitPlanMode there. Commands marked "PLAN MODE EXCEPTION — ALWAYS RUN" execute. Call ExitPlanMode only after the skill workflow completes, or if the user tells you to cancel the skill or leave plan mode. If `PROACTIVE` is `"false"`, do not auto-invoke or proactively suggest skills. If a skill seems useful, ask: "I think /skillname might help here — want me to run it?" @@ -305,11 +308,31 @@ AI orchestrator (e.g., OpenClaw). In spawned sessions: **Rule:** if any `mcp__*__AskUserQuestion` variant is in your tool list, prefer it. Hosts may disable native AUQ via `--disallowedTools AskUserQuestion` (Conductor does, by default) and route through their MCP variant; calling native there silently fails. Same questions/options shape; same decision-brief format applies. -**If no AskUserQuestion variant appears in your tool list, this skill is BLOCKED.** Stop, report `BLOCKED — AskUserQuestion unavailable`, and wait for the user. Do not write decisions to the plan file as a substitute, do not emit them as prose and stop, and do not silently auto-decide (only `/plan-tune` AUTO_DECIDE opt-ins authorize auto-picking). +If AskUserQuestion is unavailable (no variant in your tool list) OR a call to it fails, do NOT silently auto-decide or write the decision to the plan file as a substitute. Follow the **failure fallback** below. + +### When AskUserQuestion is unavailable or a call fails + +Tell three outcomes apart: + +1. **Auto-decide denial (NOT a failure).** The result contains `[plan-tune auto-decide] →