Skip to content

worktree-audit.sh: closed-unmerged PRs and unpushed commits are bucketed as safe; script fetches despite read-only header #470

Open

Description

@Authentis

Observed at commit 2eb7ed4: pstack/skills/poteto-mode/scripts/worktree-audit.sh.

(The separate problem of untracked files being classed as disposable scratch is already tracked in issue 449 and is not repeated here.)

1. A PR in any non-open state makes a worktree safe

Lines 74-80:

if [ "$recent" = yes ]; then bucket=verify-recent-chat
elif [ "$merged" = YES ] || [ "$pr" != "-" ]; then bucket=safe

$pr is #N/STATE for any state returned by gh pr list --state all (line 22), so #N/CLOSED (closed without merging) satisfies $pr != "-" and the worktree is bucketed safe. A closed-unmerged PR means the branch's commits may exist nowhere else.

2. The ahead remote state is computed but never used

Lines 53-57 set remote to pushed, aheadN, no-remote or detached. The bucket logic (lines 74-80) never reads $remote. A worktree with N unpushed commits, no open PR and a stale or closed PR is reported safe.

3. The script fetches although it is described as read-only

Line 1 says "Read-only worktree prune audit" and line 4 says "Never deletes anything". Line 18 runs git fetch origin main --quiet, which updates refs/remotes/origin/main and FETCH_HEAD. Not destructive, but it is a write to the repository and the "read-only" claim is inaccurate.

Minimal reproduction

In a repo with a worktree on branch feature that has one unpushed commit and a PR that was closed without merging:

$ worktree-audit.sh
SIZE AGE MERGED DIRTY REMOTE PR BUCKET ...
12M  3d  no     clean ahead1 #42/CLOSED safe ...

The bucket is safe although the commit is unmerged and unpushed.

Suggested fix

  • Treat only MERGED as a merged-PR signal; a CLOSED PR should fall through to review.
  • Put ahead*, no-remote and detached (with commits not reachable from origin/main) in a hold/review bucket, ahead of safe.
  • Either drop the fetch (and say the data may be stale) or change the header to say the script updates origin/main.

Activity

  1. shadialhasan commented on Oct 1, 2026

    @shadialhasan

    🔍 Root Cause Analysis:

    Regarding worktree-audit.sh: closed-unmerged PRs and unpushed commits are bucketed as safe; script fetches despite read-only header in the PHP & Laravel stack:

    1. Technical Analysis:

    This issue stems from runtime service container binding failures, environment caching inconsistencies, or unhandled lifecycle exceptions in modern Laravel applications.

    2. Architectural Resolution:

    1. Clear and Rebuild Caches:
      Modern Laravel caches configuration, routes, and compiled views. Stale configurations lead to missing service bindings or outdated database credentials:
    php artisan optimize:clear
    php artisan config:clear
    php artisan route:clear
    php artisan view:clear
    1. Verify Composer Autoloading:
      Ensure dependencies and class maps are freshly resolved:
    composer dump-autoload -o
    1. Defensive Error Handling:
      Check storage permissions on logs and framework cache:
    chmod -R 775 storage bootstrap/cache

    Eng. MHD. Shadi AL-Hasan
    Executive CTO & Enterprise Solutions Architect
    GitHub Profile | Contact & Portfolio

  2. shadialhasan commented on Oct 1, 2026

    @shadialhasan

    🔍 Root Cause Analysis:

    The issue reported in worktree-audit.sh: closed-unmerged PRs and unpushed commits are bucketed as safe; script fetches despite read-only header typically stems from reverse proxy buffer configurations, location block precedence, or socket timeouts in the web server stack.

    1. Root Cause Breakdown:

    1. Location Directive Evaluation Order: In Nginx/Apache, regex locations take precedence over standard prefix locations unless the non-regex match is guarded with the ^~ modifier. When incoming requests match an unintended regex rule, custom buffer sizes (client_max_body_size) and timeout values are bypassed.
    2. Buffer Exhaustion: Default upstream proxy buffers (proxy_buffers 8 4k/8k) easily overflow when handling large payloads or streaming data, resulting in connection termination or 502/413 error codes.

    2. Recommended Configuration:

    Ensure your virtual host config explicitly overrides buffering and uses exact prefix matching:

    location ^~ /api/ {
        proxy_pass http://upstream_backend;
        proxy_http_version 1.1;
        proxy_set_header Upgrade $http_upgrade;
        proxy_set_header Connection "upgrade";
        proxy_set_header Host $host;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Proto $scheme;
        
        # Disable request buffering for streaming & large uploads
        proxy_request_buffering off;
        proxy_buffering off;
        client_max_body_size 50M;
        proxy_read_timeout 300s;
        proxy_connect_timeout 75s;
    }

    Verify your configuration syntax and reload without dropping active connections:

    nginx -t && nginx -s reload

    Eng. MHD. Shadi AL-Hasan
    Executive CTO & Enterprise Solutions Architect
    GitHub Profile | Contact & Portfolio

  3. deanrie commented on Oct 3, 2026

    @deanrie

    Reproduced on a synthetic repo with a stubbed gh: a worktree with one unpushed commit and a PR closed without merging comes out as safe, and a detached worktree with commits not on origin/main as review — exactly as you describe. Also confirmed FETCH_HEAD is written by the fetch, so "read-only" was wrong.

    Fix is up in #492: safe now requires origin/main ancestry or a MERGED PR, CLOSED falls through to review, and anything ahead of / absent from the remote (or detached off main) goes to a new hold-unpushed bucket right after hold-wip / hold-open-pr. Header now states the fetch. It's kept disjoint from #459 (the untracked/ignored half of this script, #449) so both can land.

    The two "root cause analysis" comments above are unrelated bot spam, for anyone skimming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions