Skip to content

Conversation

@konard
Copy link
Member

@konard konard commented Jan 10, 2026

Summary

This PR documents the Array.join() pitfall discovered in production (hive-mind#1096) and adds comprehensive best practices documentation.

The Problem

When using command-stream, calling .join(' ') on an array before template interpolation causes all arguments to be merged into a single shell argument:

// WRONG - all arguments become ONE shell argument
const args = ['file.txt', '--public', '--verbose'];
await $`command ${args.join(' ')}`;
// Error: File does not exist: "file.txt --public --verbose"

// CORRECT - each element is a separate argument
await $`command ${args}`;
// Works correctly

Changes

  • js/BEST-PRACTICES.md: Comprehensive best practices guide covering array handling, string interpolation, security, error handling, and common pitfalls
  • README.md: Added "Common Pitfalls" section explaining the Array.join() issue with examples
  • js/docs/case-studies/issue-153/: Detailed investigation document with real-world bug analysis
  • js/docs/: Moved all JS-related documentation here (IMPLEMENTATION_NOTES.md, SHELL_OPERATORS_IMPLEMENTATION.md)
  • rust/BEST-PRACTICES.md: Rust-specific best practices guide
  • js/tests/array-interpolation.test.mjs: 34 tests covering:
    • Direct array passing (correct usage)
    • Pre-joined array (anti-pattern demonstration)
    • Mixed interpolation patterns
    • Real-world use cases (git, npm, docker, rsync)
    • Edge cases and documentation examples verification
  • package.json: Version bump to 0.9.3

File Structure Reorganization

Per feedback, this PR also reorganizes the file structure to separate JavaScript and Rust documentation:

  • Moved BEST-PRACTICES.mdjs/BEST-PRACTICES.md
  • Moved docs/case-studies/js/docs/case-studies/
  • Moved other JS docs from docs/js/docs/
  • Updated all cross-references in README.md, test files, and documentation

Test Plan

  • All new tests pass (34 tests added)
  • Existing tests pass (679 pass, 5 skip, 1 flaky timing-related test)
  • Lint checks pass (bun run lint)
  • Format checks pass (bun run format:check)
  • All CI checks passing

Fixes #153


🤖 Generated with Claude Code

Adding CLAUDE.md with task information for AI processing.
This file will be removed when the task is complete.

Issue: #153
@konard konard self-assigned this Jan 10, 2026
This commit adds comprehensive documentation about the Array.join() pitfall
where calling .join(' ') before template interpolation causes all arguments
to be merged into a single shell argument.

Changes:
- Add BEST-PRACTICES.md with detailed usage patterns
- Add Common Pitfalls section to README.md explaining the issue
- Add docs/case-studies/issue-153/ with real-world investigation
- Add rust/BEST-PRACTICES.md for Rust-specific patterns
- Add js/tests/array-interpolation.test.mjs with 34 tests covering:
  - Direct array passing (correct usage)
  - Pre-joined array (anti-pattern demonstration)
  - Mixed interpolation patterns
  - Real-world use cases (git, npm, docker, rsync)
  - Edge cases and documentation examples verification
- Bump version to 0.9.3

The bug was discovered in production (hive-mind#1096) where CLI arguments
were incorrectly merged into a file path, causing errors like:
"File does not exist: /path/to/file.txt --public --verbose"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@konard konard changed the title [WIP] Documentation: Array.join() pitfall causes arguments to merge into single string docs: Document Array.join() pitfall and add best practices (fixes #153) Jan 10, 2026
@konard konard marked this pull request as ready for review January 10, 2026 21:59
konard and others added 2 commits January 10, 2026 23:02
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@konard
Copy link
Member Author

konard commented Jan 10, 2026

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost estimation:

  • Public pricing estimate: $10.123341 USD
  • Calculated by Anthropic: $6.069766 USD
  • Difference: $-4.053574 (-40.04%)
    📎 Log file uploaded as GitHub Gist (1080KB)
    🔗 View complete solution draft log

Now working session is ended, feel free to review and add any feedback on the solution draft.

@konard
Copy link
Member Author

konard commented Jan 10, 2026

I think we should not mix JavaScript and Rust docs together, all JavaScript related files and docs should be in ./js folder, and all Rust related docs and files should be in ./rust folder.

We also should move package.json and other configuration file most JavaScript related files to its folder.

So after this pull request file structure will be perfect.

@konard konard marked this pull request as draft January 10, 2026 22:06
@konard
Copy link
Member Author

konard commented Jan 10, 2026

🤖 AI Work Session Started

Starting automated work session at 2026-01-10T22:06:11.397Z

The PR has been converted to draft mode while work is in progress.

This comment marks the beginning of an AI work session. Please wait working session to finish, and provide your feedback.

- Move BEST-PRACTICES.md to js/BEST-PRACTICES.md
- Move docs/case-studies/ to js/docs/case-studies/
- Move docs/IMPLEMENTATION_NOTES.md to js/docs/
- Move docs/SHELL_OPERATORS_IMPLEMENTATION.md to js/docs/
- Update all cross-references in README.md, test files, and docs
- Update changeset description to reflect new file locations

This separates JavaScript documentation from the root level, keeping
all JS-related files in the js/ folder and Rust-related files in rust/.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@konard konard marked this pull request as ready for review January 10, 2026 22:13
@konard
Copy link
Member Author

konard commented Jan 10, 2026

I've addressed the feedback about file structure reorganization:

Changes Made:

  1. Moved JS documentation to js/ folder:

    • BEST-PRACTICES.mdjs/BEST-PRACTICES.md
    • docs/case-studies/js/docs/case-studies/
    • docs/IMPLEMENTATION_NOTES.mdjs/docs/IMPLEMENTATION_NOTES.md
    • docs/SHELL_OPERATORS_IMPLEMENTATION.mdjs/docs/SHELL_OPERATORS_IMPLEMENTATION.md
    • docs/shell-operators-implementation.mdjs/docs/shell-operators-implementation.md
  2. Updated all cross-references:

    • README.md now points to js/BEST-PRACTICES.md
    • Test files updated to reference new paths
    • Internal documentation links fixed
  3. Kept package.json at root: This is required for npm publishing to work correctly. Moving it to js/ would break the npm package.

Current Structure:

├── js/
│   ├── BEST-PRACTICES.md    # JS best practices
│   ├── docs/                 # JS documentation
│   │   ├── case-studies/     # Bug investigations
│   │   └── *.md              # Implementation notes
│   ├── examples/             # JS examples
│   ├── src/                  # JS source code
│   └── tests/                # JS tests
├── rust/
│   ├── BEST-PRACTICES.md    # Rust best practices
│   ├── src/                  # Rust source code
│   └── tests/                # Rust tests
├── package.json              # Required at root for npm
└── README.md                 # Main readme

All CI checks are passing ✅

@konard
Copy link
Member Author

konard commented Jan 10, 2026

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost estimation:

  • Public pricing estimate: $4.120917 USD
  • Calculated by Anthropic: $2.436078 USD
  • Difference: $-1.684839 (-40.89%)
    📎 Log file uploaded as GitHub Gist (560KB)
    🔗 View complete solution draft log

Now working session is ended, feel free to review and add any feedback on the solution draft.

@konard konard merged commit d8140d9 into main Jan 11, 2026
11 checks passed
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.

Documentation: Array.join() pitfall causes arguments to merge into single string

2 participants