Skip to content

fix: address comprehensive code review findings - #23

Merged
nordicnode merged 1 commit into
mainfrom
fix/review-findings
Aug 15, 2026
Merged

fix: address comprehensive code review findings#23
nordicnode merged 1 commit into
mainfrom
fix/review-findings

Conversation

@nordicnode

Copy link
Copy Markdown
Owner

fix: address comprehensive code review findings

Harden verify output purity, fix render/diff edge cases, and clean up
repo hygiene:

  • verify --verbose now routes the introspection block to stderr for every
    non-human format (github/junit included), keeping machine-readable stdout
    parseable
  • add Trae to the --help ecosystem list and correct the README target count
    (18 -> 19)
  • gitignore and remove committed agent-benchmark result artifacts (they
    contained machine-specific absolute paths), delete a stray = dir, and
    extend the CI package-content guard to block both
  • dedupe ensure_no_symlink_ancestors into generate.rs, shared by
    init/update/--fix so the symlink-escape hardening can't drift
  • time-box the verify_stdin write in spawn.rs (a child that never reads
    stdin can no longer hang the probe past HELP_TIMEOUT); regression test
  • diff reports trailing-newline-only drift instead of "(no lines differ)"
  • verify --fix suggestions point at skillpack.toml for config-driven checks
    (when_to_use/allowed_tools/plugin fields) so users don't loop on a no-op
  • escape tabs in YAML descriptions and quotes/backslashes in glob lists;
    --target list honors --format json; strict is_json(); widen the watch
    noise filter; fix the misleading Shell/Powershell detection comment

Harden verify output purity, fix render/diff edge cases, and clean up
repo hygiene:

- verify --verbose now routes the introspection block to stderr for every
  non-human format (github/junit included), keeping machine-readable stdout
  parseable
- add Trae to the --help ecosystem list and correct the README target count
  (18 -> 19)
- gitignore and remove committed agent-benchmark result artifacts (they
  contained machine-specific absolute paths), delete a stray `=` dir, and
  extend the CI package-content guard to block both
- dedupe ensure_no_symlink_ancestors into generate.rs, shared by
  init/update/--fix so the symlink-escape hardening can't drift
- time-box the verify_stdin write in spawn.rs (a child that never reads
  stdin can no longer hang the probe past HELP_TIMEOUT); regression test
- diff reports trailing-newline-only drift instead of "(no lines differ)"
- verify --fix suggestions point at skillpack.toml for config-driven checks
  (when_to_use/allowed_tools/plugin fields) so users don't loop on a no-op
- escape tabs in YAML descriptions and quotes/backslashes in glob lists;
  --target list honors --format json; strict is_json(); widen the watch
  noise filter; fix the misleading Shell/Powershell detection comment

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@nordicnode
nordicnode merged commit 03d5857 into main Aug 15, 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.

1 participant