From 65c6d49cd47d319ce4b77ef6a6e153ad1f130159 Mon Sep 17 00:00:00 2001 From: mohamed-elkholy95 Date: Tue, 2 Jun 2026 13:07:49 -0400 Subject: [PATCH 1/9] fix(release): make Homebrew tap a best-effort gate in promote-release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A broken tap (lost HOMEBREW_TAP_TOKEN, org migration, etc.) was blocking the promote-release polling loop indefinitely, preventing the GitHub Release from ever reaching Latest and leaving install scripts pointing at a stale prerelease. Remove homebrew_ready from the hard polling condition. Hard gates are now only required platform assets + PyPI — the channels that actually serve the install scripts and in-app updater. Add a best-effort 'Check Homebrew tap' step that runs after promotion: it checks once and emits a warning annotation + step summary note + Slack alert if the tap lags, then exits 0. The job still succeeds; the warning is visible in the run summary without holding the release hostage. --- .github/workflows/promote-release.yml | 186 ++++++++++++++++---------- 1 file changed, 117 insertions(+), 69 deletions(-) diff --git a/.github/workflows/promote-release.yml b/.github/workflows/promote-release.yml index 58b15668..4b1d1b10 100644 --- a/.github/workflows/promote-release.yml +++ b/.github/workflows/promote-release.yml @@ -4,9 +4,14 @@ name: Promote release # release-pythinker-cli) create the GitHub Release as a PRERELEASE so it stays # out of the date-based /releases/latest endpoint (which ignores make_latest) # until every install channel is ready. This workflow waits for exact release -# assets, PyPI, and the Homebrew formula, then clears `prerelease` and marks the -# release latest — the single point where a version becomes resolvable by the -# install scripts and in-app updater. +# assets and PyPI, then clears `prerelease` and marks the release latest — the +# single point where a version becomes resolvable by the install scripts and +# in-app updater. +# +# Homebrew is checked best-effort AFTER promotion: a broken tap (lost token, +# org migration, etc.) must not hold the GitHub Latest badge and install scripts +# hostage. The Homebrew check emits a warning annotation and step summary note +# so the gap is visible without blocking. # # It runs on the tag push (not `release: published`, which a GITHUB_TOKEN-created # release never fires) so promotion always happens. workflow_dispatch allows a @@ -93,7 +98,6 @@ jobs: "pythinker-${version}-x86_64-apple-darwin-onedir.tar.gz.sha256" ) pypi_url="https://pypi.org/pypi/pythinker-code/${version}/json" - homebrew_formula_url="https://raw.githubusercontent.com/Pythoughts-labs/homebrew-pythinker/main/Formula/pythinker-code.rb" # The budget must comfortably exceed the slowest platform build, since # this job runs on the tag push in parallel with them. The long pole is # linux-installer's emulated arm64 .deb/.rpm step: on the 0.26.0 release @@ -101,6 +105,9 @@ jobs: # timed out — leaving the release stuck as a prerelease. 80x30s=40m # gives ~2x margin; a genuinely stuck build still surfaces as a failed # build workflow, and workflow_dispatch allows a manual re-promote. + # + # Homebrew is NOT checked here — it is best-effort and checked after + # promotion so a broken tap never blocks the GitHub Latest badge. max_attempts=80 poll_interval=30 budget_min=$(( max_attempts * poll_interval / 60 )) @@ -120,15 +127,9 @@ jobs: pypi_ready=true fi - homebrew_ready=false - formula_text=$(curl -fsSL --retry 2 --retry-delay 2 "$homebrew_formula_url" 2>/dev/null || true) - if grep -qF "version \"${version}\"" <<<"$formula_text"; then - homebrew_ready=true - fi - - if [[ "${#missing_assets[@]}" -eq 0 && "$pypi_ready" == "true" && "$homebrew_ready" == "true" ]]; then + if [[ "${#missing_assets[@]}" -eq 0 && "$pypi_ready" == "true" ]]; then all_ready=true - echo "All install channels ready (attempt $i)" + echo "All required install channels ready (attempt $i)" break fi @@ -139,9 +140,6 @@ jobs: if [[ "$pypi_ready" != "true" ]]; then echo "PyPI is not serving ${version} yet: $pypi_url" fi - if [[ "$homebrew_ready" != "true" ]]; then - echo "Homebrew tap formula is not at ${version} yet: $homebrew_formula_url" - fi if [[ "$i" -lt "$max_attempts" ]]; then echo "Retrying in ${poll_interval}s..." sleep "$poll_interval" @@ -163,74 +161,124 @@ jobs: gh api -X PATCH "repos/$REPO/releases/$release_id" -F prerelease=false -f make_latest=true echo "Promoted $TAG: prerelease=false, make_latest=true (all platform assets present)." - - name: Mint GitHub App token for pythinker-home - id: app-token + # Best-effort Homebrew check. The release is ALREADY promoted above; a + # broken tap (lost token, org migration, etc.) must not re-block it. + # This step emits a warning annotation and step summary note so the gap + # is visible, then exits 0. The tap is repaired separately via + # HOMEBREW_TAP_TOKEN; this step just surfaces when it lags behind. + - name: Check Homebrew tap (best-effort) env: - APP_ID: ${{ secrets.PYTHINKER_RELEASE_BOT_APP_ID }} - APP_PRIVATE_KEY: ${{ secrets.PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY }} - DISPATCH_OWNER: Pythoughts-labs - DISPATCH_REPO: pythinker-home + TAG: ${{ steps.tag.outputs.tag }} + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + SOURCE_REPO: ${{ github.repository }} run: | - set -euo pipefail - if [ -z "${APP_ID:-}" ] || [ -z "${APP_PRIVATE_KEY:-}" ]; then - echo "::error::Missing PYTHINKER_RELEASE_BOT_APP_ID or PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY. Configure the org-owned pythinker-release-bot App and install it on ${DISPATCH_OWNER}/${DISPATCH_REPO} with Contents: Read and write." >&2 - exit 1 + # No `set -e`: this step must never fail the job. + set -uo pipefail + version="${TAG#v}" + homebrew_formula_url="https://raw.githubusercontent.com/Pythoughts-labs/homebrew-pythinker/main/Formula/pythinker-code.rb" + formula_text=$(curl -fsSL --retry 2 --retry-delay 2 "$homebrew_formula_url" 2>/dev/null || true) + if grep -qF "version \"${version}\"" <<<"$formula_text"; then + echo "Homebrew tap is at ${version}. ✓" + exit 0 fi + # Tap is behind — warn but do not fail. + reason="Homebrew tap formula is not yet at ${version}: ${homebrew_formula_url}" + echo "::warning title=Homebrew tap lagging::${reason}" + { + echo "### :warning: Homebrew tap is not yet at ${version} (non-blocking)" + echo "" + echo "${reason}" + echo "" + echo "**Non-blocking:** the GitHub Release has already been promoted to Latest." + echo "The tap updates automatically once \`HOMEBREW_TAP_TOKEN\` is valid and the" + echo "\`homebrew-tap\` workflow runs. To fix: restore the token secret and" + echo "re-run the \`homebrew-tap\` workflow for this tag via \`workflow_dispatch\`." + } >> "$GITHUB_STEP_SUMMARY" + if [ -n "${SLACK_WEBHOOK_URL:-}" ]; then + alert=$(jq -n --arg run_url "$RUN_URL" --arg repo "$SOURCE_REPO" \ + --arg version "$version" --arg url "$homebrew_formula_url" \ + '{"text":":warning: *Homebrew tap lagging (non-blocking)*","attachments":[{"color":"warning","fields":[{"title":"Repo","value":$repo,"short":true},{"title":"Expected version","value":$version,"short":true},{"title":"Formula URL","value":$url,"short":false},{"title":"Run","value":"<\($run_url)|View logs>","short":false}]}]}') + curl -sS -X POST -H "Content-Type: application/json" -d "$alert" "$SLACK_WEBHOOK_URL" || true + fi + exit 0 - b64url() { openssl base64 -A | tr '+/' '-_' | tr -d '='; } - key_file=$(mktemp) - trap 'rm -f "$key_file"' EXIT - printf '%s\n' "$APP_PRIVATE_KEY" > "$key_file" - chmod 600 "$key_file" - - now=$(date +%s) - header=$(printf '{"alg":"RS256","typ":"JWT"}' | b64url) - payload=$(jq -nc --argjson iat "$((now - 60))" --argjson exp "$((now + 540))" --arg iss "$APP_ID" '{iat:$iat,exp:$exp,iss:$iss}' | b64url) - unsigned="${header}.${payload}" - signature=$(printf '%s' "$unsigned" | openssl dgst -sha256 -sign "$key_file" | b64url) - jwt="${unsigned}.${signature}" - - installation_id=$(curl --fail-with-body -sS \ - -H "Accept: application/vnd.github+json" \ - -H "Authorization: Bearer ${jwt}" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "https://api.github.com/repos/${DISPATCH_OWNER}/${DISPATCH_REPO}/installation" \ - | jq -er '.id') - token=$(jq -nc --arg repo "$DISPATCH_REPO" '{repositories:[$repo],permissions:{contents:"write"}}' \ - | curl --fail-with-body -sS \ - -X POST \ - -H "Accept: application/vnd.github+json" \ - -H "Authorization: Bearer ${jwt}" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "https://api.github.com/app/installations/${installation_id}/access_tokens" \ - -d @- \ - | jq -er '.token') - echo "::add-mask::$token" - echo "token=$token" >> "$GITHUB_OUTPUT" + # Best-effort website sync trigger. The release is ALREADY promoted above; + # this only accelerates the pythinker-home mirror, which also re-syncs on + # its own daily cron. So a missing/rotated App degrades gracefully here + # rather than failing an otherwise-successful promotion. + - name: Mint GitHub App token for pythinker-home + id: app-token + continue-on-error: true + uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # pinned from v2.2.2 + with: + app-id: ${{ secrets.PYTHINKER_RELEASE_BOT_APP_ID }} + private-key: ${{ secrets.PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY }} + owner: Pythoughts-labs + repositories: pythinker-home + permission-contents: write - - name: Trigger pythinker-home sync + - name: Trigger pythinker-home sync (best-effort) env: DISPATCH_TOKEN: ${{ steps.app-token.outputs.token }} + TOKEN_OUTCOME: ${{ steps.app-token.outcome }} SOURCE_REPO: ${{ github.repository }} RELEASE_TAG: ${{ steps.tag.outputs.tag }} DISPATCH_OWNER: Pythoughts-labs DISPATCH_REPO: pythinker-home + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | - set -euo pipefail - if [ -z "${DISPATCH_TOKEN:-}" ]; then - echo "::error::No dispatch token: the pythinker-release-bot App token mint produced an empty value. Confirm PYTHINKER_RELEASE_BOT_APP_ID and PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY org secrets are set and the App is installed on ${DISPATCH_OWNER}/${DISPATCH_REPO} with Contents: Read and write." >&2 - exit 1 + # No `set -e`: a failed website-sync trigger must not fail an already + # successful release promotion. The daily cron in pythinker-home is + # the real sync guarantee; here we warn + alert and exit 0. + set -uo pipefail + + degrade() { + reason="$1" + echo "::warning title=pythinker-home sync skipped::${reason}" + { + echo "### :warning: pythinker-home sync dispatch skipped (release was still promoted)" + echo "" + echo "${reason}" + echo "" + echo "**Non-blocking:** pythinker-home re-syncs on its daily cron (\`sync-upstream-products\` @ 04:17 UTC), so the website is not stale." + echo "" + echo "**Restore the fast path:** recreate/install the **pythinker-release-bot** App on \`${DISPATCH_OWNER}\` with *Contents: write* on \`${DISPATCH_REPO}\`, then update the \`PYTHINKER_RELEASE_BOT_APP_ID\` and \`PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY\` secrets." + } >> "$GITHUB_STEP_SUMMARY" + if [ -n "${SLACK_WEBHOOK_URL:-}" ]; then + alert=$(jq -n --arg run_url "$RUN_URL" --arg repo "$SOURCE_REPO" --arg reason "$reason" \ + '{"text":":warning: *pythinker-home sync dispatch skipped (non-blocking)*","attachments":[{"color":"warning","fields":[{"title":"Repo","value":$repo,"short":true},{"title":"Reason","value":$reason,"short":false},{"title":"Run","value":"<\($run_url)|View logs>","short":false}]}]}') + curl -sS -X POST -H "Content-Type: application/json" -d "$alert" "$SLACK_WEBHOOK_URL" || true + fi + exit 0 + } + + if [ "${TOKEN_OUTCOME}" != "success" ] || [ -z "${DISPATCH_TOKEN:-}" ]; then + degrade "Could not mint a pythinker-release-bot App token (token step outcome: ${TOKEN_OUTCOME}). The App is likely missing/uninstalled on ${DISPATCH_OWNER}, or its credentials are stale." fi - payload=$(jq -n \ - --arg source_repo "$SOURCE_REPO" \ - --arg tag "$RELEASE_TAG" \ + + payload=$(jq -n --arg source_repo "$SOURCE_REPO" --arg tag "$RELEASE_TAG" \ '{"event_type":"sync-pythinker-products","client_payload":{"source_repo":$source_repo,"tag":$tag}}') - curl --fail-with-body \ - -X POST \ - -H "Accept: application/vnd.github+json" \ - -H "Authorization: Bearer $DISPATCH_TOKEN" \ - "https://api.github.com/repos/${DISPATCH_OWNER}/${DISPATCH_REPO}/dispatches" \ - -d "$payload" + + resp=$(mktemp) + code="000" + for attempt in 1 2 3; do + code=$(curl -sS -o "$resp" -w '%{http_code}' \ + -X POST \ + -H "Accept: application/vnd.github+json" \ + -H "Authorization: Bearer $DISPATCH_TOKEN" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + "https://api.github.com/repos/${DISPATCH_OWNER}/${DISPATCH_REPO}/dispatches" \ + -d "$payload") || code="000" + if [ "$code" = "204" ]; then + echo "Dispatched sync-pythinker-products to ${DISPATCH_OWNER}/${DISPATCH_REPO} (HTTP 204)." + exit 0 + fi + echo "Dispatch attempt ${attempt} returned HTTP ${code}: $(head -c 200 "$resp")" + [ "$attempt" -lt 3 ] && sleep $((attempt * 3)) || true + done + degrade "repository_dispatch to ${DISPATCH_OWNER}/${DISPATCH_REPO} failed after 3 attempts (last HTTP ${code}): $(head -c 200 "$resp")" notify-failure: name: Notify on failure From c4ca0244f6bd7146d66a4b6105c1c4970df6541e Mon Sep 17 00:00:00 2001 From: mohamed-elkholy95 Date: Tue, 2 Jun 2026 13:13:07 -0400 Subject: [PATCH 2/9] fix(release): add CHANGELOG entry for promote-release homebrew soft gate --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index acb15271..278553de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- **Release promotion no longer stalls when the Homebrew tap is broken.** The `promote-release` workflow now gates only on platform assets and PyPI; a lagging or broken Homebrew tap emits a warning annotation and step summary note but no longer blocks the GitHub Release from reaching Latest. + ## 0.30.0 (2026-06-02) ### What changed in this release From 060cf9c4f77b960ffbf86ec31acb3747b98d0eec Mon Sep 17 00:00:00 2001 From: mohamed-elkholy95 Date: Tue, 2 Jun 2026 13:21:08 -0400 Subject: [PATCH 3/9] fix(dx): pre-hook changelog gate before gh pr create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocks gh pr create when shipped-code paths are changed but ## Unreleased in CHANGELOG.md is empty — catches the same gate that changelog-entry-required CI enforces, before the push. Escape hatches (mirroring CI): release/* branch, chore(release) title, or [skip changelog] in the PR body. Also unblocks .claude/settings.json and .claude/hooks/ from .gitignore (changed .claude to .claude/* so negation patterns work) so project-scoped hook config is tracked by the team. --- .claude/hooks/check-changelog.sh | 66 ++++++++++++++++++++++++++++++++ .claude/settings.json | 17 ++++++++ .gitignore | 5 ++- 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100755 .claude/hooks/check-changelog.sh create mode 100644 .claude/settings.json diff --git a/.claude/hooks/check-changelog.sh b/.claude/hooks/check-changelog.sh new file mode 100755 index 00000000..06e96239 --- /dev/null +++ b/.claude/hooks/check-changelog.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# Pre-hook: block `gh pr create` when shipped-code paths are changed but +# CHANGELOG.md has nothing under ## Unreleased. +# Mirrors the logic in .github/workflows/changelog-entry-required.yml so the +# gate fires locally before CI does. + +set -uo pipefail + +input=$(cat) +cmd=$(printf '%s' "$input" | jq -r '.tool_input.command // ""') + +# Only intercept gh pr create invocations. +if ! printf '%s' "$cmd" | grep -q 'gh pr create'; then + exit 0 +fi + +# Skip release-prep branches and titles (they consume ## Unreleased). +branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "") +case "$branch" in + release/*) exit 0 ;; +esac +if printf '%s' "$cmd" | grep -qF 'chore(release)'; then + exit 0 +fi +# [skip changelog] anywhere in the command body is also an escape hatch. +if printf '%s' "$cmd" | grep -qiF '[skip changelog]'; then + exit 0 +fi + +# Determine which files changed vs the merge-base with origin/main. +base=$(git merge-base HEAD origin/main 2>/dev/null || echo "") +if [ -z "$base" ]; then + # Can't determine base — don't block. + exit 0 +fi +changed=$(git diff --name-only "$base" HEAD 2>/dev/null || echo "") + +# Check for shipped-code paths (matches the CI workflow exactly). +touched=0 +while IFS= read -r f; do + [ -n "$f" ] || continue + case "$f" in + src/*|packages/*) touched=1; break ;; + scripts/install*.sh|scripts/install*.ps1) touched=1; break ;; + pythinker.spec) touched=1; break ;; + .github/workflows/linux-installer.yml|\ + .github/workflows/windows-installer.yml|\ + .github/workflows/homebrew-tap.yml|\ + .github/workflows/release-*.yml|\ + .github/workflows/promote-release.yml) touched=1; break ;; + esac +done <<< "$changed" + +[ "$touched" -eq 0 ] && exit 0 + +# Pass if ## Unreleased has at least one non-blank line. +if awk ' + /^## Unreleased[[:space:]]*$/ { inblk=1; next } + inblk && /^## / { inblk=0 } + inblk { print } +' CHANGELOG.md 2>/dev/null | grep -q '[^[:space:]]'; then + exit 0 +fi + +# Block and tell the author exactly what to do. +printf '%s' '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"CHANGELOG gate: this branch touches shipped code but ## Unreleased in CHANGELOG.md is empty.\n\nAdd a bullet under ## Unreleased before opening the PR, for example:\n - **Your change.** Brief description.\n\nEscape hatches:\n - Add [skip changelog] in the PR body\n - Use branch name release/* or title chore(release)*"}}' diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..fc5b99df --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,17 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "bash .claude/hooks/check-changelog.sh", + "timeout": 15, + "statusMessage": "Checking CHANGELOG gate..." + } + ] + } + ] + } +} diff --git a/.gitignore b/.gitignore index f07232cd..d29b4d0c 100644 --- a/.gitignore +++ b/.gitignore @@ -58,7 +58,10 @@ node_modules/ static/ .memo/ .entire -.claude +.claude/* +!.claude/settings.json +!.claude/hooks/ +!.claude/hooks/** .pythinker/ .worktrees/ blackbox/ From f16c88fd88691a16d6131b14d39bd2d01f76e463 Mon Sep 17 00:00:00 2001 From: mohamed-elkholy95 Date: Tue, 2 Jun 2026 13:32:52 -0400 Subject: [PATCH 4/9] feat(tui): improve auto mode and transcript rendering --- .claude/hooks/coderabbit-merge-gate.sh | 117 +++++++++++++ .../dispatch-pythinker-home-sync.yml | 158 ++++++++---------- src/pythinker_code/cli/__init__.py | 12 +- src/pythinker_code/config.py | 18 +- src/pythinker_code/session_recap.py | 32 +++- src/pythinker_code/soul/agent.py | 5 +- src/pythinker_code/soul/approval.py | 59 ++++++- .../soul/dynamic_injections/auto_mode.py | 82 ++++++--- src/pythinker_code/ui/shell/echo.py | 5 +- src/pythinker_code/ui/shell/motion.py | 101 +++++++---- src/pythinker_code/ui/shell/prompt.py | 21 ++- .../ui/shell/selectors/settings.py | 12 ++ .../ui/shell/tool_renderers/todo.py | 4 +- .../ui/shell/visualize/_live_view.py | 24 ++- .../ui/shell/visualize/_worklog.py | 25 ++- src/pythinker_code/ui/theme.py | 46 ++--- src/pythinker_code/utils/rich/markdown.py | 53 ++---- tests/core/test_approval_auto.py | 52 ++++++ tests/core/test_auto_injection.py | 29 +++- tests/core/test_config.py | 4 + tests/core/test_runtime_auto_state.py | 64 +++++++ tests/test_session_recap.py | 14 ++ tests/ui/test_shell_markdown.py | 24 +-- tests/ui_and_conv/test_live_view_todos.py | 25 ++- tests/ui_and_conv/test_prompt_tips.py | 34 +++- tests/ui_and_conv/test_settings_selector.py | 4 + tests/ui_and_conv/test_shell_design_system.py | 2 +- tests/ui_and_conv/test_shell_motion.py | 6 +- .../ui_and_conv/test_shell_motion_shimmer.py | 20 +++ tests/ui_and_conv/test_shell_prompt_echo.py | 15 ++ tests/ui_and_conv/test_thinking_cycle.py | 22 +-- tests/ui_and_conv/test_tui_theme_tokens.py | 15 ++ tests/utils/test_rich_markdown.py | 14 +- 33 files changed, 833 insertions(+), 285 deletions(-) create mode 100755 .claude/hooks/coderabbit-merge-gate.sh diff --git a/.claude/hooks/coderabbit-merge-gate.sh b/.claude/hooks/coderabbit-merge-gate.sh new file mode 100755 index 00000000..15727ba4 --- /dev/null +++ b/.claude/hooks/coderabbit-merge-gate.sh @@ -0,0 +1,117 @@ +#!/usr/bin/env bash +# +# coderabbit-merge-gate.sh +# +# PreToolUse(Bash) hook. Gates `gh pr merge` on CodeRabbit having FINISHED its +# review of the PR's head commit ("review-complete" gate): +# +# - CodeRabbit commit status on head == success -> allow (surface findings) +# - status == pending -> BLOCK (still reviewing) +# - status == failure/error -> BLOCK (problem) +# - status absent / cannot verify -> BLOCK (not reviewed yet) +# +# A finished review lets the merge proceed even if it has actionable comments +# (that is the "resolve-all-issues" gate, deliberately not enabled here). The +# count is surfaced so it is not silently ignored. Every failure path fails +# SAFE to BLOCK -- it never silently allows a merge it could not verify. +# +# Blocking uses permissionDecision:"deny" rather than "ask" on purpose: this +# environment runs defaultMode:auto + skipAutoPermissionPrompt, which silently +# auto-approves "ask", making it a no-op. "deny" is a hard block. To override +# (e.g. CodeRabbit is down), edit/remove this hook in .claude/settings.local.json +# or run the merge yourself outside the agent. +# +# Authoritative signal is the `CodeRabbit` commit status (set by commit_status: +# true in .coderabbit.yaml): pending while reviewing, success when complete. A +# new push resets it to pending, so this is inherently staleness-proof. +# +# Reads the hook payload on stdin, emits a PreToolUse decision as JSON on stdout. + +set -o pipefail + +input="$(cat)" +cmd="$(printf '%s' "$input" | jq -r '.tool_input.command // ""' 2>/dev/null)" + +# Fast path: only act when `gh pr merge` is an actual command, not a substring. +# Anchor it to a command boundary (line start, &&, ;, |, then, do) so it still +# catches compound forms (`cd x && gh pr merge 9`) but NOT mentions inside +# `echo "... gh pr merge ..."`, `git commit -m "... gh pr merge ..."`, or +# `rg "gh pr merge"`. Anything else passes untouched. +if ! printf '%s' "$cmd" | grep -qE '(^|&&|;|\||\bthen\b|\bdo\b)[[:space:]]*gh[[:space:]]+pr[[:space:]]+merge([[:space:]]|$)'; then + exit 0 +fi + +# Emit a hard "deny" decision (blocks the merge) and exit. +block() { + jq -nc --arg r "$1" \ + '{hookSpecificOutput:{hookEventName:"PreToolUse",permissionDecision:"deny",permissionDecisionReason:$r}}' + exit 0 +} + +# Inject context for the model but do not block (normal permission flow continues). +note() { + jq -nc --arg c "$1" \ + '{hookSpecificOutput:{hookEventName:"PreToolUse",additionalContext:$c}}' + exit 0 +} + +command -v gh >/dev/null 2>&1 || block "CodeRabbit gate: 'gh' not found — cannot verify CodeRabbit review. Confirm manually before merging." +command -v jq >/dev/null 2>&1 || exit 0 # jq missing: cannot build payload; do not block. + +# Target repo: honor -R/--repo on the merge command, else the current repo. +repo="$(printf '%s' "$cmd" | grep -oE '(-R|--repo)[ =]+[^ ]+' | head -1 | sed -E 's/^(-R|--repo)[ =]+//')" +repo_args=() +[ -n "$repo" ] && repo_args=(--repo "$repo") + +# PR number: prefer pull/ from a URL, then a bare integer argument, else the +# current branch's PR. (A bare-digit token avoids grabbing digits inside an +# owner name such as ".../mohamed-elkholy95/...".) +args="$(printf '%s' "$cmd" | sed -E 's/.*gh[[:space:]]+pr[[:space:]]+merge//')" +pr="$(printf '%s' "$args" | grep -oE 'pull/[0-9]+' | head -1 | grep -oE '[0-9]+')" +if [ -z "$pr" ]; then + pr="$(printf '%s' "$args" | tr ' ' '\n' | grep -xE '[0-9]+' | head -1)" +fi +if [ -z "$pr" ]; then + pr="$(gh pr view "${repo_args[@]}" --json number --jq '.number' 2>/dev/null)" +fi +[ -n "$pr" ] || block "CodeRabbit gate: could not determine the PR for this merge. Confirm CodeRabbit reviewed it, then merge." + +# owner/repo for the commit-status API. +nwo="$repo" +[ -n "$nwo" ] || nwo="$(gh repo view --json nameWithOwner --jq '.nameWithOwner' 2>/dev/null)" +[ -n "$nwo" ] || block "CodeRabbit gate: could not resolve the repository for PR #$pr. Confirm CodeRabbit review, then merge." + +# Head commit of the PR. +sha="$(gh pr view "$pr" "${repo_args[@]}" --json commits --jq '.commits[-1].oid' 2>/dev/null)" +[ -n "$sha" ] || block "CodeRabbit gate: could not read PR #$pr head commit. Confirm CodeRabbit review, then merge." + +# CodeRabbit commit status on the head commit. +cr_state="$(gh api "repos/$nwo/commits/$sha/status" \ + --jq '.statuses[] | select(.context=="CodeRabbit") | .state' 2>/dev/null | head -1)" + +# Latest "Actionable comments posted: N" from CodeRabbit's completion comment. +actionable="$(gh pr view "$pr" "${repo_args[@]}" --json comments --jq ' + [ .comments[] + | select(.author.login=="coderabbitai") + | select(.body | contains("coderabbit-review-completion-marker")) + | (.body | capture("Actionable comments posted: (?[0-9]+)").n) + ] | last // "unknown"' 2>/dev/null)" + +case "$cr_state" in + success) + if [ "$actionable" = "0" ] || [ "$actionable" = "unknown" ] || [ -z "$actionable" ]; then + note "CodeRabbit review complete on PR #$pr (no actionable comments). Proceeding." + else + note "CodeRabbit review complete on PR #$pr with $actionable actionable comment(s). Review-complete gate allows the merge — confirm those were addressed/resolved before merging." + fi + ;; + pending) + block "CodeRabbit is still reviewing the latest commit on PR #$pr (status: pending). Wait for the review to finish before merging." + ;; + failure|error) + block "CodeRabbit commit status on PR #$pr is '$cr_state'. Investigate and resolve before merging." + ;; + *) + block "No CodeRabbit review found on PR #$pr's head commit ($sha). CodeRabbit may not have reviewed this push yet (or is not enabled here). Confirm before merging." + ;; +esac diff --git a/.github/workflows/dispatch-pythinker-home-sync.yml b/.github/workflows/dispatch-pythinker-home-sync.yml index 03f43636..460fd1c2 100644 --- a/.github/workflows/dispatch-pythinker-home-sync.yml +++ b/.github/workflows/dispatch-pythinker-home-sync.yml @@ -1,10 +1,16 @@ name: Dispatch pythinker-home sync # Triggers a pythinker-home website sync when the install scripts or README -# change on main. Release promotion and the post-release sync live in -# promote-release.yml: a GITHUB_TOKEN-created release never fires a workflow, -# so the old `release: published` trigger here was dead code that never ran the -# wait-for-assets / mark-latest steps. +# change on main. This is a best-effort latency optimization, NOT the source of +# truth: pythinker-home re-syncs on its own daily cron (sync-upstream-products +# @ 04:17 UTC) using its own GITHUB_TOKEN, so a failed dispatch here never +# leaves the website stale. The job therefore degrades gracefully (warns + +# alerts, exits 0) instead of red-lining main when the org-owned +# pythinker-release-bot App is missing, uninstalled, or rotated. +# +# Release promotion's post-release sync lives in promote-release.yml: a +# GITHUB_TOKEN-created release never fires a workflow, so the old +# `release: published` trigger here was dead code that never ran. on: workflow_dispatch: @@ -28,99 +34,79 @@ jobs: steps: # Mint a short-lived installation token for the org-owned # pythinker-release-bot App (Contents: write on pythinker-home only). - # Replaces a personal PAT: org-owned (survives member/org changes), - # ~1h TTL, minted fresh each run, scoped to the single private site repo. + # `continue-on-error` is deliberate: a missing/rotated App must not fail + # the run — the next step detects the empty token and degrades gracefully. - name: Mint GitHub App token for pythinker-home id: app-token - env: - APP_ID: ${{ secrets.PYTHINKER_RELEASE_BOT_APP_ID }} - APP_PRIVATE_KEY: ${{ secrets.PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY }} - DISPATCH_OWNER: ${{ env.DISPATCH_OWNER }} - DISPATCH_REPO: ${{ env.DISPATCH_REPO }} - run: | - set -euo pipefail - if [ -z "${APP_ID:-}" ] || [ -z "${APP_PRIVATE_KEY:-}" ]; then - echo "::error::Missing PYTHINKER_RELEASE_BOT_APP_ID or PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY. Configure the org-owned pythinker-release-bot App and install it on ${DISPATCH_OWNER}/${DISPATCH_REPO} with Contents: Read and write." >&2 - exit 1 - fi + continue-on-error: true + uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # pinned from v2.2.2 + with: + app-id: ${{ secrets.PYTHINKER_RELEASE_BOT_APP_ID }} + private-key: ${{ secrets.PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY }} + owner: ${{ env.DISPATCH_OWNER }} + repositories: ${{ env.DISPATCH_REPO }} + permission-contents: write - b64url() { openssl base64 -A | tr '+/' '-_' | tr -d '='; } - key_file=$(mktemp) - trap 'rm -f "$key_file"' EXIT - printf '%s\n' "$APP_PRIVATE_KEY" > "$key_file" - chmod 600 "$key_file" - - now=$(date +%s) - header=$(printf '{"alg":"RS256","typ":"JWT"}' | b64url) - payload=$(jq -nc --argjson iat "$((now - 60))" --argjson exp "$((now + 540))" --arg iss "$APP_ID" '{iat:$iat,exp:$exp,iss:$iss}' | b64url) - unsigned="${header}.${payload}" - signature=$(printf '%s' "$unsigned" | openssl dgst -sha256 -sign "$key_file" | b64url) - jwt="${unsigned}.${signature}" - - installation_id=$(curl --fail-with-body -sS \ - -H "Accept: application/vnd.github+json" \ - -H "Authorization: Bearer ${jwt}" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "https://api.github.com/repos/${DISPATCH_OWNER}/${DISPATCH_REPO}/installation" \ - | jq -er '.id') - token=$(jq -nc --arg repo "$DISPATCH_REPO" '{repositories:[$repo],permissions:{contents:"write"}}' \ - | curl --fail-with-body -sS \ - -X POST \ - -H "Accept: application/vnd.github+json" \ - -H "Authorization: Bearer ${jwt}" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "https://api.github.com/app/installations/${installation_id}/access_tokens" \ - -d @- \ - | jq -er '.token') - echo "::add-mask::$token" - echo "token=$token" >> "$GITHUB_OUTPUT" - - - name: Trigger pythinker-home sync + - name: Trigger pythinker-home sync (best-effort) env: DISPATCH_TOKEN: ${{ steps.app-token.outputs.token }} + TOKEN_OUTCOME: ${{ steps.app-token.outcome }} SOURCE_REPO: ${{ github.repository }} RELEASE_TAG: ${{ github.sha }} DISPATCH_OWNER: ${{ env.DISPATCH_OWNER }} DISPATCH_REPO: ${{ env.DISPATCH_REPO }} - run: | - set -euo pipefail - if [ -z "${DISPATCH_TOKEN:-}" ]; then - echo "::error::No dispatch token: the pythinker-release-bot App token mint produced an empty value. Confirm PYTHINKER_RELEASE_BOT_APP_ID and PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY org secrets are set and the App is installed on ${DISPATCH_OWNER}/${DISPATCH_REPO} with Contents: Read and write." >&2 - exit 1 - fi - payload=$(jq -n \ - --arg source_repo "$SOURCE_REPO" \ - --arg tag "$RELEASE_TAG" \ - '{"event_type":"sync-pythinker-products","client_payload":{"source_repo":$source_repo,"tag":$tag}}') - curl --fail-with-body \ - -X POST \ - -H "Accept: application/vnd.github+json" \ - -H "Authorization: Bearer $DISPATCH_TOKEN" \ - "https://api.github.com/repos/${DISPATCH_OWNER}/${DISPATCH_REPO}/dispatches" \ - -d "$payload" - - notify-failure: - name: Notify on failure - runs-on: ubuntu-latest - needs: dispatch - if: failure() - steps: - - name: Post Slack alert - env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - REPO: ${{ github.repository }} - TRIGGER: ${{ github.event_name }} run: | - if [ -z "$SLACK_WEBHOOK_URL" ]; then + # No `set -e`: anticipated failures degrade gracefully (warn + alert + + # exit 0) so a broken App never red-lines main. The daily cron in + # pythinker-home is the real sync guarantee. + set -uo pipefail + + degrade() { + reason="$1" + echo "::warning title=pythinker-home sync skipped::${reason}" + { + echo "### :warning: pythinker-home sync dispatch skipped" + echo "" + echo "${reason}" + echo "" + echo "**Non-blocking:** pythinker-home re-syncs on its daily cron (\`sync-upstream-products\` @ 04:17 UTC) using its own token, so the website is not stale." + echo "" + echo "**Restore the fast path:** recreate/install the **pythinker-release-bot** App on \`${DISPATCH_OWNER}\` with *Contents: write* on \`${DISPATCH_REPO}\`, then update the \`PYTHINKER_RELEASE_BOT_APP_ID\` and \`PYTHINKER_RELEASE_BOT_APP_PRIVATE_KEY\` secrets." + } >> "$GITHUB_STEP_SUMMARY" + if [ -n "${SLACK_WEBHOOK_URL:-}" ]; then + alert=$(jq -n --arg run_url "$RUN_URL" --arg repo "$SOURCE_REPO" --arg reason "$reason" \ + '{"text":":warning: *pythinker-home sync dispatch skipped (non-blocking)*","attachments":[{"color":"warning","fields":[{"title":"Repo","value":$repo,"short":true},{"title":"Reason","value":$reason,"short":false},{"title":"Run","value":"<\($run_url)|View logs>","short":false}]}]}') + curl -sS -X POST -H "Content-Type: application/json" -d "$alert" "$SLACK_WEBHOOK_URL" || true + fi exit 0 + } + + if [ "${TOKEN_OUTCOME}" != "success" ] || [ -z "${DISPATCH_TOKEN:-}" ]; then + degrade "Could not mint a pythinker-release-bot App token (token step outcome: ${TOKEN_OUTCOME}). The App is likely missing/uninstalled on ${DISPATCH_OWNER}, or its credentials are stale." fi - payload=$(jq -n \ - --arg run_url "$RUN_URL" \ - --arg repo "$REPO" \ - --arg trigger "$TRIGGER" \ - '{"text":":red_circle: *Dispatch pythinker-home sync failed*","attachments":[{"color":"danger","fields":[{"title":"Repo","value":$repo,"short":true},{"title":"Trigger","value":$trigger,"short":true},{"title":"Run","value":"<\($run_url)|View logs>","short":false}]}]}') - curl --fail-with-body -X POST \ - -H "Content-Type: application/json" \ - -d "$payload" \ - "$SLACK_WEBHOOK_URL" + + payload=$(jq -n --arg source_repo "$SOURCE_REPO" --arg tag "$RELEASE_TAG" \ + '{"event_type":"sync-pythinker-products","client_payload":{"source_repo":$source_repo,"tag":$tag}}') + + # repository_dispatch returns 204 on success. Retry transient errors; + # a persistent failure degrades (the cron still backstops the sync). + resp=$(mktemp) + code="000" + for attempt in 1 2 3; do + code=$(curl -sS -o "$resp" -w '%{http_code}' \ + -X POST \ + -H "Accept: application/vnd.github+json" \ + -H "Authorization: Bearer $DISPATCH_TOKEN" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + "https://api.github.com/repos/${DISPATCH_OWNER}/${DISPATCH_REPO}/dispatches" \ + -d "$payload") || code="000" + if [ "$code" = "204" ]; then + echo "Dispatched sync-pythinker-products to ${DISPATCH_OWNER}/${DISPATCH_REPO} (HTTP 204)." + exit 0 + fi + echo "Dispatch attempt ${attempt} returned HTTP ${code}: $(head -c 200 "$resp")" + [ "$attempt" -lt 3 ] && sleep $((attempt * 3)) || true + done + degrade "repository_dispatch to ${DISPATCH_OWNER}/${DISPATCH_REPO} failed after 3 attempts (last HTTP ${code}): $(head -c 200 "$resp")" diff --git a/src/pythinker_code/cli/__init__.py b/src/pythinker_code/cli/__init__.py index fc2eed31..c6e3585b 100644 --- a/src/pythinker_code/cli/__init__.py +++ b/src/pythinker_code/cli/__init__.py @@ -390,9 +390,11 @@ def pythinker( typer.Option( "--auto", help=( - "Run in auto mode: no user is present, AskUserQuestion is auto-dismissed, " - "and tool calls are auto-approved. Use when running unattended " - "(scripts, CI, scheduled jobs). Default: no." + "Run in auto mode: no user is present and AskUserQuestion is " + "auto-dismissed. Tool calls are auto-approved only when current " + "trust/safe-mode policy permits; otherwise approval-required actions " + "fail closed. Use when running unattended (scripts, CI, scheduled jobs). " + "Default: no." ), ), ] = False, @@ -412,7 +414,9 @@ def pythinker( "--print", help=( "Run in print mode (non-interactive). Print mode auto-dismisses " - "AskUserQuestion and auto-approves tool calls for this invocation." + "AskUserQuestion and enables invocation-only auto mode; approval-required " + "tool calls still follow trust/safe-mode policy and fail closed when " + "approval is unavailable." ), ), ] = False, diff --git a/src/pythinker_code/config.py b/src/pythinker_code/config.py index 718edfe1..d3adb4eb 100644 --- a/src/pythinker_code/config.py +++ b/src/pythinker_code/config.py @@ -374,11 +374,17 @@ class Config(BaseModel): description=( "Controls AskUserQuestion behavior: always ask, ask except in auto mode, " "never pause (best judgment), or auto_deliberate (in auto mode, run an " - "advisor-assisted self-decision instead of dismissing, and bounce " - "destructive actions once for deliberation)." + "advisor-assisted self-decision instead of dismissing)." ), ) ) + auto_deliberate_destructive_actions: bool = Field( + default=False, + description=( + "When true, destructive auto-approved actions are bounced once for agent " + "deliberation before running. This is independent of AskUserQuestion policy." + ), + ) skip_auto_prompt_injection: bool = Field( default=False, description=( @@ -481,12 +487,10 @@ def _apply_agent_execution_profile(self) -> None: if "default_yolo" not in fields_set: self.default_yolo = True if "ask_user_question_policy" not in fields_set: - # NOTE: spec §6 #1 proposes shifting this to "auto_deliberate", but - # that is only safe once the profile also enables auto mode (so - # AskUserQuestion's Entry A self-decision engages and never blocks a - # headless run waiting for an absent user). Until that auto-from- - # profile path exists, keep the robust "never" (always dismiss). + # Preserve no-human AskUserQuestion behavior for headless autonomy. self.ask_user_question_policy = "never" + if "auto_deliberate_destructive_actions" not in fields_set: + self.auto_deliberate_destructive_actions = True elif profile == "plan_only": if "default_plan_mode" not in fields_set: self.default_plan_mode = True diff --git a/src/pythinker_code/session_recap.py b/src/pythinker_code/session_recap.py index 9fa9c74d..de3dd1c5 100644 --- a/src/pythinker_code/session_recap.py +++ b/src/pythinker_code/session_recap.py @@ -253,12 +253,14 @@ def _last_substantive_thread(items: list[SessionRecapItem]) -> str: _MIN_RECAP_SENTENCE_CHARS = 16 _FENCE_START_RE = re.compile(r"^ {0,3}(```+|~~~+)") +_TABLE_DELIMITER_RE = re.compile(r"^\s*\|?\s*:?-{3,}:?\s*(?:\|\s*:?-{3,}:?\s*)+\|?\s*$") def _recap_source_text(text: str) -> str: - """Return one-line recap input with machine-readable blocks removed.""" + """Return one-line recap input with bulky structured blocks removed.""" without_fences = _strip_fenced_blocks(text) - without_ticks = without_fences.replace("`", "") + without_tables = _strip_markdown_tables(without_fences) + without_ticks = without_tables.replace("`", "") return " ".join(without_ticks.split()) @@ -282,6 +284,32 @@ def _strip_fenced_blocks(text: str) -> str: return "\n".join(lines) +def _is_table_delimiter(line: str) -> bool: + return _TABLE_DELIMITER_RE.match(line) is not None + + +def _is_table_row(line: str) -> bool: + stripped = line.strip() + return stripped.startswith("|") and stripped.endswith("|") and stripped.count("|") >= 2 + + +def _strip_markdown_tables(text: str) -> str: + lines = text.splitlines() + kept: list[str] = [] + index = 0 + while index < len(lines): + line = lines[index] + next_line = lines[index + 1] if index + 1 < len(lines) else "" + if _is_table_row(line) and _is_table_delimiter(next_line): + index += 2 + while index < len(lines) and _is_table_row(lines[index]): + index += 1 + continue + kept.append(line) + index += 1 + return "\n".join(kept) + + def _first_sentence(text: str) -> str: cleaned = " ".join(text.split()) if not cleaned: diff --git a/src/pythinker_code/soul/agent.py b/src/pythinker_code/soul/agent.py index d5dfe4d2..f1ff6399 100644 --- a/src/pythinker_code/soul/agent.py +++ b/src/pythinker_code/soul/agent.py @@ -300,7 +300,10 @@ def _on_approval_change() -> None: auto=session.state.approval.auto, runtime_auto=runtime_auto, safe_mode=effective_safe_mode, - auto_deliberate=config.ask_user_question_policy == "auto_deliberate", + auto_deliberate=( + config.auto_deliberate_destructive_actions + or config.ask_user_question_policy == "auto_deliberate" + ), auto_approve_actions=saved_actions, on_change=_on_approval_change, ) diff --git a/src/pythinker_code/soul/approval.py b/src/pythinker_code/soul/approval.py index 0e3254ac..bd142dcb 100644 --- a/src/pythinker_code/soul/approval.py +++ b/src/pythinker_code/soul/approval.py @@ -31,6 +31,19 @@ "enumerate the realistic alternatives, weigh them against the current task, and commit " "to the best one. If this exact action is still right, re-issue it and it will run." ) +_EDIT_OUTSIDE_ACTION = "edit file outside of working directory" +_SAFE_MODE_UNATTENDED_FEEDBACK = ( + "Approval is required for this action, but this run is in auto/non-interactive mode " + "and safe mode prevents auto-approval. The action was denied instead of waiting " + "indefinitely for a user who is not present. Trust the workspace first or rerun with " + "explicit yolo/--yes after verifying the action is safe." +) +_OUTSIDE_WORKSPACE_UNATTENDED_FEEDBACK = ( + "Outside-workspace file changes require explicit approval. Auto mode does not " + "auto-approve them, even in a trusted workspace, because they cross the workspace " + "trust boundary. Rerun interactively, or use explicit yolo/--yes only after verifying " + "the exact path and change are safe." +) @dataclass(frozen=True) @@ -63,13 +76,21 @@ def deliberation_scope(context_id: str, generation: int) -> Generator[None, None class ApprovalResult: """Result of an approval request. Behaves as bool for backward compatibility.""" - __slots__ = ("approved", "feedback", "deliberation") + __slots__ = ("approved", "feedback", "deliberation", "user_rejection") - def __init__(self, approved: bool, feedback: str = "", deliberation: bool = False): + def __init__( + self, + approved: bool, + feedback: str = "", + deliberation: bool = False, + user_rejection: bool = True, + ): self.approved = approved self.feedback = feedback self.deliberation = deliberation """True when the bounce is an auto-mode deliberation prompt, not a user rejection.""" + self.user_rejection = user_rejection + """True when the denial came from a user-backed approval response.""" def __bool__(self) -> bool: return self.approved @@ -84,6 +105,12 @@ def rejection_error(self) -> ToolRejectedError: has_feedback=True, ) if self.feedback: + if not self.user_rejection: + return ToolRejectedError( + message=self.feedback, + brief="Approval unavailable", + has_feedback=True, + ) return ToolRejectedError( message=(f"The tool call is rejected by the user. User feedback: {self.feedback}"), brief=f"Rejected: {self.feedback}", @@ -128,10 +155,10 @@ def __init__( self.auto_deliberate = auto_deliberate """When true, destructive auto-approved actions must deliberate once first. - Wired in ``Runtime.create`` from ``config.ask_user_question_policy == - "auto_deliberate"``. Gates *ahead* of yolo/auto: an irreversible action - (``rm -rf``, ``git push --force``, ...) is bounced back once for the agent - to weigh alternatives before it runs. + Wired in ``Runtime.create`` from the destructive deliberation config flag + and the legacy ``ask_user_question_policy == "auto_deliberate"`` mode. Gates + *ahead* of yolo/auto: an irreversible action (``rm -rf``, ``git push --force``, + ...) is bounced back once for the agent to weigh alternatives before it runs. """ self.auto_approve_actions: set[str] = auto_approve_actions or set() """Set of action names that should automatically be approved.""" @@ -226,6 +253,16 @@ def is_runtime_auto(self) -> bool: """True only when auto mode came from this invocation.""" return self._state.runtime_auto + def _unattended_denial_feedback(self, action: str) -> str | None: + """Fail closed when an unattended run would otherwise wait for approval forever.""" + if not self.is_auto() or self._state.yolo: + return None + if str(action) == _EDIT_OUTSIDE_ACTION: + return _OUTSIDE_WORKSPACE_UNATTENDED_FEEDBACK + if self._state.safe_mode and action not in self._state.auto_approve_actions: + return _SAFE_MODE_UNATTENDED_FEEDBACK + return None + def is_orchestration_approved(self, fingerprint: str) -> bool: return fingerprint in self._state.approved_orchestration_fingerprints @@ -355,6 +392,16 @@ async def request( feedback=_DELIBERATION_FEEDBACK.format(reason=reason), deliberation=True, ) + if (feedback := self._unattended_denial_feedback(action)) is not None: + from pythinker_code.telemetry import track + + track( + "tool_rejected", + tool_name=tool_call.function.name, + approval_mode="auto_unavailable", + ) + return ApprovalResult(approved=False, feedback=feedback, user_rejection=False) + if self.is_auto_approve(): from pythinker_code.telemetry import track diff --git a/src/pythinker_code/soul/dynamic_injections/auto_mode.py b/src/pythinker_code/soul/dynamic_injections/auto_mode.py index b9b3e2f8..17e10471 100644 --- a/src/pythinker_code/soul/dynamic_injections/auto_mode.py +++ b/src/pythinker_code/soul/dynamic_injections/auto_mode.py @@ -13,31 +13,54 @@ _AUTO_INJECTION_TYPE = "auto_mode" _AUTO_PROMPT = ( - "You are running in auto mode. No user is present to answer " - "questions or approve actions. All tool calls are auto-approved by " - "the harness.\n" - "- Do NOT call AskUserQuestion — it will be auto-dismissed with no " - "answer, wasting a turn. Make your best judgment and proceed.\n" - "- You CAN use EnterPlanMode / ExitPlanMode normally. They will be " - "auto-approved. Planning still helps you think before acting; use " - "it for non-trivial tasks, then exit and execute.\n" - "- Finish the user's request end-to-end in this run. Do not defer " - "decisions to a human." + "You are running in auto mode. No user is present to answer questions or " + "approve actions.\n" + "- Do NOT call AskUserQuestion — it will be auto-dismissed with no answer, " + "wasting a turn. Make your best judgment and proceed.\n" + "- Tool calls are auto-approved only when the current trust/safe-mode policy " + "allows. If approval is unavailable, the tool fails closed instead of " + "waiting forever; choose a safe alternative or explain the required explicit " + "trust/yolo step.\n" + "- Outside-workspace file writes are not auto-approved by auto mode.\n" + "- You CAN use EnterPlanMode / ExitPlanMode normally when available. Planning " + "still helps you think before acting; use it for non-trivial tasks, then " + "exit and execute.\n" + "- Finish the user's request end-to-end in this run. Do not defer decisions " + "to a human." +) + +_AUTO_PROMPT_DESTRUCTIVE_DELIBERATE = ( + "You are running in auto mode. No user is present to answer questions or " + "approve actions.\n" + "- Do NOT call AskUserQuestion — it will be auto-dismissed with no answer, " + "wasting a turn. Make your best judgment and proceed.\n" + "- Tool calls are auto-approved only when the current trust/safe-mode policy " + "allows. If approval is unavailable, the tool fails closed instead of " + "waiting forever; choose a safe alternative or explain the required explicit " + "trust/yolo step.\n" + "- Irreversible auto-approved actions may be bounced once for deliberation. " + "Weigh alternatives, then retry only if the exact action is still right.\n" + "- Outside-workspace file writes are not auto-approved by auto mode.\n" + "- Finish the user's request end-to-end in this run. Do not defer decisions " + "to a human." ) _AUTO_PROMPT_DELIBERATE = ( - "You are running in auto mode. No user is present to answer " - "questions or approve actions. Most tool calls are auto-approved by " - "the harness; irreversible ones may be bounced once for deliberation.\n" + "You are running in auto mode. No user is present to answer questions or " + "approve actions.\n" + "- Tool calls are auto-approved only when the current trust/safe-mode policy " + "allows. If approval is unavailable, the tool fails closed instead of " + "waiting forever; choose a safe alternative or explain the required explicit " + "trust/yolo step.\n" + "- Irreversible auto-approved actions may be bounced once for deliberation. " + "Weigh alternatives, then retry only if the exact action is still right.\n" "- At a genuine, consequential, hard-to-reverse fork, you MAY call " - "AskUserQuestion: it triggers an advisor-assisted self-decision (you " - "still decide). Do NOT ask routine confirmations or progress " - "check-ins — proceed instantly on trivial, reversible choices.\n" - "- You CAN use EnterPlanMode / ExitPlanMode normally. They will be " - "auto-approved. Planning still helps you think before acting; use " - "it for non-trivial tasks, then exit and execute.\n" - "- Finish the user's request end-to-end in this run. Do not defer " - "decisions to a human." + "AskUserQuestion: it triggers an advisor-assisted self-decision (you still " + "decide). Do NOT ask routine confirmations or progress check-ins — proceed " + "instantly on trivial, reversible choices.\n" + "- Outside-workspace file writes are not auto-approved by auto mode.\n" + "- Finish the user's request end-to-end in this run. Do not defer decisions " + "to a human." ) AUTO_DISABLED_REMINDER = ( @@ -66,17 +89,24 @@ async def get_injections( _ = history if not soul.is_auto: return [] - if not soul.is_auto_flag: - return [] if self._injected: return [] self._injected = True # Under the auto_deliberate policy AskUserQuestion self-decides (advisor- # assisted) instead of being dismissed, so invite it at consequential - # forks; every other policy keeps the "do not call it" guidance. - deliberate = soul.runtime.config.ask_user_question_policy == "auto_deliberate" - content = _AUTO_PROMPT_DELIBERATE if deliberate else _AUTO_PROMPT + # forks. Destructive deliberation can also be enabled independently while + # AskUserQuestion remains auto-dismissed. + ask_deliberate = soul.runtime.config.ask_user_question_policy == "auto_deliberate" + destructive_deliberate = ( + soul.runtime.config.auto_deliberate_destructive_actions or ask_deliberate + ) + if ask_deliberate: + content = _AUTO_PROMPT_DELIBERATE + elif destructive_deliberate: + content = _AUTO_PROMPT_DESTRUCTIVE_DELIBERATE + else: + content = _AUTO_PROMPT return [DynamicInjection(type=_AUTO_INJECTION_TYPE, content=content)] async def on_context_compacted(self) -> None: diff --git a/src/pythinker_code/ui/shell/echo.py b/src/pythinker_code/ui/shell/echo.py index 26cfc55d..c6e10093 100644 --- a/src/pythinker_code/ui/shell/echo.py +++ b/src/pythinker_code/ui/shell/echo.py @@ -5,6 +5,7 @@ from rich.measure import Measurement from rich.text import Text +from pythinker_code.ui.shell.components.markdown import PythinkerMarkdown from pythinker_code.ui.shell.prompt import PROMPT_SYMBOL_AGENT_INPUT from pythinker_code.ui.shell.spacing import BLANK_ROW from pythinker_code.utils.message import message_stringify @@ -17,7 +18,9 @@ class UserEcho: def __init__(self, text: str) -> None: self._text = text self.plain = f"{PROMPT_SYMBOL_AGENT_INPUT} {text}" - self._body = BulletColumns(Text(text), bullet=Text(PROMPT_SYMBOL_AGENT_INPUT), padding=1) + self._body = BulletColumns( + PythinkerMarkdown(text), bullet=Text(PROMPT_SYMBOL_AGENT_INPUT), padding=1 + ) def __rich_measure__(self, console: Console, options: ConsoleOptions) -> Measurement: return Measurement.get(console, options, self._body) diff --git a/src/pythinker_code/ui/shell/motion.py b/src/pythinker_code/ui/shell/motion.py index fa7fe851..5c6f02d0 100644 --- a/src/pythinker_code/ui/shell/motion.py +++ b/src/pythinker_code/ui/shell/motion.py @@ -23,44 +23,69 @@ TRANSCRIPT_ACTIVE_MARKER, ) from pythinker_code.ui.terminal_capabilities import colors_disabled, motion_disabled -from pythinker_code.ui.theme import tui_rich_style +from pythinker_code.ui.theme import get_tui_tokens, tui_rich_style from pythinker_code.utils.datetime import format_elapsed _FRAMES = SPINNER_FRAMES _FRAME_INTERVAL_S = SPINNER_FRAME_INTERVAL_S +def _activity_color_tokens() -> tuple[str, str, str, str]: + """Return theme-standardized activity colors. + + The active verb uses a premium champagne/platinum ramp in dark mode and a + contrast-safe bronze/ink ramp in light mode. Keep these in theme tokens so + Rich and prompt_toolkit renderers stay visually aligned. + """ + tokens = get_tui_tokens() + return ( + tokens.activity_verb, + tokens.activity_verb_mid, + tokens.activity_verb_highlight, + tokens.activity_spinner, + ) + + def verb_spinner_style() -> Style: - """Muted orange-yellow style for the active verb spinner word.""" + """Theme-standardized style for the active verb spinner word.""" if colors_disabled(): return Style() - return Style(color=Color.parse(_SHIMMER_BASE)) + base, _mid, _highlight, _spinner = _activity_color_tokens() + return Style(color=Color.parse(base)) -# Terminal-native shimmer: a restrained silver sheen sweeping over the muted -# orange-yellow active verb. -_SHIMMER_BASE = "#D49E5A" # brand-exception: muted orange-yellow verb literal -_SHIMMER_MID = "#E2C18A" # brand-exception: light warm amber sheen-trail literal -_SHIMMER_HIGHLIGHT = "#D8DCE2" # brand-exception: silver sheen highlight literal +# Backwards-compatible dark-theme constants used by tests and older callers. +_SHIMMER_BASE = "#C8B176" +_SHIMMER_MID = "#E1CC94" +_SHIMMER_HIGHLIGHT = "#EEF2F7" _SHIMMER_INTERVAL_S = 0.22 -_SPINNER_SILVER_STYLE = Style(color=Color.parse("#C0C0C0")) # brand-exception: silver spinner +_SPINNER_SILVER = "#B8C0CC" def shimmer_spinner_style(elapsed_s: float, *, reduced_motion: bool = False) -> Style: """Clean shimmer color for active verb text. - Reduced motion pins to the base muted orange-yellow so the word stays calm. + Reduced motion pins to the base activity color so the word stays calm. """ if colors_disabled(): return Style() + base, mid, highlight, _spinner = _activity_color_tokens() if reduced_motion or reduced_motion_enabled(): - return Style(color=Color.parse(_SHIMMER_BASE)) - palette = (_SHIMMER_BASE, _SHIMMER_MID, _SHIMMER_HIGHLIGHT, _SHIMMER_MID) + return Style(color=Color.parse(base)) + palette = (base, mid, highlight, mid) idx = int(max(0.0, elapsed_s) / _SHIMMER_INTERVAL_S) % len(palette) return Style(color=Color.parse(palette[idx])) -def _wave_colors(chars: list[str], local_phase: int, *, rightward: bool) -> list[str | None]: +def _wave_colors( + chars: list[str], + local_phase: int, + *, + rightward: bool, + base: str, + mid: str, + highlight: str, +) -> list[str | None]: """Per-character colors for a single traveling-wave sweep. One bright highlight crosses the label with an asymmetric, slightly wider @@ -82,25 +107,27 @@ def _wave_colors(chars: list[str], local_phase: int, *, rightward: bool) -> list continue offset = i - head if offset == 0: - colors.append(_SHIMMER_HIGHLIGHT) + colors.append(highlight) elif offset in trail: - colors.append(_SHIMMER_MID) + colors.append(mid) else: - colors.append(_SHIMMER_BASE) + colors.append(base) return colors -def _splash_colors(chars: list[str], local_phase: int) -> list[str | None]: +def _splash_colors( + chars: list[str], local_phase: int, *, base: str, mid: str, highlight: str +) -> list[str | None]: """Per-character colors for the center-out splash bloom. A wavefront expands from the middle of the label toward both edges, leaving - a filled coral interior behind it, then settles the whole word to base amber. + a filled sheen behind it, then settles the whole word to the base activity color. """ n = len(chars) fill_frames = (n + 1) // 2 + 1 # frames for the wavefront to clear both edges center = (n - 1) / 2 if local_phase >= fill_frames: # settle beat before the next wave launches - return [None if char.isspace() else _SHIMMER_BASE for char in chars] + return [None if char.isspace() else base for char in chars] radius = local_phase colors: list[str | None] = [] for i, char in enumerate(chars): @@ -109,11 +136,11 @@ def _splash_colors(chars: list[str], local_phase: int) -> list[str | None]: continue dist = abs(i - center) if radius - 0.5 <= dist <= radius + 0.5: - colors.append(_SHIMMER_HIGHLIGHT) + colors.append(highlight) elif dist < radius - 0.5: - colors.append(_SHIMMER_MID) + colors.append(mid) else: - colors.append(_SHIMMER_BASE) + colors.append(base) return colors @@ -131,8 +158,9 @@ def _shimmer_segments( return [] if colors_disabled(): return [(None, label)] + base, mid, highlight, _spinner = _activity_color_tokens() if reduced_motion or reduced_motion_enabled(): - return [(_SHIMMER_BASE, label)] + return [(base, label)] chars = list(label) n = len(chars) @@ -141,13 +169,24 @@ def _shimmer_segments( cycle_len = 2 * wave_len + 2 * splash_len frame = int(max(0.0, elapsed_s) / _SHIMMER_INTERVAL_S) % cycle_len if frame < wave_len: - colors = _wave_colors(chars, frame, rightward=False) + colors = _wave_colors( + chars, frame, rightward=False, base=base, mid=mid, highlight=highlight + ) elif frame < wave_len + splash_len: - colors = _splash_colors(chars, frame - wave_len) + colors = _splash_colors(chars, frame - wave_len, base=base, mid=mid, highlight=highlight) elif frame < 2 * wave_len + splash_len: - colors = _wave_colors(chars, frame - wave_len - splash_len, rightward=True) + colors = _wave_colors( + chars, + frame - wave_len - splash_len, + rightward=True, + base=base, + mid=mid, + highlight=highlight, + ) else: - colors = _splash_colors(chars, frame - 2 * wave_len - splash_len) + colors = _splash_colors( + chars, frame - 2 * wave_len - splash_len, base=base, mid=mid, highlight=highlight + ) segments: list[tuple[str | None, str]] = [] for char, color in zip(chars, colors, strict=True): @@ -257,8 +296,12 @@ def activity_status_line(snapshot: ActivitySnapshot, *, width: int | None = None # Composing / Thinking: neutral muted grey, not the bright coral verb accent. glyph_style = thinking_style else: - # The dotted braille spinner is a marker; keep it silver while the verb shimmers. - glyph_style = Style() if colors_disabled() else _SPINNER_SILVER_STYLE + # The dotted braille spinner is a marker; keep it platinum while the verb shimmers. + if colors_disabled(): + glyph_style = Style() + else: + _base, _mid, _highlight, spinner = _activity_color_tokens() + glyph_style = Style(color=Color.parse(spinner)) if snapshot.label_style is not None: label_style = snapshot.label_style elif snapshot.spinner == "shape": diff --git a/src/pythinker_code/ui/shell/prompt.py b/src/pythinker_code/ui/shell/prompt.py index 92cf99dc..6c5faa9d 100644 --- a/src/pythinker_code/ui/shell/prompt.py +++ b/src/pythinker_code/ui/shell/prompt.py @@ -64,7 +64,7 @@ from pythinker_code.llm import ModelCapability from pythinker_code.share import get_share_dir from pythinker_code.soul import StatusSnapshot, format_context_status -from pythinker_code.thinking import model_uses_native_thinking +from pythinker_code.thinking import available_thinking_levels, model_uses_native_thinking from pythinker_code.tools.display import TodoDisplayItem from pythinker_code.ui.shell import placeholders as prompt_placeholders from pythinker_code.ui.shell.console import console @@ -2438,10 +2438,17 @@ def _thinking_prompt_prefix_style(self) -> str: def _uses_native_thinking(self) -> bool: return model_uses_native_thinking(getattr(self, "_model_capabilities", None)) + def _supports_thinking_effort(self) -> bool: + return available_thinking_levels(getattr(self, "_model_capabilities", None)) != ("off",) + def _prompt_separator_style(self, fallback: str) -> str: if getattr(self, "_mode", PromptMode.AGENT) != PromptMode.AGENT: return fallback - level = "high" if self._uses_native_thinking() else self._current_thinking_effort() + if not self._supports_thinking_effort(): + # Non-effort models use the standard input frame color (#3A506D in dark mode) + # instead of borrowing a thinking level color. + return "class:compact-input.frame" + level = self._current_thinking_effort() return thinking_frame_style(level) or fallback def _thinking_footer_label(self) -> str: @@ -2861,6 +2868,10 @@ def _render_background_todo_rows(self, columns: int) -> FormattedText: muted_style = f"fg:{tokens.muted}" if tokens.muted else "" warning_style = f"fg:{tokens.warning}" if tokens.warning else muted_style success_style = f"fg:{tokens.success}" if tokens.success else muted_style + activity_style = f"fg:{tokens.activity_verb}" if tokens.activity_verb else warning_style + active_title_style = ( + f"fg:{tokens.activity_label} bold" if tokens.activity_label else activity_style + ) text_style = ( f"fg:{tokens.text or tokens.activity_label}" if tokens.text or tokens.activity_label @@ -2881,8 +2892,8 @@ def _render_background_todo_rows(self, columns: int) -> FormattedText: title_style = muted_style elif todo.status == "in_progress": icon = "◼" - icon_style = warning_style - title_style = warning_style + icon_style = activity_style + title_style = active_title_style else: icon = "◻" icon_style = muted_style @@ -2932,7 +2943,7 @@ def _render_background_working_status( detail = f"{counts.bash} background bash task{'s' if counts.bash != 1 else ''}" tokens = _get_tui_tokens() muted_style = f"fg:{tokens.muted}" if tokens.muted else "" - frame_style = f"fg:{tokens.thinking_text}" if tokens.thinking_text else muted_style + frame_style = f"fg:{tokens.activity_spinner}" if tokens.activity_spinner else muted_style frame_text = f"{frame} " if show_verb: verb_text = spinner_message(now) diff --git a/src/pythinker_code/ui/shell/selectors/settings.py b/src/pythinker_code/ui/shell/selectors/settings.py index e0d486ab..754b075d 100644 --- a/src/pythinker_code/ui/shell/selectors/settings.py +++ b/src/pythinker_code/ui/shell/selectors/settings.py @@ -132,6 +132,13 @@ def _build_settings_config(config: Config) -> SettingsListConfig: current_value=_bool(config.default_yolo), values=_BOOL_VALUES, ), + SettingItem( + id="auto_deliberate_destructive_actions", + label="Destructive deliberation", + description="Bounce destructive auto-approved actions once for deliberation.", + current_value=_bool(config.auto_deliberate_destructive_actions), + values=_BOOL_VALUES, + ), SettingItem( id="default_plan_mode", label="Default plan mode", @@ -316,6 +323,11 @@ def mark(setting_id: str) -> None: if config.default_yolo != new: config.default_yolo = new mark(setting_id) + case "auto_deliberate_destructive_actions": + new = value == "true" + if config.auto_deliberate_destructive_actions != new: + config.auto_deliberate_destructive_actions = new + mark(setting_id) case "default_plan_mode": new = value == "true" if config.default_plan_mode != new: diff --git a/src/pythinker_code/ui/shell/tool_renderers/todo.py b/src/pythinker_code/ui/shell/tool_renderers/todo.py index a122569a..f8d2b3a8 100644 --- a/src/pythinker_code/ui/shell/tool_renderers/todo.py +++ b/src/pythinker_code/ui/shell/tool_renderers/todo.py @@ -48,7 +48,7 @@ def _icon_token(status: str) -> str: if status == "done": return "success" if status == "in_progress": - return "accent" + return "activity_verb" return "muted" @@ -68,7 +68,7 @@ def _status_title(status: str, title: str) -> Text: if status == "done": return fg("muted", title) if status == "in_progress": - out = fg("accent", title) + out = fg("activity_label", title) out.stylize("bold") return out return fg("tool_output", title) diff --git a/src/pythinker_code/ui/shell/visualize/_live_view.py b/src/pythinker_code/ui/shell/visualize/_live_view.py index f464a1d9..8ef010bc 100644 --- a/src/pythinker_code/ui/shell/visualize/_live_view.py +++ b/src/pythinker_code/ui/shell/visualize/_live_view.py @@ -597,6 +597,7 @@ def _working_indicator(self) -> RenderableType: label, elapsed_s=elapsed, width=width, + shimmer_label=active_todo_title is None, ) if todo_block is not None: return Group(line, todo_block) @@ -618,7 +619,9 @@ def _working_indicator(self) -> RenderableType: tip.append(current_tip(now), style=tui_rich_style("dim")) return Group(line, tip) - def _todo_activity_line(self, label: str, *, elapsed_s: float, width: int) -> Text: + def _todo_activity_line( + self, label: str, *, elapsed_s: float, width: int, shimmer_label: bool = True + ) -> Text: label = _todo_activity_label(label) parts = [format_elapsed(elapsed_s)] if self._latest_context_tokens: @@ -629,14 +632,17 @@ def _todo_activity_line(self, label: str, *, elapsed_s: float, width: int) -> Te label_width = max(1, width - cell_width(prefix) - cell_width(suffix)) label_text = truncate_to_width(label, label_width) - line = Text(prefix, style=tui_rich_style("thinking_text")) - line.append_text( - shimmer_text( - label_text, - elapsed_s, - reduced_motion=reduced_motion_enabled(), + line = Text(prefix, style=tui_rich_style("activity_spinner")) + if shimmer_label: + line.append_text( + shimmer_text( + label_text, + elapsed_s, + reduced_motion=reduced_motion_enabled(), + ) ) - ) + else: + line.append(label_text, style=tui_rich_style("activity_label") + Style(bold=True)) line.append(suffix, style=tui_rich_style("muted")) return line @@ -717,7 +723,7 @@ def _pinned_todo_row( title_style = tui_rich_style("muted") + Style(strike=True) elif todo.status == "in_progress": icon = "■" - icon_token = "warning" + icon_token = "activity_verb" title_style = tui_rich_style("activity_label") + Style(bold=True) else: icon = "□" diff --git a/src/pythinker_code/ui/shell/visualize/_worklog.py b/src/pythinker_code/ui/shell/visualize/_worklog.py index 75d0a910..f8ddb832 100644 --- a/src/pythinker_code/ui/shell/visualize/_worklog.py +++ b/src/pythinker_code/ui/shell/visualize/_worklog.py @@ -245,19 +245,26 @@ def render_display_blocks( idx += 1 continue if isinstance(block, TodoDisplayBlock): - lines: list[str] = [] - for todo in block.items: + body = Text() + for index, todo in enumerate(block.items): + if index: + body.append("\n") match todo.status: case "done": - marker = "✓" + body.append("✓", style=tui_rich_style("muted")) + body.append(" ") + body.append(todo.title, style=tui_rich_style("muted") + Style(strike=True)) case "in_progress": - marker = "→" + body.append("→", style=tui_rich_style("activity_verb")) + body.append(" ") + body.append( + todo.title, style=tui_rich_style("activity_label") + Style(bold=True) + ) case _: - marker = "·" - lines.append(f"{marker} {todo.title}") - rendered.append( - render_worklog_card("Todos", Text("\n".join(lines), style=tui_rich_style("muted"))) - ) + body.append("·", style=tui_rich_style("muted")) + body.append(" ") + body.append(todo.title, style=tui_rich_style("muted")) + rendered.append(render_worklog_card("Todos", body)) idx += 1 continue if isinstance(block, BackgroundTaskDisplayBlock): diff --git a/src/pythinker_code/ui/theme.py b/src/pythinker_code/ui/theme.py index b62a8bc6..ef3f88d0 100644 --- a/src/pythinker_code/ui/theme.py +++ b/src/pythinker_code/ui/theme.py @@ -441,6 +441,10 @@ class TuiTokens: text: str thinking_text: str activity_label: str + activity_verb: str + activity_verb_mid: str + activity_verb_highlight: str + activity_spinner: str # Backgrounds selected_bg: str user_message_bg: str @@ -480,6 +484,10 @@ class TuiTokens: text="", thinking_text="#C0C0C0", activity_label="#F4F4F5", + activity_verb="#C8B176", + activity_verb_mid="#E1CC94", + activity_verb_highlight="#EEF2F7", + activity_spinner="#B8C0CC", selected_bg="#243C54", user_message_bg="#1B2738", user_message_text="", @@ -513,6 +521,10 @@ class TuiTokens: text="#213853", thinking_text="#7A7A7A", activity_label="#213853", + activity_verb="#7A5C24", + activity_verb_mid="#8A6A2D", + activity_verb_highlight="#213853", + activity_spinner="#6B7280", selected_bg="#E6F2F6", user_message_bg="#F0E4E4", user_message_text="", @@ -569,31 +581,23 @@ def tui_rich_style(token: str, *, theme: ThemeName | None = None) -> RichStyle: # --------------------------------------------------------------------------- # Thinking-level prompt frame colors (Shift+Tab cycle). Keyed by the plain level -# string to avoid a theme<->selector import cycle; ThinkingLevel values are -# exactly these strings. +# string to avoid a theme<->selector import cycle. ``minimal`` is the canonical +# ThinkingLevel value; ``min`` is accepted as the compact palette step alias. # --------------------------------------------------------------------------- -_THINKING_FRAME_DARK: dict[str, str] = { - # Cool-to-warm ramp: quiet slate when disabled, then increasingly vivid - # thinking states as the model spends more effort. - "off": "#94A3B8", # slate - "minimal": "#60A5FA", # blue - "low": "#22D3EE", # cyan - "medium": "#34D399", # emerald - "high": "#FBBF24", # amber - "xhigh": "#FB7185", # rose - "max": "#F472B6", # pink +_THINKING_FRAME_SCALE: dict[str, str] = { + "off": "#64748b", # muted grey / slate-500 + "min": "#cbd5e1", # lighter grey / slate-300 + "minimal": "#cbd5e1", # canonical value for minimum + "low": "#3b82f6", # rich digital blue / blue-500 + "medium": "#22d3ee", # electric light cyan / cyan-400 + "high": "#c4b5fd", # whitish purple / violet-300 + "xhigh": "#a855f7", # vibrant purple / purple-500 + "max": "#6d28d9", # deep violet / violet-700 } -_THINKING_FRAME_LIGHT: dict[str, str] = { - "off": "#475569", # slate - "minimal": "#0369A1", # blue - "low": "#0E7490", # cyan - "medium": "#047857", # emerald - "high": "#92400E", # amber - "xhigh": "#9F1239", # rose - "max": "#9D174D", # pink -} +_THINKING_FRAME_DARK: dict[str, str] = _THINKING_FRAME_SCALE +_THINKING_FRAME_LIGHT: dict[str, str] = _THINKING_FRAME_SCALE def thinking_frame_color(level: str, *, theme: ThemeName | None = None) -> str: diff --git a/src/pythinker_code/utils/rich/markdown.py b/src/pythinker_code/utils/rich/markdown.py index 07ff9892..c342bdc3 100644 --- a/src/pythinker_code/utils/rich/markdown.py +++ b/src/pythinker_code/utils/rich/markdown.py @@ -5,7 +5,7 @@ import sys from collections.abc import Iterable, Mapping -from typing import ClassVar, get_args +from typing import ClassVar, cast, get_args from markdown_it import MarkdownIt from markdown_it.token import Token @@ -309,31 +309,23 @@ def on_child_close(self, context: MarkdownContext, child: MarkdownElement) -> bo return False def __rich_console__(self, console: Console, options: ConsoleOptions) -> RenderResult: - headers = ( - [column.content.plain.strip() for column in self.header.row.cells] - if self.header is not None and self.header.row is not None - else [] + table = Table( + box=box.SQUARE, + show_edge=True, + show_lines=True, + border_style="markdown.hr", + header_style="markdown.strong", + padding=(0, 1), + pad_edge=True, ) - rows = [row.cells for row in self.body.rows] if self.body is not None else [] - - if headers and rows and _table_should_render_as_records(headers, rows): - for row_index, row in enumerate(rows, start=1): - yield _record_title(row_index, row) - for header, cell in zip(headers[1:], row[1:], strict=False): - value = _cell_plain(cell.content) - if not value: - continue - line = Text(" ") - line.append(f"{header}: ", style="bold") - line.append_text(cell.content) - yield line - return - - table = Table(box=box.SIMPLE_HEAVY, show_edge=False) if self.header is not None and self.header.row is not None: for column in self.header.row.cells: - table.add_column(column.content) + table.add_column( + column.content, + justify=_table_cell_justify(column.justify), + overflow="fold", + ) if self.body is not None: for row in self.body.rows: @@ -343,19 +335,10 @@ def __rich_console__(self, console: Console, options: ConsoleOptions) -> RenderR yield table -def _cell_plain(cell: Text) -> str: - return cell.plain.strip() - - -def _table_should_render_as_records(headers: list[str], rows: list[list[TableDataElement]]) -> bool: - if len(headers) >= 4: - return True - return any(len(_cell_plain(cell.content)) > 48 for row in rows for cell in row) - - -def _record_title(row_index: int, row: list[TableDataElement]) -> Text: - title = _cell_plain(row[0].content) if row else "Row" - return Text(f"{row_index}. {title}", style="bold") +def _table_cell_justify(justify: str) -> JustifyMethod: + normalized = "left" if justify == "default" else justify + assert normalized in get_args(JustifyMethod) + return cast(JustifyMethod, normalized) class TableHeaderElement(MarkdownElement): diff --git a/tests/core/test_approval_auto.py b/tests/core/test_approval_auto.py index ad8a72a5..56d79136 100644 --- a/tests/core/test_approval_auto.py +++ b/tests/core/test_approval_auto.py @@ -2,9 +2,11 @@ from __future__ import annotations +import asyncio import json from pythinker_code.soul.approval import Approval, ApprovalState, deliberation_scope +from pythinker_code.tools.file import FileActions from pythinker_code.wire.types import ToolCall @@ -226,6 +228,56 @@ def test_deliberation_gate_conditions() -> None: assert benign.deliberation_gate(safe) is None +async def test_auto_safe_mode_denies_approval_without_waiting() -> None: + """Unattended safe-mode runs fail closed instead of waiting forever for approval.""" + from tests.conftest import tool_call_context + + approval = Approval(state=ApprovalState(auto=True, safe_mode=True)) + with tool_call_context("Shell", arguments={"command": "echo hello"}): + result = await asyncio.wait_for( + approval.request("Shell", "run command", "Run command `echo hello`"), + timeout=0.1, + ) + + assert not result + assert approval.runtime.list_pending() == [] + error = result.rejection_error() + assert "safe mode prevents auto-approval" in error.message + assert "rejected by the user" not in error.message + + +async def test_trusted_auto_denies_outside_workspace_write_without_yolo() -> None: + """Trusted auto mode still fails closed for outside-workspace file mutations.""" + from tests.conftest import tool_call_context + + approval = Approval(state=ApprovalState(auto=True, safe_mode=False)) + with tool_call_context("WriteFile", arguments={"path": "/tmp/out.txt", "content": "x"}): + result = await asyncio.wait_for( + approval.request("WriteFile", FileActions.EDIT_OUTSIDE, "Write file `/tmp/out.txt`"), + timeout=0.1, + ) + + assert not result + assert approval.runtime.list_pending() == [] + error = result.rejection_error() + assert "Outside-workspace file changes require explicit approval" in error.message + assert "rejected by the user" not in error.message + + +async def test_explicit_yolo_allows_outside_workspace_auto_write_boundary() -> None: + from tests.conftest import tool_call_context + + approval = Approval(state=ApprovalState(auto=True, yolo=True, safe_mode=False)) + with tool_call_context("WriteFile", arguments={"path": "/tmp/out.txt", "content": "x"}): + result = await approval.request( + "WriteFile", + FileActions.EDIT_OUTSIDE, + "Write file `/tmp/out.txt`", + ) + + assert result + + async def test_request_bounces_destructive_then_approves_retry() -> None: """End-to-end through request(): a destructive command in auto + auto_deliberate is bounced once with deliberation feedback that does NOT masquerade as a user rejection, diff --git a/tests/core/test_auto_injection.py b/tests/core/test_auto_injection.py index 7fb62813..738d92cc 100644 --- a/tests/core/test_auto_injection.py +++ b/tests/core/test_auto_injection.py @@ -8,6 +8,7 @@ _AUTO_INJECTION_TYPE, _AUTO_PROMPT, _AUTO_PROMPT_DELIBERATE, + _AUTO_PROMPT_DESTRUCTIVE_DELIBERATE, AutoModeInjectionProvider, ) @@ -19,6 +20,7 @@ def _mock_soul( is_subagent: bool = False, has_ask_user: bool = True, ask_user_question_policy: str = "ask_except_auto", + auto_deliberate_destructive_actions: bool = False, ) -> MagicMock: soul = MagicMock() soul.is_auto = is_auto @@ -27,6 +29,7 @@ def _mock_soul( soul.is_subagent = is_subagent soul.has_tool.return_value = has_ask_user soul.runtime.config.ask_user_question_policy = ask_user_question_policy + soul.runtime.config.auto_deliberate_destructive_actions = auto_deliberate_destructive_actions return soul @@ -38,6 +41,8 @@ async def test_injects_when_auto_enabled() -> None: assert result[0].content == _AUTO_PROMPT assert "auto" in result[0].content.lower() assert "Do NOT call AskUserQuestion" in result[0].content + assert "All tool calls are auto-approved" not in result[0].content + assert "fail" in result[0].content.lower() async def test_injects_deliberate_prompt_under_auto_deliberate_policy() -> None: @@ -49,10 +54,12 @@ async def test_injects_deliberate_prompt_under_auto_deliberate_policy() -> None: assert "advisor-assisted self-decision" in result[0].content -async def test_runtime_auto_does_not_inject_prompt() -> None: +async def test_runtime_auto_injects_non_persistent_prompt() -> None: provider = AutoModeInjectionProvider() result = await provider.get_injections([], _mock_soul(is_auto=True, is_auto_flag=False)) - assert result == [] + assert len(result) == 1 + assert result[0].type == _AUTO_INJECTION_TYPE + assert result[0].content == _AUTO_PROMPT async def test_no_injection_when_auto_disabled() -> None: @@ -61,6 +68,20 @@ async def test_no_injection_when_auto_disabled() -> None: assert result == [] +async def test_injects_destructive_deliberation_without_ask_user_deliberation() -> None: + provider = AutoModeInjectionProvider() + soul = _mock_soul( + is_auto=True, + ask_user_question_policy="never", + auto_deliberate_destructive_actions=True, + ) + result = await provider.get_injections([], soul) + assert len(result) == 1 + assert result[0].content == _AUTO_PROMPT_DESTRUCTIVE_DELIBERATE + assert "Do NOT call AskUserQuestion" in result[0].content + assert "Irreversible auto-approved actions" in result[0].content + + async def test_persistent_auto_injected_once_even_if_auto_stays_on() -> None: provider = AutoModeInjectionProvider() first = await provider.get_injections([], _mock_soul(is_auto=True)) @@ -69,12 +90,12 @@ async def test_persistent_auto_injected_once_even_if_auto_stays_on() -> None: assert second == [] -async def test_runtime_auto_does_not_rearm_prompt() -> None: +async def test_runtime_auto_injected_once_even_if_auto_stays_on() -> None: provider = AutoModeInjectionProvider() soul = _mock_soul(is_auto=True, is_auto_flag=False) first = await provider.get_injections([], soul) second = await provider.get_injections([], soul) - assert first == [] + assert len(first) == 1 assert second == [] diff --git a/tests/core/test_config.py b/tests/core/test_config.py index cc5f6705..eb7738ed 100644 --- a/tests/core/test_config.py +++ b/tests/core/test_config.py @@ -27,6 +27,7 @@ def test_default_config_dump(): "agent_execution_profile": "default", "default_yolo": False, "ask_user_question_policy": "ask_except_auto", + "auto_deliberate_destructive_actions": False, "default_plan_mode": False, "default_editor": "", "theme": "dark", @@ -115,6 +116,7 @@ def test_agent_execution_profile_autonomous_sets_autonomy_defaults(): assert config.default_yolo is True assert config.ask_user_question_policy == "never" + assert config.auto_deliberate_destructive_actions is True def test_agent_execution_profile_respects_explicit_values(): @@ -124,12 +126,14 @@ def test_agent_execution_profile_respects_explicit_values(): 'agent_execution_profile = "autonomous_coding"', "default_yolo = false", 'ask_user_question_policy = "always"', + "auto_deliberate_destructive_actions = false", ] ) ) assert config.default_yolo is False assert config.ask_user_question_policy == "always" + assert config.auto_deliberate_destructive_actions is False def test_agent_execution_profile_plan_only_sets_plan_defaults(): diff --git a/tests/core/test_runtime_auto_state.py b/tests/core/test_runtime_auto_state.py index fa378ea3..f6180f21 100644 --- a/tests/core/test_runtime_auto_state.py +++ b/tests/core/test_runtime_auto_state.py @@ -2,6 +2,8 @@ from __future__ import annotations +import asyncio +import json from unittest.mock import AsyncMock import pytest @@ -9,6 +11,14 @@ import pythinker_code.soul.agent as agent_module from pythinker_code.auth.oauth import OAuthManager from pythinker_code.soul.agent import Runtime +from pythinker_code.wire.types import ToolCall + + +def _shell_call(cmd: str) -> ToolCall: + return ToolCall( + id="call-1", + function=ToolCall.FunctionBody(name="Shell", arguments=json.dumps({"command": cmd})), + ) @pytest.fixture @@ -86,6 +96,60 @@ async def test_runtime_auto_overlay_does_not_persist_to_session_state( assert session.state.approval.auto is False +@pytest.mark.asyncio +@pytest.mark.parametrize(("auto", "runtime_auto"), [(True, False), (False, True)]) +@pytest.mark.asyncio +async def test_unattended_runtime_in_default_safe_mode_denies_without_waiting( + config, + session, + lightweight_runtime_create, + auto: bool, + runtime_auto: bool, +) -> None: + from tests.conftest import tool_call_context + + runtime = await Runtime.create( + config, + OAuthManager(config), + llm=None, + session=session, + yolo=False, + auto=auto, + runtime_auto=runtime_auto, + ) + + assert runtime.approval.is_auto() is True + assert runtime.approval.is_auto_approve() is False + with tool_call_context("Shell", arguments={"command": "echo hello"}): + result = await asyncio.wait_for( + runtime.approval.request("Shell", "run command", "Run command `echo hello`"), + timeout=0.1, + ) + + assert not result + assert runtime.approval.runtime.list_pending() == [] + assert "safe mode prevents auto-approval" in result.rejection_error().message + + +@pytest.mark.asyncio +async def test_runtime_create_enables_destructive_deliberation_from_config( + config, + session, + lightweight_runtime_create, +) -> None: + config.auto_deliberate_destructive_actions = True + + runtime = await Runtime.create( + config, + OAuthManager(config), + llm=None, + session=session, + yolo=True, + ) + + assert runtime.approval.deliberation_gate(_shell_call("rm -rf build")) is not None + + @pytest.mark.asyncio async def test_runtime_set_auto_persists_to_session_state( config, diff --git a/tests/test_session_recap.py b/tests/test_session_recap.py index 7c887f87..8a8292cf 100644 --- a/tests/test_session_recap.py +++ b/tests/test_session_recap.py @@ -87,6 +87,20 @@ def test_build_turn_recap_line_strips_report_blocks() -> None: assert line == ("※ recap: Deep scan completed. · 28 steps (disable recaps in /settings)") +def test_build_turn_recap_line_strips_markdown_tables_from_request() -> None: + line = build_turn_recap_line( + request=( + "| Step | Level | Hex |\n" + "| --- | --- | --- |\n" + "| off | Off | #475569 |\n" + "| high | High | #cc704b |\n" + ), + assistant_text="", + ) + + assert line is None + + def test_build_turn_recap_line_prefers_closing_summary_over_opening_intent() -> None: # The opening sentence is pure intent; the recap should surface the outcome. line = build_turn_recap_line( diff --git a/tests/ui/test_shell_markdown.py b/tests/ui/test_shell_markdown.py index 027dd451..a1bb85f7 100644 --- a/tests/ui/test_shell_markdown.py +++ b/tests/ui/test_shell_markdown.py @@ -138,7 +138,7 @@ def test_shell_markdown_keeps_emoji_icons_in_code() -> None: assert "● High" in output -def test_shell_markdown_keeps_rich_fork_table_records() -> None: +def test_shell_markdown_renders_multi_column_tables_as_bordered_grid() -> None: output = _render_text( PythinkerMarkdown( "| Area | Issue | Why it matters | Suggested improvement | Priority | Effort |\n" @@ -149,12 +149,11 @@ def test_shell_markdown_keeps_rich_fork_table_records() -> None: ) ) - assert "1. Accessibility" in output - assert "Issue:" in output - assert "Why it matters:" in output - assert "Suggested improvement:" in output - assert "Priority: High" in output - assert "Effort: XS" in output + assert "┌" in output and "┬" in output and "┘" in output + assert "Area" in output and "Accessibil" in output and "ity" in output + assert "Suggested" in output and "improvemen" in output + assert "Priority" in output and "High" in output + assert "Issue:" not in output def test_shell_markdown_repairs_report_heading_crammed_into_table_header() -> None: @@ -169,11 +168,12 @@ def test_shell_markdown_repairs_report_heading_crammed_into_table_header() -> No ) assert "● MEDIUM — address soon" in output - assert "1. M1" in output - assert "File: approval.py:208–228" in output - assert "CWE: CWE-285" in output - assert "Finding: No per-subagent approval isolation." in output - assert "Evidence: auto approve broadens scope" in output + assert "┌" in output and "┬" in output and "┘" in output + assert "M1" in output + assert "approval.py:208–228" in output + assert "CWE-285" in output + assert "No per-subagent" in output and "approval" in output and "isolation." in output + assert "auto approve" in output and "broadens scope" in output assert "| # | File" not in output diff --git a/tests/ui_and_conv/test_live_view_todos.py b/tests/ui_and_conv/test_live_view_todos.py index a7f59f7e..aaab76a6 100644 --- a/tests/ui_and_conv/test_live_view_todos.py +++ b/tests/ui_and_conv/test_live_view_todos.py @@ -15,7 +15,7 @@ _SHIMMER_MID, ) from pythinker_code.ui.shell.visualize import _LiveView -from pythinker_code.ui.theme import tui_rich_style +from pythinker_code.ui.theme import set_active_theme, tui_rich_style from pythinker_code.wire.types import StatusUpdate, TurnBegin _live_view_module = importlib.import_module("pythinker_code.ui.shell.visualize._live_view") @@ -172,17 +172,34 @@ def test_finished_todos_move_to_bottom_of_menu(monkeypatch) -> None: assert rendered.index("✓ Finished first") < rendered.index("✓ Finished second") -def test_active_todo_activity_line_uses_standard_spinner_shimmer() -> None: +def test_todo_activity_line_uses_standard_spinner_shimmer_for_generic_verbs() -> None: + set_active_theme("dark") view = _LiveView(StatusUpdate(context_tokens=10_000)) line = view._todo_activity_line("Implement pinned todos", elapsed_s=0.88, width=100) marker_style = Style.parse(line.style) if isinstance(line.style, str) else line.style - assert marker_style.color == tui_rich_style("thinking_text").color + assert marker_style.color == tui_rich_style("activity_spinner").color assert _span_colors_for(line, "Implement pinned todos") >= _SHIMMER_HEXES +def test_active_todo_activity_line_uses_stable_label_not_shimmer() -> None: + set_active_theme("dark") + view = _LiveView(StatusUpdate(context_tokens=10_000)) + + line = view._todo_activity_line( + "Implement pinned todos", elapsed_s=0.88, width=100, shimmer_label=False + ) + + active_color = _color_hex(tui_rich_style("activity_label").color) + marker_style = Style.parse(line.style) if isinstance(line.style, str) else line.style + assert marker_style.color == tui_rich_style("activity_spinner").color + assert _span_colors_for(line, "Implement pinned todos") == {active_color} + assert _span_colors_for(line, "Implement pinned todos").isdisjoint(_SHIMMER_HEXES) + + def test_active_pinned_todo_row_uses_neutral_title_not_shimmer() -> None: + set_active_theme("dark") view = _LiveView(StatusUpdate()) row = view._pinned_todo_row( @@ -194,7 +211,7 @@ def test_active_pinned_todo_row_uses_neutral_title_not_shimmer() -> None: active_color = _color_hex(tui_rich_style("activity_label").color) shimmer_colors = _SHIMMER_HEXES - assert _span_colors_for(row, "■") == {_color_hex(tui_rich_style("warning").color)} + assert _span_colors_for(row, "■") == {_color_hex(tui_rich_style("activity_verb").color)} assert _span_colors_for(row, "Implement pinned todos") == {active_color} assert _span_colors_for(row, "Implement pinned todos").isdisjoint(shimmer_colors) title_start = row.plain.index("Implement pinned todos") diff --git a/tests/ui_and_conv/test_prompt_tips.py b/tests/ui_and_conv/test_prompt_tips.py index e9f3eb2a..fdfc5f78 100644 --- a/tests/ui_and_conv/test_prompt_tips.py +++ b/tests/ui_and_conv/test_prompt_tips.py @@ -608,7 +608,9 @@ def get_size() -> Any: def test_card_toolbar_separator_matches_thinking_prompt_color(monkeypatch: Any) -> None: from pythinker_code.ui.theme import set_active_theme, thinking_frame_style - prompt_session = _make_toolbar_session(model_name="fast-model", tips=[]) + prompt_session = _make_toolbar_session( + model_name="fast-model", model_capabilities={"thinking"}, tips=[] + ) prompt_session._thinking = True prompt_session._thinking_effort = "xhigh" @@ -634,6 +636,36 @@ def get_size() -> Any: ) +def test_card_toolbar_separator_uses_standard_frame_for_non_thinking_models( + monkeypatch: Any, +) -> None: + prompt_session = _make_toolbar_session( + model_name="fast-model", model_capabilities=set(), tips=[] + ) + prompt_session._thinking = False + prompt_session._thinking_effort = "off" + + class _DummyOutput: + @staticmethod + def get_size() -> Any: + return SimpleNamespace(columns=120) + + monkeypatch.setenv("PYTHINKER_TUI_STYLE", "card") + monkeypatch.setattr( + shell_prompt, "get_app_or_none", lambda: SimpleNamespace(output=_DummyOutput()) + ) + monkeypatch.setattr(shell_prompt, "_get_git_branch", lambda: None) + monkeypatch.setattr(shell_prompt, "_shorten_cwd", lambda _: "~/proj") + monkeypatch.setattr("pythinker_code.extensions.footer_statuses", lambda: {}) + + fragments = list(prompt_session._render_bottom_toolbar()) + + assert fragments[0] == ( + "class:compact-input.frame", + shell_prompt._prompt_rule(120), + ) + + def test_bottom_toolbar_drops_agent_badge_before_bash_when_narrow(monkeypatch: Any) -> None: # With only ~width budget for one badge after CWD/mode, keeping bash and # dropping agent is the documented priority. diff --git a/tests/ui_and_conv/test_settings_selector.py b/tests/ui_and_conv/test_settings_selector.py index 69b6a927..3549c375 100644 --- a/tests/ui_and_conv/test_settings_selector.py +++ b/tests/ui_and_conv/test_settings_selector.py @@ -69,6 +69,7 @@ def test_build_settings_config_exposes_backed_settings_only(): assert "theme" in ids assert "tui.style" in ids assert "default_thinking" in ids + assert "auto_deliberate_destructive_actions" in ids assert "telemetry" in ids # No-op image controls are intentionally not exposed yet. assert "show-images" not in ids @@ -137,6 +138,7 @@ def test_apply_settings_changes_mutates_config(): "theme": "light", "default_thinking": "high", "telemetry": "true", + "auto_deliberate_destructive_actions": "true", "loop_control.max_retries_per_step": "5", "config_file": "/ignored/read-only", }, @@ -147,11 +149,13 @@ def test_apply_settings_changes_mutates_config(): assert changed == [ "theme", "default_thinking", + "auto_deliberate_destructive_actions", "loop_control.max_retries_per_step", ] assert config.theme == "light" assert config.default_thinking is True assert config.telemetry is True + assert config.auto_deliberate_destructive_actions is True assert config.loop_control.max_retries_per_step == 5 diff --git a/tests/ui_and_conv/test_shell_design_system.py b/tests/ui_and_conv/test_shell_design_system.py index 230904f7..9c69b817 100644 --- a/tests/ui_and_conv/test_shell_design_system.py +++ b/tests/ui_and_conv/test_shell_design_system.py @@ -87,7 +87,7 @@ def test_shell_style_resolves_brand_tokens_and_switches_theme(): set_active_theme("dark") -def test_verb_spinner_stays_muted_yellow_independent_of_accent_token(): +def test_verb_spinner_stays_champagne_independent_of_accent_token(): from pythinker_code.ui.shell.motion import _SHIMMER_BASE, verb_spinner_style from pythinker_code.ui.theme import set_active_theme diff --git a/tests/ui_and_conv/test_shell_motion.py b/tests/ui_and_conv/test_shell_motion.py index 41b6b844..0fed0499 100644 --- a/tests/ui_and_conv/test_shell_motion.py +++ b/tests/ui_and_conv/test_shell_motion.py @@ -130,8 +130,8 @@ def test_activity_status_line_uses_clean_metadata_separator(): assert "Pythinking… · 30s · ↓ 1.3k tokens" in output -def test_activity_status_line_uses_silver_spinner_and_muted_yellow_verb(): - from pythinker_code.ui.theme import set_active_theme +def test_activity_status_line_uses_platinum_spinner_and_champagne_verb(): + from pythinker_code.ui.theme import set_active_theme, tui_rich_style set_active_theme("dark") start = activity_status_line(ActivitySnapshot(label="Cultivating", elapsed_s=0.0)) @@ -139,7 +139,7 @@ def test_activity_status_line_uses_silver_spinner_and_muted_yellow_verb(): later_sheen = activity_status_line(ActivitySnapshot(label="Cultivating", elapsed_s=1.10)) base_style = Style.parse(start.style) if isinstance(start.style, str) else start.style - assert _color_hex(base_style.color) == "#c0c0c0" + assert base_style.color == tui_rich_style("activity_spinner").color assert _span_colors_for(sheen, "Cultivating") >= _SHIMMER_HEXES assert _span_colors_for(later_sheen, "Cultivating") >= _SHIMMER_HEXES assert "Cultivating…" in _plain(start) diff --git a/tests/ui_and_conv/test_shell_motion_shimmer.py b/tests/ui_and_conv/test_shell_motion_shimmer.py index 1185ac4f..fbbb8bed 100644 --- a/tests/ui_and_conv/test_shell_motion_shimmer.py +++ b/tests/ui_and_conv/test_shell_motion_shimmer.py @@ -45,6 +45,7 @@ def test_shimmer_varies_over_time_when_motion_enabled(monkeypatch): def test_prompt_shimmer_fragments_share_silver_sheen_palette(monkeypatch): monkeypatch.delenv("PYTHINKER_REDUCED_MOTION", raising=False) + set_active_theme("dark") fragments = shimmer_prompt_fragments("Schlepping…", 0.88) styles = {style.lower() for style, text in fragments if text.strip()} @@ -55,8 +56,25 @@ def test_prompt_shimmer_fragments_share_silver_sheen_palette(monkeypatch): assert "".join(text for _style, text in fragments) == "Schlepping…" +def test_shimmer_fragments_use_light_theme_activity_tokens(monkeypatch): + from pythinker_code.ui.theme import get_tui_tokens + + monkeypatch.delenv("PYTHINKER_REDUCED_MOTION", raising=False) + set_active_theme("light") + + fragments = shimmer_prompt_fragments("Schlepping…", 0.88) + styles = {style.lower() for style, text in fragments if text.strip()} + tokens = get_tui_tokens("light") + + assert f"fg:{tokens.activity_verb.lower()}" in styles + assert f"fg:{tokens.activity_verb_mid.lower()}" in styles + assert f"fg:{tokens.activity_verb_highlight.lower()}" in styles + assert "".join(text for _style, text in fragments) == "Schlepping…" + + def test_splash_originates_at_center_and_widens(monkeypatch): monkeypatch.delenv("PYTHINKER_REDUCED_MOTION", raising=False) + set_active_theme("dark") label = "abcdefg" # n=7, center index 3, no spaces wave_len = len(label) + 6 # phase B (first splash) starts here @@ -72,6 +90,7 @@ def test_splash_originates_at_center_and_widens(monkeypatch): def test_phase_c_trail_mirrors_phase_a(monkeypatch): monkeypatch.delenv("PYTHINKER_REDUCED_MOTION", raising=False) + set_active_theme("dark") label = "abcdefg" n = len(label) wave_len = n + 6 @@ -94,6 +113,7 @@ def test_phase_c_trail_mirrors_phase_a(monkeypatch): def test_cycle_returns_to_start(monkeypatch): monkeypatch.delenv("PYTHINKER_REDUCED_MOTION", raising=False) + set_active_theme("dark") label = "Reticulating" n = len(label) cycle_len = 2 * (n + 6) + 2 * ((n + 1) // 2 + 3) diff --git a/tests/ui_and_conv/test_shell_prompt_echo.py b/tests/ui_and_conv/test_shell_prompt_echo.py index ce86661b..2144d9e7 100644 --- a/tests/ui_and_conv/test_shell_prompt_echo.py +++ b/tests/ui_and_conv/test_shell_prompt_echo.py @@ -167,6 +167,21 @@ def test_user_echo_wraps_continuation_under_text_start() -> None: assert not lines[2].startswith("❯") +def test_user_echo_renders_pasted_markdown_tables() -> None: + rendered = render_user_echo_text( + "| Step | Level | Hex |\n" + "| --- | --- | --- |\n" + "| off | Off | #475569 |\n" + "| high | High | #cc704b |\n" + ) + plain = render_plain(rendered, width=72) + + assert plain.startswith("\n❯ ┌") + assert "│ Step" in plain + assert "#475569" in plain + assert "| --- |" not in plain + + def test_should_echo_agent_input_for_plain_agent_message() -> None: shell = _make_shell() assert shell._should_echo_agent_input(_make_user_input("hi")) is True diff --git a/tests/ui_and_conv/test_thinking_cycle.py b/tests/ui_and_conv/test_thinking_cycle.py index 71da1350..ca4eb562 100644 --- a/tests/ui_and_conv/test_thinking_cycle.py +++ b/tests/ui_and_conv/test_thinking_cycle.py @@ -35,19 +35,21 @@ def test_next_thinking_level_cycles_and_wraps( def test_thinking_frame_color_maps_each_level_dark() -> None: from pythinker_code.ui.theme import thinking_frame_color - assert thinking_frame_color("off", theme="dark") == "#94A3B8" # slate - assert thinking_frame_color("minimal", theme="dark") == "#60A5FA" # blue - assert thinking_frame_color("low", theme="dark") == "#22D3EE" # cyan - assert thinking_frame_color("medium", theme="dark") == "#34D399" # emerald - assert thinking_frame_color("high", theme="dark") == "#FBBF24" # amber - assert thinking_frame_color("xhigh", theme="dark") == "#FB7185" # rose + assert thinking_frame_color("off", theme="dark") == "#64748b" # slate-500 + assert thinking_frame_color("min", theme="dark") == "#cbd5e1" # slate-300 alias + assert thinking_frame_color("minimal", theme="dark") == "#cbd5e1" # slate-300 canonical + assert thinking_frame_color("low", theme="dark") == "#3b82f6" # blue-500 + assert thinking_frame_color("medium", theme="dark") == "#22d3ee" # cyan-400 + assert thinking_frame_color("high", theme="dark") == "#c4b5fd" # violet-300 + assert thinking_frame_color("xhigh", theme="dark") == "#a855f7" # purple-500 + assert thinking_frame_color("max", theme="dark") == "#6d28d9" # violet-700 -def test_thinking_frame_color_light_differs_from_dark() -> None: +def test_thinking_frame_color_light_uses_same_standard_scale() -> None: from pythinker_code.ui.theme import thinking_frame_color - assert thinking_frame_color("high", theme="light") == "#92400E" - assert thinking_frame_color("high", theme="light") != thinking_frame_color("high", theme="dark") + assert thinking_frame_color("high", theme="light") == "#c4b5fd" + assert thinking_frame_color("high", theme="light") == thinking_frame_color("high", theme="dark") def test_thinking_frame_color_unknown_level_falls_back_to_border() -> None: @@ -59,7 +61,7 @@ def test_thinking_frame_color_unknown_level_falls_back_to_border() -> None: def test_thinking_frame_style_is_ptk_fg_directive() -> None: from pythinker_code.ui.theme import thinking_frame_style - assert thinking_frame_style("high", theme="dark") == "fg:#FBBF24" + assert thinking_frame_style("high", theme="dark") == "fg:#c4b5fd" def test_core_thinking_cycle_uses_available_model_levels() -> None: diff --git a/tests/ui_and_conv/test_tui_theme_tokens.py b/tests/ui_and_conv/test_tui_theme_tokens.py index 50566cf0..c118a1ec 100644 --- a/tests/ui_and_conv/test_tui_theme_tokens.py +++ b/tests/ui_and_conv/test_tui_theme_tokens.py @@ -39,6 +39,10 @@ def test_dark_tokens_have_brand_values(): assert t.error == "#EF5E62" assert t.thinking_text == "#C0C0C0" # lighter neutral grey, not purple-tinted muted assert t.thinking_text != t.muted + assert t.activity_verb == "#C8B176" # champagne activity verb + assert t.activity_verb_mid == "#E1CC94" + assert t.activity_verb_highlight == "#EEF2F7" + assert t.activity_spinner == "#B8C0CC" assert t.tool_title == t.activity_label assert t.tool_pending_bg == "#1B2230" assert t.tool_error_bg == "#2E1D24" @@ -53,6 +57,10 @@ def test_light_tokens_have_brand_values(): assert t.error == "#C0392B" assert t.thinking_text == "#7A7A7A" # lighter neutral grey, not blue/purple muted assert t.thinking_text != t.muted + assert t.activity_verb == "#7A5C24" # contrast-safe bronze activity verb + assert t.activity_verb_mid == "#8A6A2D" + assert t.activity_verb_highlight == "#213853" + assert t.activity_spinner == "#6B7280" assert t.tool_title == t.activity_label assert t.tool_pending_bg == "#EFE7E8" @@ -148,6 +156,13 @@ def test_code_block_bg_in_token_names(): assert "code_block_bg" in TUI_TOKEN_NAMES +def test_activity_tokens_in_token_names(): + assert "activity_verb" in TUI_TOKEN_NAMES + assert "activity_verb_mid" in TUI_TOKEN_NAMES + assert "activity_verb_highlight" in TUI_TOKEN_NAMES + assert "activity_spinner" in TUI_TOKEN_NAMES + + def test_code_block_bg_dark_value(): assert get_tui_tokens("dark").code_block_bg == "#1f2030" diff --git a/tests/utils/test_rich_markdown.py b/tests/utils/test_rich_markdown.py index 77ae7fb1..e8bf9b16 100644 --- a/tests/utils/test_rich_markdown.py +++ b/tests/utils/test_rich_markdown.py @@ -11,7 +11,7 @@ def test_markdown_html_block_renders_without_stack_error() -> None: assert "" in rendered -def test_wide_markdown_table_renders_as_readable_records() -> None: +def test_wide_markdown_table_renders_as_bordered_grid() -> None: console = Console(width=72, record=True, color_system=None) markdown = Markdown( "| Area | Issue | Why it matters | Suggested improvement | Priority | Effort |\n" @@ -25,12 +25,12 @@ def test_wide_markdown_table_renders_as_readable_records() -> None: console.print(markdown) output = console.export_text() - assert "1. Accessibility" in output - assert "Issue:" in output - assert "Why it matters:" in output - assert "Suggested improvement:" in output - assert "Priority: High" in output - assert "Effort: XS" in output + assert "┌" in output and "┬" in output and "┘" in output + assert "Area" in output + assert "Accessibil" in output and "ity" in output + assert "Why it" in output and "matters" in output + assert "Priority" in output and "High" in output + assert "Issue:" not in output def test_fenced_code_block_renders_as_labeled_report_panel() -> None: From f0bd6b53a53367aa5a8d14a5b017340ceebd70a9 Mon Sep 17 00:00:00 2001 From: mohamed-elkholy95 Date: Tue, 2 Jun 2026 14:15:24 -0400 Subject: [PATCH 5/9] feat(review-ui): enhance reviewflow and TUI rendering --- packages/pythinker-review/README.md | 6 +- .../pythinker-review/docs/blackbox-parity.md | 4 +- .../src/pythinker_review/cli/review.py | 27 + .../pythinker_review/reviewflow/provider.py | 399 ++++- .../pythinker_review/reviewflow/workflow.py | 189 ++- .../security_scan/knowledge.py | 1348 +++++++++++++++++ .../security_scan/processor.py | 61 +- .../pythinker_review/security_scan/prompt.py | 178 +-- .../pythinker_review/security_scan/tech.py | 2 + .../src/pythinker_review/signals/advisor.py | 77 +- .../tests/e2e/test_reviewflow_workflow.py | 65 + .../tests/unit/test_reviewflow.py | 33 + .../tests/unit/test_security_scan.py | 68 + .../tests/unit/test_signals.py | 15 + .../agents/default/code_reviewer.yaml | 8 +- .../agents/default/security_reviewer.yaml | 9 +- src/pythinker_code/session_recap.py | 87 +- src/pythinker_code/tools/__init__.py | 4 + src/pythinker_code/tools/agent/description.md | 2 + src/pythinker_code/ui/shell/slash.py | 32 +- .../ui/shell/tool_renderers/__init__.py | 2 + .../ui/shell/tool_renderers/_render_utils.py | 12 +- .../ui/shell/tool_renderers/agent.py | 2 +- .../ui/shell/tool_renderers/skill.py | 84 + .../ui/shell/visualize/_live_view.py | 4 +- .../ui/shell/visualize/_worklog.py | 1 + tests/e2e/test_shell_modal_e2e.py | 4 +- tests/e2e/test_shell_pty_e2e.py | 6 +- tests/test_session_recap.py | 42 +- tests/ui_and_conv/README_contract_registry.md | 11 +- .../test_live_view_notifications.py | 8 +- tests/ui_and_conv/test_md_table_contract.py | 28 +- .../ui_and_conv/test_shell_slash_commands.py | 59 +- .../test_tui_blocks_integration.py | 10 +- .../test_tui_card_tool_renderers.py | 98 +- .../test_visualize_running_prompt.py | 9 +- 36 files changed, 2562 insertions(+), 432 deletions(-) create mode 100644 packages/pythinker-review/src/pythinker_review/security_scan/knowledge.py create mode 100644 src/pythinker_code/ui/shell/tool_renderers/skill.py diff --git a/packages/pythinker-review/README.md b/packages/pythinker-review/README.md index 29df516a..dac48d90 100644 --- a/packages/pythinker-review/README.md +++ b/packages/pythinker-review/README.md @@ -25,6 +25,7 @@ pythinker-review show-finding pythinker-review init pythinker-review map pythinker-review review --limit 3 --jobs 3 +pythinker-review review --limit 3 --jobs 3 --prompt-file review-guidance.md --rate-limit-per-minute 6 # optional guidance/rate cap pythinker-review report --status open pythinker-review show --finding pythinker-review triage --finding --status false-positive @@ -100,8 +101,9 @@ The stateful Reviewflow workflow writes `.pythinker-review-flow/` by default: Phase 1 now ports the highest-value behavior from the mounted blackbox repos: -- Reviewflow-style evidence validation rejects findings outside the reviewed chunk/feature, unsafe - paths, stale line ranges, or non-matching evidence snippets. +- Reviewflow-style evidence validation uses line-numbered prompt manifests and rejects findings + outside the reviewed chunk/feature, unsafe paths, omitted/truncated line ranges, or non-matching + evidence snippets; invalid sibling findings are recorded as drops without failing the whole run. - Reviewflow pure-Python stateful commands cover `init`, `map`, `status`, `review`, `ci`, `report`, `show --finding`, `next`, `triage`, `revalidate`, `fix`, `open-pr`, `doctor`, and `clean-locks`. diff --git a/packages/pythinker-review/docs/blackbox-parity.md b/packages/pythinker-review/docs/blackbox-parity.md index 0ed2ff87..79604bce 100644 --- a/packages/pythinker-review/docs/blackbox-parity.md +++ b/packages/pythinker-review/docs/blackbox-parity.md @@ -10,7 +10,7 @@ compatibility. | --- | --- | --- | --- | --- | | `blackbox/clawpatch-main/README.md`, `docs/index.md`, `docs/spec.md` | Review is evidence-first; state is durable; fix/PR flows are explicit follow-ups. | `reviewflow/workflow.py`, `reviewflow/state.py`, `packages/pythinker-review/src/pythinker_review/engine/orchestrator.py`, `store/` | Store round-trip, legacy state migration, stateful init/map/review/report/triage/fix e2e, list/show, fail-closed runner tests. | Diff review still persists `.pythinker-review/`; the stateful Reviewflow workflow uses `.pythinker-review-flow/` by default and non-destructively imports legacy state when needed. | | `blackbox/clawpatch-main/src/prompt.ts` review/fix/revalidate prompts | Bounded context, strict JSON, evidence/reasoning/test-analysis/minimum-fix-scope concepts, plus explicit unified-diff fix plans. | `reviewers/prompts/code_review.system.md`, `reviewers/prompts/debug_review.system.md`, `reviewers/prompts/deslopify_review.system.md`, `reviewers/schema.py`, `store/models.py`, `reviewflow/provider.py` | Reviewer prompt/caller tests, schema round-trip tests, malformed-output retry tests, fix unified-diff e2e. | Stateful feature review uses compact pure-Python prompts rather than a literal TypeScript prompt copy. | -| `blackbox/clawpatch-main/src/review-validation.ts` | Reject stale/out-of-context evidence; never silently persist hallucinated findings. | `reviewers/validation.py`, `engine/runner.py`, `reviewers/schema.py` | Validation tests for escaping paths, out-of-chunk files, out-of-hunk line ranges, and evidence snippets. | Validation is chunk-scoped in Phase 1; full semantic feature-context validation is deferred to whole-repo audit mode. | +| `blackbox/clawpatch-main/src/review-validation.ts` | Reject stale/out-of-context evidence; never silently persist hallucinated findings. Stateful review records schema/evidence drops without failing valid sibling findings. | `reviewers/validation.py`, `engine/runner.py`, `reviewers/schema.py`, `reviewflow/provider.py`, `reviewflow/workflow.py` | Validation tests for escaping paths, out-of-chunk files, out-of-hunk line ranges, evidence snippets, prompt manifests, and non-fatal stateful validation drops. | Diff validation is chunk-scoped; stateful feature review is prompt-manifest-scoped with line-numbered excerpts. Full semantic feature-context validation beyond included excerpts remains deferred. | | `blackbox/clawpatch-main/src/app.ts` | Bounded worker pool, retry malformed model output once, run metadata, partial failure visibility, workflow commands. | `engine/runner.py`, `store/models.py`, `store/findings_store.py`, `reviewflow/workflow.py`, `cli/review.py` | Runner fail-closed/allow-partial tests; store atomicity tests; stateful workflow e2e. | Stateful feature review is intentionally conservative and pure Python; agent enrichment is not yet implemented. | | `blackbox/clawpatch-main/src/types.ts`, `src/mapper.ts`, `src/mappers/task-graph.ts` | Durable project/feature/run/finding/patch records and heuristic feature/task mapping. | `reviewflow/models.py`, `reviewflow/mapping.py`, `reviewflow/provider.py`, `reviewflow/state.py` | Pydantic schema import/type checks, mapper partition/script/state/report unit tests, package lint/typecheck. | Mapper coverage includes source partitioning, nearby-test association, Python console scripts, and broad file-pattern grouping; framework-specific mapper details are compacted rather than byte-identical. | | `blackbox/clawpatch-main/src/selection.ts`, `src/git.ts` | Git-scoped selection (`since`/dirty/range), changed-file focus, path-relative behavior. | `engine/diff_source.py`, `engine/chunker.py`, `reviewflow/workflow.py`, `reviewflow/utils.py` | Git fixture tests for base/staged/working-tree/range and glob filters; stateful changed-file selectors are covered through workflow tests. | Diff review remains hunk-scoped; stateful review is feature-scoped. | @@ -41,6 +41,6 @@ compatibility. | `packages/processor/src/agents/shared.ts` JSON parsing | Malformed/non-array model output is a batch error, not "no findings". | `reviewers/security_review.py`, `engine/runner.py` | Security reviewer retries once then records `malformed_output`; fail-closed runner tests. | Pythinker schema is `{"findings": [...]}` instead of the source scanner's array payload. | | Security prompt core | Static-analysis mindset, trace inputs/imports/mitigations, report only validated exploitable issues. | `reviewers/prompts/security_review.system.md` | Prompt/caller tests and signal scanner tests. | Severity taxonomy maps to Pythinker `critical/high/medium/low/info`. | | Scanner rule metadata/matchers | Deterministic signals are prompt anchors, not findings, and carry rule metadata/reasons/confidence/CWE/severity hints. | `signals/models.py`, `signals/scanner.py` | Secret, shell/RCE, SQL, NoSQL, deserialization, SSRF, path traversal, XSS, redirect, JWT, CORS, debug, prompt-injection, weak-crypto rule tests. | Curated in-process rules replace the source plugin marketplace for Phase 1. | -| Prompt assembly with tech highlights/slug notes/project context | Batch-scoped anchors avoid prompt bloat while preserving signal context. | `signals/tech.py`, `signals/advisor.py`, `reviewers/security_review.py`, `engine/orchestrator.py` | Advisor context, tech detection, and security reviewer prompt tests. | INFO.md/config prompt append remain deferred; built-in tech highlights and slug notes are implemented. | +| Prompt assembly with tech highlights/slug notes/project context | Batch-scoped anchors avoid prompt bloat while preserving signal context. | `security_scan/knowledge.py`, `security_scan/tech.py`, `signals/advisor.py`, `reviewers/security_review.py`, `engine/orchestrator.py` | Advisor context, tech detection, and security reviewer prompt tests. | INFO.md/config prompt append remain deferred for diff review; shared built-in tech highlights and slug notes are implemented. | | Revalidation/verdict workflow | Findings should be validated before being treated as final. | `reviewers/validation.py`, `engine/runner.py`; future read-only `revalidate.py` | Validation and fail-closed runner tests. | Separate saved-finding revalidation is deferred; initial model output is evidence-validated now. | | Export/report/metrics | Machine-readable output and CI gating only on net findings. | `output/json.py`, `output/sarif.py`, `cli/_shared.py` | JSON/SARIF schema tests and threshold exit-code tests. | Markdown PR comments and metrics dashboards are deferred. | diff --git a/packages/pythinker-review/src/pythinker_review/cli/review.py b/packages/pythinker-review/src/pythinker_review/cli/review.py index a4bc1b72..ce6e905f 100644 --- a/packages/pythinker-review/src/pythinker_review/cli/review.py +++ b/packages/pythinker-review/src/pythinker_review/cli/review.py @@ -127,6 +127,17 @@ def _resolve_llm() -> ReviewLLM: raise typer.Exit(code=3) +def _read_text_option(path: Path | None) -> str | None: + if path is None: + return None + try: + return path.read_text(encoding="utf-8") + except OSError as exc: + raise ReviewflowWorkflowError( + f"failed to read --prompt-file {path}: {exc}", "invalid-usage" + ) from exc + + def _emit(fmt: OutputFormat, *, meta: RunMeta, findings: list[Finding], no_color: bool) -> str: if fmt is OutputFormat.json: return render_json(meta, findings) @@ -412,6 +423,20 @@ def review_stateful( dry_run: bool = typer.Option(False, "--dry-run"), include_dirty: bool = typer.Option(False, "--include-dirty"), timeout_s: float = typer.Option(180.0, "--timeout-s", min=1.0), + prompt_file: Path | None = typer.Option( + None, + "--prompt-file", + exists=True, + dir_okay=False, + readable=True, + help="Additional reviewer guidance for stateful feature review.", + ), + rate_limit_per_minute: int | None = typer.Option( + None, + "--rate-limit-per-minute", + min=1, + help="Maximum provider review starts per rolling minute.", + ), repo: Path = typer.Option(Path.cwd(), "--repo", "--root"), state_dir: str = typer.Option(".pythinker-review-flow", "--state-dir"), config: Path | None = typer.Option(None, "--config"), @@ -434,6 +459,8 @@ def review_stateful( mode=mode.value, dry_run=dry_run, per_feature_timeout_s=timeout_s, + custom_prompt=_read_text_option(prompt_file), + rate_limit_per_minute=rate_limit_per_minute, ) ) except ReviewflowWorkflowError as exc: diff --git a/packages/pythinker-review/src/pythinker_review/reviewflow/provider.py b/packages/pythinker-review/src/pythinker_review/reviewflow/provider.py index 3a6b141e..012f8907 100644 --- a/packages/pythinker-review/src/pythinker_review/reviewflow/provider.py +++ b/packages/pythinker-review/src/pythinker_review/reviewflow/provider.py @@ -3,12 +3,17 @@ from __future__ import annotations import json +from dataclasses import dataclass from pathlib import Path +from typing import Any, Literal + +from pydantic import BaseModel, Field, ValidationError from pythinker_review.llm.protocol import ReviewLLM from pythinker_review.reviewers.common import complete_typed_json from pythinker_review.reviewflow.models import ( FeatureRecord, + FeatureReviewFinding, FeatureReviewOutput, FindingRecord, FixPlanOutput, @@ -17,6 +22,8 @@ ) from pythinker_review.reviewflow.utils import read_text_bounded +REVIEW_PROMPT_FILE_CHAR_LIMIT = 24_000 + REVIEW_SYSTEM = ( "You are Pythinker Review running a pure-Python Reviewflow review.\n" "Return strict JSON only. Review is read-only. Report only concrete, actionable findings " @@ -34,29 +41,135 @@ Do not include markdown fences. Keep the diff minimal and scoped to the finding. """ +ReviewPromptFileRole = Literal["owned", "context", "test"] +ReviewDropLayer = Literal["schema", "validation"] + + +@dataclass(frozen=True, slots=True) +class ReviewPromptLineRange: + start_line: int + end_line: int + + +@dataclass(frozen=True, slots=True) +class ReviewPromptFileManifest: + path: str + role: ReviewPromptFileRole + reason: str + bytes: int + included_bytes: int + included_line_ranges: tuple[ReviewPromptLineRange, ...] + truncated: bool + readable: bool + skipped_reason: str | None + included_text: str = "" + + +@dataclass(frozen=True, slots=True) +class ReviewPromptManifest: + max_owned_files: int + max_context_files: int + included_files: tuple[ReviewPromptFileManifest, ...] + omitted_files: tuple[dict[str, str], ...] + prompt_bytes: int + approximate_tokens: int + + +@dataclass(frozen=True, slots=True) +class ReviewPromptBundle: + prompt: str + manifest: ReviewPromptManifest + + +@dataclass(frozen=True, slots=True) +class ReviewDrop: + path: tuple[str | int, ...] + message: str + layer: ReviewDropLayer + + +@dataclass(frozen=True, slots=True) +class PartitionedFeatureReviewResult: + output: FeatureReviewOutput + manifest: ReviewPromptManifest + dropped_findings: tuple[ReviewDrop, ...] + + +class _LooseFeatureReviewOutput(BaseModel): + findings: list[Any] = Field(default_factory=list) + def feature_review_user_prompt( - *, root: Path, feature: FeatureRecord, config: ReviewflowConfig, mode: str + *, + root: Path, + feature: FeatureRecord, + config: ReviewflowConfig, + mode: str, + custom_prompt: str | None = None, ) -> str: - files = ( - feature.owned_files[: config.review.max_owned_files] - + feature.context_files[: config.review.max_context_files] - ) + return build_feature_review_prompt_bundle( + root=root, + feature=feature, + config=config, + mode=mode, + custom_prompt=custom_prompt, + ).prompt + + +def build_feature_review_prompt_bundle( + *, + root: Path, + feature: FeatureRecord, + config: ReviewflowConfig, + mode: str, + custom_prompt: str | None = None, +) -> ReviewPromptBundle: + prompt_files = _collect_prompt_files(feature, config) + included_files: list[ReviewPromptFileManifest] = [] file_blocks: list[str] = [] - for ref in files: - path = root / ref.path - file_blocks.append( - f"## {ref.path}\nReason: {ref.reason}\n```\n{read_text_bounded(path)}\n```" - ) + for path, role, reason in prompt_files: + prompt_file = _prompt_file(root=root, path=path, role=role, reason=reason) + included_files.append(prompt_file.manifest) + file_blocks.append(prompt_file.block) + omitted_files = _omitted_prompt_files(feature, config, {path for path, _, _ in prompt_files}) + valid_evidence_paths = [file.path for file in included_files if file.readable] + custom_block = _custom_prompt_block(custom_prompt) + prompt_context = _manifest_prompt_context( + max_owned_files=config.review.max_owned_files, + max_context_files=config.review.max_context_files, + included_files=included_files, + omitted_files=omitted_files, + ) tests = "\n".join(f"- {test.path} ({test.command or 'no command'})" for test in feature.tests) - return f""" + prompt = f""" Review mode: {mode} Feature JSON: {feature.model_dump_json(by_alias=True, indent=2)} -Relevant tests: +{custom_block}Relevant tests: {tests or "- none detected"} +Review guidance: +- Inspect owned files, context files, and linked tests as one feature slice. +- Treat tests as evidence of intended behavior. If tests contradict a suspected bug, skip it or + downgrade confidence and explain the uncertainty. +- Avoid speculative low-evidence findings. Prefer an empty findings array over a weak guess. +- Deduplicate sibling/root-cause issues: report one finding with multiple evidence refs. +- Evidence paths must be exactly one of the valid paths below. +- When citing line ranges, use the gutter numbers in the Files section. +- Do not cite files or line ranges outside the shown excerpts. If an excerpt is truncated, only cite + lines that appear in the Files section. +- Provide whyTestsDoNotAlreadyCoverThis, suggestedRegressionTest, and minimumFixScope when useful. +- For shell/YAML/subprocess/Markdown command recipes, treat parsed command output as process-exec + code; flag mixed command-capture/fallback output that can concatenate machine-readable values. +{_review_mode_guidance(mode)} + +Valid evidence paths: +{chr(10).join(f"- {path}" for path in valid_evidence_paths) or "- none"} + +Prompt context: +{json.dumps(prompt_context, indent=2)} + Files: {chr(10).join(file_blocks) or "No readable files."} @@ -73,9 +186,8 @@ def feature_review_user_prompt( "startLine": 1, "endLine": 1, "symbol": null, - "quote": "exact snippet" + "quote": "exact snippet or null" }}], - "reasoning": "why this is a real issue", "reproduction": "optional concrete trigger or null", "recommendation": "minimum safe fix", @@ -86,6 +198,186 @@ def feature_review_user_prompt( ] }} """.strip() + prompt_bytes = len(prompt.encode("utf-8")) + manifest = ReviewPromptManifest( + max_owned_files=config.review.max_owned_files, + max_context_files=config.review.max_context_files, + included_files=tuple(included_files), + omitted_files=tuple(omitted_files), + prompt_bytes=prompt_bytes, + approximate_tokens=max(1, prompt_bytes // 4), + ) + return ReviewPromptBundle(prompt=prompt, manifest=manifest) + + +@dataclass(frozen=True, slots=True) +class _PromptFile: + block: str + manifest: ReviewPromptFileManifest + + +def _collect_prompt_files( + feature: FeatureRecord, config: ReviewflowConfig +) -> list[tuple[str, ReviewPromptFileRole, str]]: + output: list[tuple[str, ReviewPromptFileRole, str]] = [] + seen: set[str] = set() + + def add(path: str, role: ReviewPromptFileRole, reason: str) -> None: + normalized = _normalize_prompt_path(path) + if normalized in seen: + return + seen.add(normalized) + output.append((normalized, role, reason)) + + for ref in feature.owned_files[: config.review.max_owned_files]: + add(ref.path, "owned", ref.reason) + for ref in feature.context_files[: config.review.max_context_files]: + add(ref.path, "context", ref.reason) + for test in feature.tests[: config.review.max_context_files]: + add(test.path, "test", test.command or "linked test") + return output + + +def _omitted_prompt_files( + feature: FeatureRecord, config: ReviewflowConfig, included: set[str] +) -> list[dict[str, str]]: + omitted: list[dict[str, str]] = [] + + def add_omitted(path: str, role: str, reason: str) -> None: + normalized = _normalize_prompt_path(path) + if normalized not in included: + omitted.append({"path": normalized, "role": role, "reason": reason}) + + for ref in feature.owned_files[config.review.max_owned_files :]: + add_omitted(ref.path, "owned", "maxOwnedFiles") + for ref in feature.context_files[config.review.max_context_files :]: + add_omitted(ref.path, "context", "maxContextFiles") + for test in feature.tests[config.review.max_context_files :]: + add_omitted(test.path, "test", "maxContextFiles") + return omitted + + +def _prompt_file(*, root: Path, path: str, role: ReviewPromptFileRole, reason: str) -> _PromptFile: + full_path = _safe_prompt_path(root, path) + if full_path is None: + manifest = ReviewPromptFileManifest( + path=path, + role=role, + reason=reason, + bytes=0, + included_bytes=0, + included_line_ranges=(), + truncated=False, + readable=False, + skipped_reason="unsafe path", + ) + return _PromptFile( + block=f"## {path}\nRole: {role}\nReason: {reason}\n[unsafe path]", + manifest=manifest, + ) + try: + text = full_path.read_text(encoding="utf-8", errors="replace") + except OSError as exc: + manifest = ReviewPromptFileManifest( + path=path, + role=role, + reason=reason, + bytes=0, + included_bytes=0, + included_line_ranges=(), + truncated=False, + readable=False, + skipped_reason=f"unreadable: {exc.__class__.__name__}", + ) + return _PromptFile( + block=f"## {path}\nRole: {role}\nReason: {reason}\n[unreadable]", manifest=manifest + ) + included = text[:REVIEW_PROMPT_FILE_CHAR_LIMIT] + truncated = len(included) < len(text) + numbered = _line_numbered(included) + line_count = max(1, included.count("\n") + (0 if included.endswith("\n") else 1)) + manifest = ReviewPromptFileManifest( + path=path, + role=role, + reason=reason, + bytes=len(text.encode("utf-8")), + included_bytes=len(included.encode("utf-8")), + included_line_ranges=(ReviewPromptLineRange(start_line=1, end_line=line_count),), + truncated=truncated, + readable=True, + skipped_reason=None, + included_text=included, + ) + trailer = "\n[truncated: only the lines above are valid evidence]" if truncated else "" + block = f"## {path}\nRole: {role}\nReason: {reason}\n```text\n{numbered}{trailer}\n```" + return _PromptFile(block=block, manifest=manifest) + + +def _safe_prompt_path(root: Path, path: str) -> Path | None: + try: + root_resolved = root.resolve() + full_path = (root / path).resolve() + full_path.relative_to(root_resolved) + except (OSError, ValueError): + return None + return full_path + + +def _line_numbered(text: str) -> str: + lines = text.splitlines() + if not lines: + lines = [""] + return "\n".join(f"{idx:>5} | {line}" for idx, line in enumerate(lines, start=1)) + + +def _manifest_prompt_context( + *, + max_owned_files: int, + max_context_files: int, + included_files: list[ReviewPromptFileManifest], + omitted_files: list[dict[str, str]], +) -> dict[str, object]: + return { + "maxOwnedFiles": max_owned_files, + "maxContextFiles": max_context_files, + "includedFiles": [ + { + "path": file.path, + "role": file.role, + "reason": file.reason, + "bytes": file.bytes, + "includedBytes": file.included_bytes, + "includedLineRanges": [ + {"startLine": item.start_line, "endLine": item.end_line} + for item in file.included_line_ranges + ], + "truncated": file.truncated, + "readable": file.readable, + "skippedReason": file.skipped_reason, + } + for file in included_files + ], + "omittedFiles": omitted_files, + } + + +def _custom_prompt_block(custom_prompt: str | None) -> str: + if custom_prompt is None or not custom_prompt.strip(): + return "" + return f"Additional reviewer guidance from --prompt-file:\n{custom_prompt.strip()}\n\n" + + +def _review_mode_guidance(mode: str) -> str: + if mode != "deslopify": + return "" + return """- Deslopify mode: report only concrete simplification findings in category + maintainability or performance. +- Do not look for general bugs, security issues, API contract problems, or hypothetical edge cases. +- Findings must remove real complexity or measurable waste without changing behavior.""" + + +def _normalize_prompt_path(path: str) -> str: + return path.replace("\\", "/").removeprefix("./").rstrip("/") async def review_feature( @@ -96,17 +388,81 @@ async def review_feature( config: ReviewflowConfig, mode: str, timeout_s: float, + custom_prompt: str | None = None, ) -> FeatureReviewOutput: + return ( + await review_feature_partitioned( + llm=llm, + root=root, + feature=feature, + config=config, + mode=mode, + timeout_s=timeout_s, + custom_prompt=custom_prompt, + ) + ).output + + +async def review_feature_partitioned( + *, + llm: ReviewLLM, + root: Path, + feature: FeatureRecord, + config: ReviewflowConfig, + mode: str, + timeout_s: float, + custom_prompt: str | None = None, +) -> PartitionedFeatureReviewResult: + bundle = build_feature_review_prompt_bundle( + root=root, + feature=feature, + config=config, + mode=mode, + custom_prompt=custom_prompt, + ) result = await complete_typed_json( llm=llm, system=REVIEW_SYSTEM, - user=feature_review_user_prompt(root=root, feature=feature, config=config, mode=mode), + user=bundle.prompt, timeout_s=timeout_s, - output_type=FeatureReviewOutput, + output_type=_LooseFeatureReviewOutput, ) if not result.ok or result.output is None: raise RuntimeError(result.failure_message or result.failure_reason or "review failed") - return result.output + output, drops = _partition_review_output(result.output) + return PartitionedFeatureReviewResult( + output=output, + manifest=bundle.manifest, + dropped_findings=drops, + ) + + +def _partition_review_output( + output: _LooseFeatureReviewOutput, +) -> tuple[FeatureReviewOutput, tuple[ReviewDrop, ...]]: + findings: list[FeatureReviewFinding] = [] + drops: list[ReviewDrop] = [] + for idx, candidate in enumerate(output.findings): + try: + findings.append(FeatureReviewFinding.model_validate(candidate)) + except ValidationError as exc: + drops.append( + ReviewDrop( + path=("findings", idx), + message=_format_validation_error(exc), + layer="schema", + ) + ) + return FeatureReviewOutput(findings=findings), tuple(drops) + + +def _format_validation_error(error: ValidationError) -> str: + first = error.errors()[0] if error.errors() else None + if first is None: + return "schema validation failed" + loc = ".".join(str(item) for item in first.get("loc", ())) or "" + msg = str(first.get("msg", "schema validation failed")) + return f"{loc}: {msg}" async def revalidate_finding( @@ -193,8 +549,17 @@ def validation_commands_for_feature(feature: FeatureRecord, config: ReviewflowCo __all__ = [ + "PartitionedFeatureReviewResult", + "REVIEW_PROMPT_FILE_CHAR_LIMIT", + "ReviewDrop", + "ReviewPromptFileManifest", + "ReviewPromptManifest", + "ReviewPromptBundle", + "build_feature_review_prompt_bundle", + "feature_review_user_prompt", "plan_fix", "review_feature", + "review_feature_partitioned", "revalidate_finding", "validation_commands_for_feature", ] diff --git a/packages/pythinker-review/src/pythinker_review/reviewflow/workflow.py b/packages/pythinker-review/src/pythinker_review/reviewflow/workflow.py index 63ec21f2..6fa03869 100644 --- a/packages/pythinker-review/src/pythinker_review/reviewflow/workflow.py +++ b/packages/pythinker-review/src/pythinker_review/reviewflow/workflow.py @@ -34,9 +34,11 @@ derive_finding_triage, ) from pythinker_review.reviewflow.provider import ( + ReviewDrop, + ReviewPromptManifest, plan_fix, revalidate_finding, - review_feature, + review_feature_partitioned, validation_commands_for_feature, ) from pythinker_review.reviewflow.reporting import ( @@ -75,7 +77,6 @@ discover_git, git_output, now_iso, - read_text_bounded, run_id, run_process, run_shell_command, @@ -99,6 +100,27 @@ def __init__(self, message: str, code: str = "workflow-error") -> None: self.code = code +class _AsyncRateLimiter: + """Small in-process start-rate limiter for provider calls.""" + + def __init__(self, per_minute: int | None) -> None: + self._interval_s = 0.0 if per_minute is None or per_minute <= 0 else 60.0 / per_minute + self._lock = asyncio.Lock() + self._next_start_at = 0.0 + + async def wait(self) -> None: + if self._interval_s <= 0: + return + async with self._lock: + loop = asyncio.get_running_loop() + now = loop.time() + if self._next_start_at > now: + await asyncio.sleep(self._next_start_at - now) + now = loop.time() + self._next_start_at = max(self._next_start_at, now) + self._interval_s + + +_NONFATAL_REVIEW_ERROR_CODES = {"schema-drop", "validation-drop"} _DEFAULT_STATE_DIR = ".pythinker-review-flow" _LEGACY_STATE_DIR = ".clawpatch" @@ -323,6 +345,8 @@ async def review_project( mode: str = "default", dry_run: bool = False, per_feature_timeout_s: float = 180.0, + custom_prompt: str | None = None, + rate_limit_per_minute: int | None = None, ) -> dict[str, Any]: loaded = load_project_state(root=root.resolve(), state_dir=state_dir, config_path=config_path) features = select_review_features( @@ -346,6 +370,7 @@ async def review_project( run.claimed_feature_ids = [feature.feature_id for feature in features] write_run(loaded.paths, run) semaphore = asyncio.Semaphore(max(1, min(jobs, max(len(features), 1)))) + limiter = _AsyncRateLimiter(rate_limit_per_minute) finding_ids: list[str] = [] errors: list[RunError] = [] @@ -364,19 +389,33 @@ async def worker(feature: FeatureRecord) -> None: ), allow_non_pending=feature_id is not None, ) - produced = await review_feature( + await limiter.wait() + produced = await review_feature_partitioned( llm=llm, root=loaded.root, feature=locked, config=loaded.config, mode=mode, timeout_s=per_feature_timeout_s, + custom_prompt=custom_prompt, + ) + valid, validation_drops = _validated_review_findings( + loaded.root, + produced.manifest, + produced.output.findings, ) - valid = _validated_review_findings(loaded.root, locked, produced.findings) + for drop in (*produced.dropped_findings, *validation_drops): + errors.append(_drop_run_error(locked.feature_id, drop)) ids = _merge_review_findings(loaded, locked, valid, current_run_id) finding_ids.extend(ids) _mark_feature_reviewed( - loaded, locked, ids, current_run_id, provider=llm.model_display_name + loaded, + locked, + ids, + current_run_id, + provider=llm.model_display_name, + manifest=produced.manifest, + dropped=len(produced.dropped_findings) + len(validation_drops), ) release_feature_lock(loaded.paths, locked.feature_id) locked = None @@ -394,7 +433,8 @@ async def worker(feature: FeatureRecord) -> None: write_feature(loaded.paths, feature) await asyncio.gather(*(worker(feature) for feature in features)) - run.status = "failed" if errors else "completed" + fatal_errors = [error for error in errors if error.code not in _NONFATAL_REVIEW_ERROR_CODES] + run.status = "failed" if fatal_errors else "completed" run.finished_at = now_iso() run.finding_ids = finding_ids run.errors = errors @@ -402,8 +442,10 @@ async def worker(feature: FeatureRecord) -> None: report_path = _write_markdown_report( loaded.paths, read_findings(loaded.paths), read_features(loaded.paths) ) - if errors: - raise ReviewflowWorkflowError(errors[0].message, errors[0].code or "review-failed") + if fatal_errors: + raise ReviewflowWorkflowError( + fatal_errors[0].message, fatal_errors[0].code or "review-failed" + ) return { "run": current_run_id, "reviewed": len(features), @@ -937,40 +979,113 @@ def _new_run(command: str, loaded: LoadedState, current_run_id: str) -> RunRecor ) +def _drop_run_error(feature_id: str, drop: ReviewDrop) -> RunError: + return RunError( + message=( + f"dropped 1 finding from feature {feature_id} at " + f"{'.'.join(str(item) for item in drop.path)}: {drop.message}" + ), + code=f"{drop.layer}-drop", + ) + + def _validated_review_findings( - root: Path, feature: FeatureRecord, findings: list[Any] -) -> list[Any]: - allowed = {ref.path for ref in feature.owned_files} | { - ref.path for ref in feature.context_files - } + root: Path, + manifest: ReviewPromptManifest, + findings: list[Any], +) -> tuple[list[Any], list[ReviewDrop]]: out: list[Any] = [] - for finding in findings: + drops: list[ReviewDrop] = [] + for idx, finding in enumerate(findings): if not finding.evidence: + drops.append( + ReviewDrop( + path=("findings", idx, "evidence"), + message="finding has no evidence", + layer="validation", + ) + ) continue - if all(_valid_evidence(root, evidence, allowed) for evidence in finding.evidence): - out.append(finding) - return out - - -def _valid_evidence(root: Path, evidence: EvidenceRef, allowed: set[str]) -> bool: - if evidence.path not in allowed: - return False + failures = [ + reason + for evidence in finding.evidence + if (reason := _evidence_validation_failure(root, evidence, manifest)) is not None + ] + if failures: + drops.append( + ReviewDrop( + path=("findings", idx, "evidence"), + message=failures[0], + layer="validation", + ) + ) + continue + out.append(finding) + return out, drops + + +def _evidence_validation_failure( + root: Path, evidence: EvidenceRef, manifest: ReviewPromptManifest +) -> str | None: + prompt_file = next( + ( + file + for file in manifest.included_files + if file.path == _normalize_repo_path(evidence.path) + ), + None, + ) + if prompt_file is None: + return f"evidence file was not included in review context: {evidence.path}" + if not prompt_file.readable: + return f"evidence file was not readable in review context: {evidence.path}" try: resolved = (root / evidence.path).resolve() resolved.relative_to(root.resolve()) except ValueError: - return False + return f"evidence file escapes repository root: {evidence.path}" if not resolved.is_file(): - return False - text = read_text_bounded(resolved, limit_chars=100_000) - lines = text.splitlines() - if ( - evidence.start_line is not None - and evidence.end_line is not None - and (evidence.start_line < 1 or evidence.end_line > len(lines)) - ): - return False - return not (evidence.quote and evidence.quote not in text) + return f"evidence file is not readable inside repository: {evidence.path}" + text = resolved.read_text(encoding="utf-8", errors="replace") + if evidence.start_line is None and evidence.end_line is None: + if not evidence.quote or not evidence.quote.strip(): + return f"evidence must include a line range or quote: {evidence.path}" + elif evidence.start_line is None or evidence.end_line is None: + return f"evidence line range must include both startLine and endLine: {evidence.path}" + else: + if evidence.start_line > evidence.end_line: + return f"evidence line range is inverted: {evidence.path}" + if evidence.end_line > _review_line_count(text): + return f"evidence line range exceeds file length: {evidence.path}" + if not _range_included(evidence.start_line, evidence.end_line, prompt_file): + return f"evidence line range was not included in review context: {evidence.path}" + if evidence.quote and evidence.quote.strip(): + target = prompt_file.included_text + if evidence.start_line is not None and evidence.end_line is not None: + target = "\n".join(text.splitlines()[evidence.start_line - 1 : evidence.end_line]) + if evidence.quote not in target and _compact_whitespace( + evidence.quote + ) not in _compact_whitespace(target): + return f"evidence quote does not match file contents: {evidence.path}" + return None + + +def _review_line_count(contents: str) -> int: + if contents == "": + return 1 + count = contents.count("\n") + return count if contents.endswith("\n") else count + 1 + + +def _range_included(start_line: int, end_line: int, prompt_file: Any) -> bool: + return any( + start_line >= line_range.start_line and end_line <= line_range.end_line + for line_range in prompt_file.included_line_ranges + ) + + +def _compact_whitespace(value: str) -> str: + return " ".join(value.split()) def _merge_review_findings( @@ -1034,6 +1149,8 @@ def _mark_feature_reviewed( run_id_value: str, *, provider: str, + manifest: ReviewPromptManifest, + dropped: int, ) -> None: all_ids = sorted({*feature.finding_ids, *finding_ids}) feature.finding_ids = all_ids @@ -1044,7 +1161,11 @@ def _mark_feature_reviewed( AnalysisEntry( run_id=run_id_value, kind="review", - summary=f"Reviewed with {len(finding_ids)} findings.", + summary=( + f"Reviewed with {len(finding_ids)} findings; dropped {dropped}; " + f"context {len(manifest.included_files)} files, " + f"~{manifest.approximate_tokens} tokens." + ), provider=provider, model=loaded.config.provider.model, reasoning_effort=loaded.config.provider.reasoning_effort, diff --git a/packages/pythinker-review/src/pythinker_review/security_scan/knowledge.py b/packages/pythinker-review/src/pythinker_review/security_scan/knowledge.py new file mode 100644 index 00000000..99830dd4 --- /dev/null +++ b/packages/pythinker-review/src/pythinker_review/security_scan/knowledge.py @@ -0,0 +1,1348 @@ +"""Shared security-review knowledge for prompts and advisor context. + +Most framework highlights and slug notes are ported from the TypeScript +``blackbox/pythinker-security-scanner`` prompt tables. Keep entries short: these are +reviewer instincts and false-positive checks, not tutorials. +""" + +from __future__ import annotations + +TechHighlight = tuple[str, tuple[str, ...], tuple[str, ...]] + +TECH_HIGHLIGHTS: dict[str, TechHighlight] = { + "actix": ( + "Actix-web", + ("rust",), + ( + "Middleware via `App::new().wrap(...)` is global; per-scope wraps via " + "`web::scope().wrap()` — flag scopes with skipped wraps", + "Extractors `web::Query` / `web::Json` / `web::Path` are user input — types " + "only validate STRUCTURE, not content", + "Auth middleware that returns `next.call(req)` unconditionally before the check is the " + "bypass shape", + '`HttpResponse::Ok().body(format!("{}", x))` is XSS — use a templating ' + "crate with escape", + ), + ), + "aiohttp": ( + "aiohttp", + ("python",), + ( + "Middleware via `@web.middleware` runs in declaration order — auth before logging is " + "the safe layout", + "`request.query` / `request.json()` / `request.match_info` / `request.read()` are " + "user input", + "`aiohttp_session` cookies need an explicit storage backend with secret rotation; " + "default `EncryptedCookieStorage` is fine", + "ClientSession (outbound) — flag user-controlled URLs without an allowlist (SSRF)", + ), + ), + "airflow": ( + "Airflow", + ("python",), + ( + "DAGs run with the Airflow scheduler's privileges — operator template fields (`{{ " + "params.x }}`) interpolated into Bash/SQL/HTTP are injection sinks", + '`BashOperator(bash_command=f"... {x}")` is shell injection — even non-templated ' + "f-strings are risky if x is user-influenced", + "Connections and Variables hold credentials — leaking them via XCom or logs is data " + "exposure", + "REST API auth (`auth_backends`) — defaults can be permissive on older versions", + ), + ), + "android": ( + "Android", + (), + ( + '`android:exported="true"` on Activity/Service/Receiver/Provider exposes the ' + "component to other apps — confirm with intent + permission", + "Implicit `` makes a component exported on pre-API-31 even without " + '`android:exported="true"` — flag legacy code', + 'Deeplink schemes (``) — review URL handling for SSRF ' + "(WebView), file:// loads, JS bridges", + "`WebView` with `setJavaScriptEnabled(true)` + `addJavascriptInterface` is RCE if " + "the loaded URL is attacker-controlled", + "ContentProvider exported without permission grants reads/writes to any app — " + '`android:grantUriPermissions="true"` widens scope further', + ), + ), + "apex": ( + "Apex (Salesforce)", + (), + ( + "`without sharing` classes BYPASS row-level security — confirm every `without sharing` " + "is intentional and that the methods can't be invoked by unprivileged users", + "`@AuraEnabled` methods are reachable from Lightning components without extra auth — " + "same surface as REST", + "`Database.query('SELECT ... WHERE ... = \\'' + userInput + '\\'')` is SOQL injection; " + "`[SELECT ... WHERE id = :userInput]` is bound and safe", + "FLS / CRUD checks (`Schema.sObjectType.X.isAccessible()`) are NOT automatic — flag DML " + "on sObjects without explicit checks", + "`@RestResource(urlMapping='...')` exposes the class on `/services/apexrest/` — public " + "to authenticated Salesforce users; confirm the data filter", + ), + ), + "astro": ( + "Astro", + ("typescript", "javascript"), + ( + "`pages/api/**/*.ts` exports (`GET`/`POST`/...) are public; `prerender = false` opts a " + "page into SSR with the same auth concerns", + "`Astro.request` / `Astro.cookies` / `Astro.params` are user input — same sinks as " + "Next.js", + "Default output is static; double-check if a route silently became SSR via `export " + "const prerender = false`", + "Astro uses Vite — env vars prefixed with `PUBLIC_` ship to the client bundle", + ), + ), + "aws-lambda": ( + "AWS Lambda", + (), + ( + "API Gateway authorizer claims live on `event.requestContext.authorizer` — " + "handlers that don't read them are unauthenticated", + "`event.body` is JSON-string in proxy integrations — JSON.parse failures should " + "NOT echo `event` (leaks request data into logs)", + "IAM role on the function determines blast radius — over-permissioned roles + RCE " + "= account takeover", + "Cold-start global state is shared across invocations on the same container — " + "credentials/PII can leak between tenants", + "Lambda timeouts default to 3s but can be 15min — long-running handlers without " + "per-call rate limits enable cost amplification", + ), + ), + "axum": ( + "Axum", + ("rust",), + ( + '`Router::new().route("/", get(h)).layer(auth_layer)` — `.layer` order matters; routes ' + "added AFTER `.layer` may not be wrapped", + "`Extension` / `State` carry auth identity — flag handlers that skip them", + "`Path` / `Query` / `Json` extractors are user input; same sinks as Actix", + "`.merge(other_router)` and `.nest(prefix, other)` — sub-routers inherit parent layers " + "but the order of `.layer` vs `.merge`/`.nest` matters", + ), + ), + "azure-functions": ( + "Azure Functions", + ("csharp", "javascript", "typescript", "python"), + ( + "`AuthorizationLevel.Anonymous` on `HttpTrigger` is a public endpoint — " + "confirm intent; `Function`/`Admin` require a function key", + "Function keys are NOT user identity — they authenticate the *caller app*, " + "not a user; for user auth use Easy Auth or App Service Authentication", + "Triggers (Queue/ServiceBus/Blob) are reached via Azure infra — payloads are " + "still user input if any web caller can write to the queue", + 'Bindings (e.g. `[Blob("path/{queueTrigger}")]`) interpolate input into ' + "resource paths — can be path traversal across containers", + ), + ), + "bottle": ( + "Bottle", + ("python",), + ( + "Bottle has no built-in auth — every `@route` is public unless a decorator chain " + "enforces a check", + "`request.query` / `request.forms` / `request.json` are user input", + "SimpleTemplate `{{!x}}` is unescaped; `{{x}}` auto-escapes — flag the bang form", + "`static_file(filename, root)` without `path.basename(filename)` is path traversal", + ), + ), + "buffalo": ( + "Buffalo", + ("go",), + ( + "`app.Use(...)` middleware is global; `app.Resource(...)` registers CRUD — confirm " + "auth wraps both", + "`c.Param('x')` / `c.Request()` / `c.Bind(&v)` are user input", + "`render.Auto` chooses HTML / JSON / XML by Accept header — DB rows in the response " + "include all columns; use a response shape", + ), + ), + "bullmq": ( + "BullMQ", + ("typescript", "javascript"), + ( + "`job.data` is whatever the producer enqueued — treat it as user input if any web " + "handler can enqueue", + "Workers run with elevated trust (no auth context) — confirm the queue boundary " + "validates / authorizes the request before enqueue", + "Retry on poison messages can amplify a single attacker payload across retries — flag " + "handlers without idempotency keys", + "`Queue.add(..., { delay })` at long delays plus user-controlled payload = " + "stored-XSS-via-job", + ), + ), + "bun": ( + "Bun", + ("typescript", "javascript"), + ( + "`Bun.serve({ fetch })` is a raw HTTP entry — auth/validation lives entirely in the " + "handler, no framework gates", + "`Bun.spawn(...)` / `Bun.$`...`` shell template — interpolated user input is RCE-shaped", + "Bun's TLS/HTTP defaults differ from Node; verify rejected-cert handling on outbound " + "`fetch`", + ), + ), + "cakephp": ( + "CakePHP", + ("php",), + ( + "`$this->Auth->allow(...)` opens specific actions to the public — confirm the list " + "is intentional", + "`$this->request->getData()` is user input; mass assignment via `patchEntity()` " + "without `accessibleFields` is the bug", + "Bake-generated views use `h($x)` for escape — flag templates that emit raw `$x` " + "without `h()`", + "`->find()->where(['col' => $x])` is parameterized; `->find()->where(\"col = " + "'$x'\")` is SQLi", + ), + ), + "celery": ( + "Celery", + ("python",), + ( + "Task args are deserialized via the configured serializer — `pickle` is unsafe " + "deserialization (RCE)", + "Tasks run with worker-level trust (no request user) — re-validate ownership when a " + "task acts on user data", + "`task.delay(user_id=...)` invocations from web code: confirm the call site " + "authenticates the user before enqueue", + "Long retries on poison messages can amplify a single bad payload", + ), + ), + "chi": ( + "Chi", + ("go",), + ( + "`r.Use(middleware)` and `r.Group(...)` define auth scopes — sub-routers inherit, but " + '`r.Mount("/x", h)` does NOT inherit middleware applied after the mount', + '`chi.URLParam(r, "id")` is user input; treat as untrusted in DB / fs / exec calls', + "`render.JSON(w, r, data)` returns whatever you pass — DB rows often include secret " + "columns; use a response-shape struct", + ), + ), + "clojure": ( + "Clojure (Ring/Compojure)", + (), + ( + "Ring middleware composes via `wrap-*`; auth must be in the chain BEFORE the route " + "handler", + "`wrap-anti-forgery` (CSRF) is opt-in — flag apps using session cookies without it", + 'Compojure `(GET "/x" [id] ...)` destructures params; `(get-in request [:params ' + ":x])` is the same — both untrusted", + "`ring.util.response/redirect` to user-controlled paths is open-redirect without an " + "allowlist", + ), + ), + "cobra": ( + "Cobra", + ("go",), + ( + "Privileged CLI surface — flags often hold secrets (`--token`, `--password`); flag any " + "logging of `cmd.Flags()`", + "`Run`/`RunE` handlers operate with the operator's privileges; user-supplied args " + "interpolated into shell or SQL are injection", + "`PersistentFlags` propagate to subcommands — credential flags on a parent leak to all " + "children", + ), + ), + "codeigniter": ( + "CodeIgniter", + ("php",), + ( + "Filters in `app/Config/Filters.php` are the auth gate; routes outside the " + "filter scope are public", + "`$this->request->getVar('x')` / `getPost()` are user input — concatenation into " + 'SQL via `$db->query("...$x...")` is injection', + "`view('name', $data)` auto-escapes; setting the third arg to disable escape " + "requires explicit trust review", + "`helper()` and `service()` calls can load arbitrary code if names are user-influenced", + ), + ), + "dart": ( + "Dart (Shelf)", + (), + ( + "Shelf has no built-in auth — `Pipeline().addMiddleware()` is the gate, registration " + "order matters", + "`request.url.queryParameters` / `request.readAsString()` are user input", + "`Response.ok(body)` doesn't HTML-escape; templates need explicit escape if rendering " + "HTML", + "`io.serve(handler, ...)` exposes the handler directly — no framework gates beyond what " + "you write", + ), + ), + "deno": ( + "Deno", + ("typescript", "javascript"), + ( + "`Deno.serve(handler)` is the entry; no built-in auth — middleware order is hand-rolled", + "Permissions (`--allow-net`, `--allow-read`, `--allow-env`) are deploy-time; code that " + "calls `Deno.permissions.request` at runtime is suspicious", + "Oak `ctx.request.body()` / `ctx.params` are untrusted; same sinks as Express", + ), + ), + "django": ( + "Django", + ("python",), + ( + "`@csrf_exempt` views handling state-changing POSTs without an alternate auth " + "(signature, token) are CSRF-vulnerable", + "`Model.objects.raw(...)` / `cursor.execute()` with f-string interpolation is SQL " + "injection — flag any %-formatted SQL", + "`mark_safe()` / `format_html()` on user input is XSS; same for `{% autoescape off " + "%}` blocks", + "`ModelForm` without `fields = [...]` (or with `__all__`) exposes mass-assignment of " + "every model column", + "`DEBUG=True` + `ALLOWED_HOSTS=['*']` in any reachable settings file leaks tracebacks " + "and SECRET_KEY material", + ), + ), + "djangorestframework": ( + "Django REST Framework", + ("python",), + ( + "`permission_classes` missing or set to `AllowAny` on a sensitive " + "`ModelViewSet` exposes full CRUD", + "`ModelSerializer` with `fields = '__all__'` allows mass-assignment of " + "admin-only columns via PATCH", + "`@action(detail=True)` methods inherit the viewset's permissions but " + "custom routers can break this — confirm", + ), + ), + "dotnet": ( + ".NET / ASP.NET Core", + ("csharp",), + ( + "`[Authorize]` is the gate; `[AllowAnonymous]` on a sensitive action opens it back up " + "— confirm intent", + "`[FromQuery]` / `[FromBody]` / `[FromRoute]` are user input — model binding is " + "structure-only", + "`[ApiController]` adds automatic 400 on model-state errors; absence means the " + "handler MUST check `ModelState.IsValid`", + "Razor `@Html.Raw(x)` on user input is XSS; bare `@x` HTML-encodes (safe)", + "Minimal API `app.MapGet(...).RequireAuthorization()` is the gate — flag chains " + "without it on sensitive routes", + 'EF Core `FromSqlRaw($"... {x} ...")` is SQLi; `FromSqlInterpolated($"... {x} ...")` ' + "parameterizes correctly", + ), + ), + "drupal": ( + "Drupal", + ("php",), + ( + "`*.routing.yml` `_permission`/`_access` keys are the gate — `access content` is " + "permissive (most authenticated users have it)", + "`\\Drupal::request()->query->get('x')` / `request->get('x')` are user input", + "`$this->t('@name', ['@name' => $userInput])` auto-escapes via `@`/`%`; bare " + "placeholders without prefix are unsafe", + "`db_query(\"... $x\")` is SQL injection; `\\Drupal::database()->query('... :x', " + "[':x' => $x])` is parameterized", + ), + ), + "echo": ( + "Echo", + ("go",), + ( + "`e.Use(middleware)` order matters — routes registered before `Use` aren't covered", + '`c.Bind(&v)` accepts JSON/form/query — fields with `json:"-"` matter only if you USE ' + '`json:"-"`; explicit allowlists in DTO structs are the safe form', + 'Group-level middleware (`g := e.Group("/api", auth)`) — confirm sensitive routes live ' + "under the group, not on the root `e`", + ), + ), + "erlang": ( + "Erlang (Cowboy)", + (), + ( + "`init/2` is the cowboy entry — auth check must happen before any state-changing call", + "`cowboy_req:binding(name, Req)` / `read_body/1` / `parse_qs/1` are user input", + "Erlang term decoding from external sources via `binary_to_term/1` is unsafe " + "deserialization — use `binary_to_term/2` with `[safe]`", + "Process-per-request model isolates handler crashes, but supervision-tree restart " + "strategies can hide errors", + ), + ), + "express": ( + "Express.js", + ("typescript", "javascript"), + ( + "Each `app.get/post/...` and `router.use` is a public endpoint — confirm auth " + "middleware actually wraps it (order matters; routes mounted before " + "`app.use(authMiddleware)` are unprotected)", + "`req.query`/`req.params`/`req.body` are user input; concatenation into SQL, shell, " + "paths, or URLs is the usual sink", + "`express.static` on a user-influenced root, or `res.sendFile(req.params.x)`, is " + "path traversal", + "Error handlers that send `err.stack` or `err.message` to the response leak internals", + "CORS `origin: true` reflecting credentials enables CSRF-via-fetch", + ), + ), + "falcon": ( + "Falcon", + ("python",), + ( + "`on_(self, req, resp, ...)` handlers are public unless a middleware/hook " + "checks auth", + "`req.media` / `req.params` / `req.get_param('x')` are user input", + "`req.context` carries auth-claim data — confirm it's set BEFORE the resource handler " + "runs", + "Falcon's `resp.media` accepts dicts directly; over-fetching DB rows leaks PII", + ), + ), + "fastapi": ( + "FastAPI", + ("python",), + ( + "Auth lives in `Depends(...)`; routes without an auth dependency are public — " + "`@app.get('/admin')` with no Depends is the common gap", + "Pydantic models validate input but `Optional[Any]` / `dict` fields are an escape " + "hatch — flag them on inputs", + "`response_model=...` filters server output; without it, you may return DB columns " + "containing secrets", + "`StaticFiles(directory=...)` rooted at a user-influenced path is path traversal", + ), + ), + "fastify": ( + "Fastify", + ("typescript", "javascript"), + ( + "`preHandler` / `onRequest` hooks are the auth layer; routes registered without them " + "or before the auth plugin are unprotected", + "Schema validation (`schema: { body, querystring }`) is the default mitigation — " + "flag handlers that read raw `request.body` without a schema", + "Plugins registered with `register()` inherit hooks per-scope; cross-scope auth " + "bypass is common in monorepos", + "`reply.send(err)` returns full error objects in dev mode; check the prod config", + ), + ), + "fiber": ( + "Fiber", + ("go",), + ( + "`app.Get/Post/...` registers public endpoints; middleware via `app.Use(auth)` must " + "precede them, and route-level middleware override group middleware", + "`c.Query` / `c.Params` / `c.Body` / `c.BodyParser(&v)` are user input; injection " + "sinks are the same as net/http", + "Fiber wraps fasthttp — request bodies/headers are not safe to retain past the handler " + "return; flag goroutines that capture `c` by reference", + ), + ), + "flask": ( + "Flask", + ("python",), + ( + "`@app.route(...)` without a `@login_required` (or equivalent) decorator is public; " + "check the order of decorators — `@app.route` must be outermost", + "`render_template_string(user_input)` is server-side template injection (RCE)", + "`request.args` / `request.form` / `request.json` interpolated into SQL via " + '`db.engine.execute(f"...")` is SQL injection', + "`send_from_directory(dir, request.args['file'])` without a basename check is path " + "traversal", + "`session` cookies use `app.secret_key` — hardcoded keys in source are session forgery", + ), + ), + "gcp-cloud-functions": ( + "GCP Cloud Functions", + (), + ( + "Allow-unauthenticated invocations (`--allow-unauthenticated`) make the " + "function public — confirm via deploy config", + "IAM-based auth (Cloud IAM) is invoker-level; for user identity, " + "integrate Identity Platform / Firebase Auth in the handler", + "Function URLs include the project ID and region — leakage of these is " + "information disclosure", + "Background functions (Pub/Sub, Storage triggers) — payload comes from " + "the GCP infra but is still ATTACKER-INFLUENCED if any web path can " + "write to the bucket/topic", + ), + ), + "gin": ( + "Gin", + ("go",), + ( + "Each `r.GET/POST/...` and `r.Group(...)` is a public endpoint; auth middleware applied " + "via `r.Use(...)` must precede route registration in the same group", + "`c.Query`/`c.Param`/`c.PostForm` are user input — usual injection surfaces (SQL, exec, " + "fs, URL) apply", + '`c.HTML(http.StatusOK, "tmpl", data)` with `data` containing untrusted strings is XSS ' + "unless the template uses `{{.X}}` (auto-escaped) and not `{{.X | safehtml}}`", + ), + ), + "github-actions": ( + "GitHub Actions", + ("yaml", "yml"), + ( + "pull_request_target and workflow_run can expose secrets to untrusted code.", + "Actions should be pinned; github.event/head_ref interpolation in run scripts " + "is shell-injection shaped.", + "permissions: write-all and broad id-token: write need justification.", + ), + ), + "go": ( + "Go web services", + ("go",), + ( + "Router middleware must wrap the exact route/group before registration.", + "c.Query/r.URL.Query/FormValue/path params are untrusted for SQL, exec, fs, " + "and HTTP clients.", + "Prefer response-shape structs to raw DB rows.", + ), + ), + "gorilla": ( + "Gorilla mux", + ("go",), + ( + "`router.Use(authMiddleware)` covers the router; subrouters via `Subrouter()` " + "inherit, but `PathPrefix(...).Handler(other)` does not", + "`mux.Vars(r)` is user input — usual injection sinks (SQL, exec, fs, URL)", + '`router.HandleFunc("/x", h).Methods("GET")` — flag handlers without an explicit ' + "`.Methods` (accept any verb)", + ), + ), + "grape": ( + "Grape", + ("ruby",), + ( + "Auth lives in `before do ... end` or `helpers do ... end` — endpoints without it are " + "public", + "`params` is the only safe input accessor; raw `request.body.read` skips Grape's " + "coercion", + "`declared(params, include_missing: false)` is strong-params equivalent — flag " + "handlers that use `params` directly for mass assignment", + "API versioning paths (`version 'v1'`) — confirm deprecated versions still enforce " + "auth", + ), + ), + "graphql": ( + "GraphQL", + ("typescript", "javascript"), + ( + "Per-resolver auth: every Query/Mutation/Subscription field is independently " + "reachable — flag resolvers that don't check `context.user`", + "Field-level vs object-level auth: returning a User object grants access to all " + "fields unless guards exist on `email`/`role`/etc.", + "Disabled introspection in prod — leaving it on leaks the full schema (informational " + "severity)", + "Query depth/complexity limits stop abusive nested queries; absence is the bug", + "Aliasing + batching can multiply the cost of an unauthenticated query — expensive " + "resolvers need rate limits", + ), + ), + "hanami": ( + "Hanami", + ("ruby",), + ( + "Each `Hanami::Action` subclass is publicly addressable via the router — `before` " + "callbacks are the auth gate", + "Strong-params equivalent: `params.valid?` + a Contract — handlers using raw `params` " + "skip validation", + "`include Deps[...]` for DI: shared DB / repo objects can leak ownership semantics if " + "used as singletons", + ), + ), + "hapi": ( + "Hapi", + ("typescript", "javascript"), + ( + "`auth: false` on a `server.route(...)` opts out of the default auth strategy — confirm " + "it's intentional, especially on writes", + "Validate routes use `validate: { query, payload, params }`; routes without validation " + "pass raw input to handlers", + "`server.auth.default(...)` sets the global gate; flag handlers that pre-date it or " + "that pass `auth: 'optional'`", + "`request.payload` / `request.query` / `request.params` are user input", + ), + ), + "hono": ( + "Hono", + ("typescript", "javascript"), + ( + "Each `app.get('/path', handler)` is a public endpoint; auth middleware (`app.use('*', " + "auth)`) must come BEFORE the route declarations or it's a no-op", + "`c.req.query()` / `c.req.param()` / `c.req.json()` are user input — usual injection " + "surfaces apply", + "Hono runs on Workers/edge runtimes — check whether route handlers reach into a " + "separate Node backend without re-authenticating", + ), + ), + "ios": ( + "iOS", + (), + ( + "`CFBundleURLSchemes` registers your app as a URL handler — `application(_:open:)` / " + "`scene(_:openURLContexts:)` receive attacker-controlled URLs", + "Universal Links via `apple-app-site-association` — host association determines which " + "domains can open the app; misconfig is account-takeover-shaped", + "WKWebView with `loadHTMLString(html, baseURL:)` and a `file://` baseURL gives the page " + "access to local files", + "Keychain access without `kSecAttrAccessibleWhenUnlocked` (or stricter) leaks " + "credentials at app launch", + "App Transport Security exceptions in Info.plist (`NSAllowsArbitraryLoads`) downgrade " + "TLS — flag any plist that opts out", + ), + ), + "jaxrs": ( + "JAX-RS (Jersey/Quarkus/RESTEasy)", + ("java", "kotlin"), + ( + "`@RolesAllowed`/`@DenyAll`/`@PermitAll` are the gate; absence on a `@Path` resource " + "is public", + "`@QueryParam`/`@PathParam`/`@FormParam`/`@HeaderParam` are user input", + "`@RequestScoped` provider classes can leak per-request state if held by " + "`@ApplicationScoped` resources", + "`Response.ok(entity)` with raw JPA entities over-fetches columns; use a DTO", + ), + ), + "kemal": ( + "Crystal Kemal", + (), + ( + "No built-in auth — every `get '/path' do ... end` is public unless a `before_*` " + "filter intercepts", + '`env.params.url["x"]` / `env.params.json["x"]` / `env.params.body["x"]` are user ' + "input", + "Crystal's macro-driven JSON parsing is type-safe but content-unvalidated; bounds " + "checks on collections matter", + ), + ), + "koa": ( + "Koa", + ("typescript", "javascript"), + ( + "`router.` routes registered before `app.use(authMiddleware)` are unprotected — " + "middleware order matters", + "`ctx.request.body` / `ctx.query` / `ctx.params` are user input; same injection sinks as " + "Express", + "`ctx.throw(401)` is a soft response — confirm it's reached BEFORE any data is " + "fetched/returned", + "`koa-bodyparser` defaults to forms+json; large payload limits and prototype-pollution " + "opts must be set explicitly", + ), + ), + "ktor": ( + "Ktor", + ("kotlin",), + ( + '`authenticate("jwt") { ... }` blocks are the gate — routes outside them are public', + "`call.receive()` deserializes user input — `kotlinx.serialization` is " + "structure-validating, not content-validating", + "`call.parameters` / `call.request.queryParameters` are user input", + "Status pages plugin handles errors — confirm prod config doesn't echo exceptions to " + "the response", + ), + ), + "lambda-rs": ( + "Rust AWS Lambda", + ("rust",), + ( + "`LambdaEvent::payload` is API Gateway / SQS / etc. payload — type-driven but " + "content is user-supplied", + "`event.payload.request_context.authorizer` carries claims when API Gateway " + "authorizer is configured — handler must verify", + "Cold-start global state (lazy_static / OnceCell) survives across invocations — " + "credentials/state leakage between tenants", + ), + ), + "laravel": ( + "Laravel", + ("php",), + ( + "`Model::create($request->all())` without `$fillable`/`$guarded` is mass assignment " + "— admin columns get overwritten", + "`DB::raw()` / `whereRaw()` / `selectRaw()` with interpolated input is SQL injection", + "`VerifyCsrfToken::$except` lists that include state-changing routes are " + "CSRF-vulnerable unless an alternate verification (signed URL, webhook signature) " + "exists", + "Blade `{!! $x !!}` renders raw HTML — XSS sink", + "Routes outside the `auth` middleware group, or routes with " + "`->withoutMiddleware([...])`, need explicit per-action auth checks", + ), + ), + "magento": ( + "Magento", + ("php",), + ( + "ACL via `etc/acl.xml`; webapi routes via `etc/webapi.xml` `` — flag " + "routes set to `anonymous` doing sensitive work", + "`$this->getRequest()->getParam('x')` is user input", + "Plugin/observer code runs in core context — privilege escalation is easy if input " + "isn't sanitized", + "Customer data via `\\Magento\\Customer\\Api` requires customer ID; flag any read " + "using user-supplied ID without ownership check", + ), + ), + "mcp": ( + "MCP / agentic tools", + ("typescript", "javascript", "python"), + ( + "Tool inputs and retrieved content are untrusted data, not instructions.", + "Tool schemas need allowlists, execution caps, and explicit filesystem/network " + "boundaries.", + ), + ), + "micronaut": ( + "Micronaut", + ("java", "kotlin"), + ( + "`@Secured(SecurityRule.IS_AUTHENTICATED)` on controller is the gate; `@PermitAll` " + "opens it back up", + "`@Body` / `@QueryValue` / `@PathVariable` are user input", + "Reactive endpoints return `Mono`/`Flux` — auth check must be in the reactive " + "chain, not just the handler signature", + "Bean introspection (compile-time DI) means runtime config can't easily swap auth " + "— flag config-driven gates", + ), + ), + "nestjs": ( + "NestJS", + ("typescript", "javascript"), + ( + "`@UseGuards(...)` on controller or method is the auth check; missing guards on a " + "`@Controller()` are a common gap", + "`@Body()` / `@Query()` without a `class-validator` DTO is unvalidated input", + "Global pipes/interceptors registered late or only in main.ts may not apply to " + "e2e-test routes shipped to prod", + "`@Public()` decorators that opt OUT of a global auth guard — confirm they are " + "intentional", + ), + ), + "nextjs": ( + "Next.js", + ("typescript", "javascript"), + ( + "Next.js `middleware.ts` runs at the edge and is NOT sufficient auth — too easy to " + "misconfigure or bypass via routes that escape the matcher", + "Server Actions are publicly callable POST endpoints — every one needs explicit auth " + "+ authorization checks", + "`JSON.stringify()` inside `dangerouslySetInnerHTML` or inline `