Skip to content

Address jcode-dev Claude review findings#7

Open
ShawnSantiago wants to merge 1 commit into
masterfrom
fix/jcode-dev-claude-review-20260521
Open

Address jcode-dev Claude review findings#7
ShawnSantiago wants to merge 1 commit into
masterfrom
fix/jcode-dev-claude-review-20260521

Conversation

@ShawnSantiago
Copy link
Copy Markdown
Owner

Summary

  • Honor CARGO_TARGET_DIR when locating the built jcode-dev binary.
  • Preserve the caller working directory for JCODE_DEV_USE_SELFDEV=1.
  • Raise the dev RUST_MIN_STACK default to 64 MiB per rustc's recommendation on fresh target builds.

Validation

  • bash -n scripts/dev_cargo.sh scripts/jcode-dev.sh
  • env -u RUST_MIN_STACK scripts/dev_cargo.sh --print-setup | grep 'rust_min_stack=67108864'
  • cd /home/shawn/Documents/projects/constructive-toys-headless && jcode-dev --version
  • scripts/dev_cargo.sh test -p jcode-overnight-core
  • git diff --check origin/master...HEAD

Notes

  • Claude review found no blockers. This addresses the medium CARGO_TARGET_DIR finding and the low selfdev caller-cwd finding.
  • A fresh custom CARGO_TARGET_DIR build progressed past the prior rustc stack SIGSEGV with 64 MiB, then hit a separate local rust-lld/toolchain link failure: unknown section type 0x24 in an AWS SDK rlib. Default target builds validate successfully.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request increases the default RUST_MIN_STACK to 64 MiB in dev_cargo.sh to improve build resilience against stack overflow crashes. Additionally, jcode-dev.sh is updated to ensure the correct working directory is set before executing selfdev and to properly resolve the binary path when a custom CARGO_TARGET_DIR is specified. I have no feedback to provide as there were no review comments.

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