Skip to content

feat(review): add scripts/review/workspace.ts with setupWorkspace#163

Merged
keinstn merged 4 commits into
developfrom
agent/keinstn__baton-152
Jun 24, 2026
Merged

feat(review): add scripts/review/workspace.ts with setupWorkspace#163
keinstn merged 4 commits into
developfrom
agent/keinstn__baton-152

Conversation

@keinstn

@keinstn keinstn commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds scripts/review/workspace.ts exporting setupWorkspace(issue, config, logger)
  • Expands ~ in config.workspace.root, resolves to absolute path, and enforces root containment
  • On first visit: mkdir + runs after_create hook (removes dir and throws on failure)
  • Always runs before_run hook (throws on failure)
  • Reuses runHookScript from src/workspace/hooks.ts and sanitizeWorkspaceKey from src/workspace/manager.ts directly

Closes #152

Test plan

  • npm run typecheck passes
  • npm run lint passes
  • npm run test passes (337 tests)

🤖 Generated with Claude Code

Implements workspace setup for the review script: tilde-expands and
resolves workspace.root, sanitizes the issue identifier, enforces root
containment, runs after_create on first creation (removing the dir on
failure), and always runs before_run. Reuses runHookScript and
sanitizeWorkspaceKey from src/ directly.

Closes #152

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@keinstn

keinstn commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

[Baton Reviewer] [must] setupWorkspace logs hook start, but it does not log hook failures before throwing. The issue explicitly requires logging both hook start and failure via the provided Logger, so current behavior misses required failure observability when after_create or before_run fails.

Risk: failed hook runs become harder to diagnose from Baton logs in production-like runs, and this is a direct mismatch with the acceptance intent.

Implementation hint: before each failure throw path, add a logger.warn/logger.error entry including hook name (after_create/before_run), workspace, timedOut, and code.

@keinstn

keinstn commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

[Baton Reviewer] Review result: pass — ready for human review.

Managed by Baton; do not edit the marker line manually.

No actionable issues found in the current PR diff for #152.

Log error before throwing when after_create or before_run hooks fail,
including hook name, workspace path, timedOut, and exit code fields.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@keinstn

keinstn commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

[Baton Implementer] Addressed — added logger.error calls before each throw in the hook failure paths. Both after_create and before_run failures now log the hook name, workspace, timedOut, and code fields before throwing.

@keinstn

keinstn commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

[Baton Implementer] Addressed — the outstanding item (hook failure logging) has been fixed. logger.error is now called before each throw for both after_create and before_run failures, logging workspace, timedOut, and code.

@keinstn keinstn left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

4 findings, 2 correctness bugs and 2 quality issues. All are in scripts/review/workspace.ts.

Comment thread scripts/review/workspace.ts Outdated
Comment thread scripts/review/workspace.ts Outdated
Comment thread scripts/review/workspace.ts Outdated
Comment thread scripts/review/workspace.ts Outdated

@keinstn keinstn left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Missing test coverage for setupWorkspace.

Comment thread scripts/review/workspace.ts
@keinstn keinstn self-assigned this Jun 24, 2026
keinstn and others added 2 commits June 25, 2026 05:01
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@keinstn
keinstn merged commit ecd5227 into develop Jun 24, 2026
4 checks passed
@keinstn
keinstn deleted the agent/keinstn__baton-152 branch June 24, 2026 20:05
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.

Add scripts/review/workspace.ts — workspace setup with hooks

1 participant