Skip to content

fix(ci): tidy stop-session.sh scans, listings and arguments - #8724

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/stop-session-nits
Oct 7, 2026
Merged

waleedlatif1 merged 1 commit into
stagingfrom
fix/stop-session-nits

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • PIDs from the session scan and the tag scan are deduped. ps pads PIDs with spaces, so sort -u missed repeats.
  • The "outlived its leader" heading prints only when the same scan has rows to list under it.
  • The deadline check also requires two empty scans 0.1s apart, like the polling loop.
  • An empty tag or a non-numeric leader is refused (exit 2) instead of matching E2E_APP=.
  • The expected-flush label sits in its own column, so rows stay aligned with the ps header.
  • /proc reads for processes that exit mid-scan no longer print "No such file or directory".

Type of Change

  • Bug fix

Testing

  • Container checks in node:22-bookworm: 14 of 14 pass. That covers the earlier literal-match, self-exclusion and label checks, plus these new ones:
    • PID columns line up under the header;
    • the flush is listed once;
    • an empty tag or a missing leader exits 2 and stops nothing;
    • the first SIGTERM names each PID once.
  • The Linux session checks still pass. A member that ignores SIGTERM and a leader that ignores SIGTERM are each SIGKILLed at about 10s, and nothing is left running.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

- dedupe PIDs from the session and tag scans (`ps` pads them, so `sort -u` missed repeats)
- print the "outlived its leader" heading only with the rows of the same scan under it
- apply the two-empty-scans rule at the deadline too
- refuse an empty tag or a non-numeric leader instead of matching `E2E_APP=`
- keep the expected-flush label in its own column so rows stay aligned with the ps header
- silence /proc reads for processes that exit mid-scan
@vercel

vercel Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Oct 7, 2026 5:17am UTC

Request Review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors a CI session cleanup script with validation and output changes.

The PR appears safe to merge; no actionable defects were found.

What we checked:

  • Distinct PIDs remain visible: Both scans produce numeric PIDs. Removing spaces and sorting numerically merges duplicate PIDs, not different processes.
  • Deadline success needs two scans: At the deadline, a live-process result fails immediately. An empty result must stay empty after a 0.1-second wait.
  • CI callers pass valid arguments: All four workflow callers pass the background process PID and a nonempty run-specific tag.

Summary

Updates .github/scripts/stop-session.sh to make CI cleanup scans and logs more reliable.

  • Rejects missing arguments, nonnumeric leaders, and empty tags.
  • Deduplicates PIDs from the session and tag scans.
  • Prints leftover-process headings with rows from the same scan and aligns telemetry labels.
  • Requires two empty scans at the deadline and hides expected errors when processes exit mid-scan.

No actionable issues found. This review checked the code and workflow callers; it did not run the script.

Reviews (1) · Last reviewed commit: "fix(ci): tidy stop-session.sh scans, lis..." · Reviewed by Greptile

@waleedlatif1
waleedlatif1 merged commit 48cbd4f into staging Oct 7, 2026
59 of 60 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/stop-session-nits branch October 7, 2026 07:15

This branch was successfully deployed

1 active deployment
Preview — a3b78614 Deployed Oct 7, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant