Skip to content

fix(ci): strip trailing slash from sample_dir path variable#693

Merged
diberry merged 1 commit intobradygaster:devfrom
diberry:squad/fix-samples-path-slash
Mar 29, 2026
Merged

fix(ci): strip trailing slash from sample_dir path variable#693
diberry merged 1 commit intobradygaster:devfrom
diberry:squad/fix-samples-path-slash

Conversation

@diberry
Copy link
Copy Markdown
Collaborator

@diberry diberry commented Mar 29, 2026

What

Strips trailing slash from sample_dir in samples-build CI job.

Why

Copilot code review found that the glob pattern samples/*/ produces paths with trailing slash. When interpolated into require('.//package.json'), this creates ./samples/name//package.json (double slash). While Unix tolerates this, it's incorrect and could fail on some systems.

How

Adds \ to strip trailing slash after the for loop iterator.

Copilot Review Finding

This was identified by automated Copilot code review on the merged upstream dev branch after PRs #690, #691, #692 were squash-merged.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Fixes double-slash in path construction (./samples/name//package.json)
found by Copilot code review. The glob samples/*/ produces paths with
trailing slash which caused redundant // in require() paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 29, 2026 18:40
@diberry
Copy link
Copy Markdown
Collaborator Author

diberry commented Mar 29, 2026

🤖 Copilot Code Review Finding

Issue: Shell variable expansion inside double-quoted Node.js string
Severity: Low (Unix tolerates //, but incorrect)
Problem: \ from \samples/*/\ glob includes trailing slash. Interpolation into
equire('.//package.json')\ produces ./samples/name//package.json.
Fix: Strip trailing slash with \\ after the for loop iterator.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the samples-build CI job by normalizing sample_dir values produced by the samples/*/ glob so downstream path interpolation doesn’t produce double slashes.

Changes:

  • Strip the trailing / from sample_dir inside the samples loop to prevent paths like ./samples/name//package.json.

@diberry diberry merged commit 428a20d into bradygaster:dev Mar 29, 2026
13 checks passed
robzelt pushed a commit to robzelt/squad that referenced this pull request Apr 1, 2026
…trol-docs

docs: add remote control (squad start --tunnel) documentation
robzelt pushed a commit to robzelt/squad that referenced this pull request Apr 1, 2026
Session: 2026-03-03T20-42-41Z-pr693-merge
Requested by: Brady

Changes:
- Logged Kobayashi orchestration (PR bradygaster#693 merge to main, version conflict resolution)
- Logged PR bradygaster#693 merge session
- Merged decision inbox (hockney-docker-fix) into decisions.md
- Deleted inbox file

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

2 participants