Conversation
…ate_safe_outputs_tools Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3ba6b526-f9a4-4964-aac7-3d4196459928 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…e-throw Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3ba6b526-f9a4-4964-aac7-3d4196459928 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR brings two safe-output setup/handler scripts into USE-001 conformance by ensuring thrown errors include standardized ERR_* prefixes for structured logging.
Changes:
- Added
ERR_CONFIGtogenerate_safe_outputs_tools.cjsand prefixed missing-file error messages. - Added
ERR_APItoclose_entity_helpers.cjsand replaced a bare rethrow with a wrappedErrorthat includes an error code and preserves the original error viacause.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/generate_safe_outputs_tools.cjs | Prefixes missing source/config file errors with ERR_CONFIG for standardized error codes. |
| actions/setup/js/close_entity_helpers.cjs | Wraps comment-posting failures with an ERR_API-prefixed error and attaches the original error as cause. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
| if (!fs.existsSync(toolsSourcePath)) { | ||
| const msg = `Error: Source tools file not found at: ${toolsSourcePath}`; | ||
| const msg = `${ERR_CONFIG}: Source tools file not found at: ${toolsSourcePath}`; | ||
| console.error(msg); | ||
| throw new Error(msg); |
There was a problem hiding this comment.
This change introduces a standardized ERR_CONFIG prefix for missing-file errors, but the existing unit tests for the missing source/config file cases only assert that the script throws (they don’t assert the error message starts with ERR_CONFIG:). Adding an assertion for the thrown error message would prevent regressions in USE-001 conformance.
| } else { | ||
| throw commentError; | ||
| throw new Error(`${ERR_API}: Failed to add comment to ${entityConfig.displayName} #${entityNumber}: ${errorMsg}`, { cause: commentError }); | ||
| } |
There was a problem hiding this comment.
The new wrapped throw (with ${ERR_API}: ... and { cause: commentError }) isn’t covered by tests. Since this file already has a Vitest suite, add a unit test that forces callbacks.addComment to reject when continueOnCommentError is false and assert the returned error message includes the ERR_API: prefix (and optionally that the thrown Error has the expected cause).
Two handlers were throwing bare errors without the
ERR_*prefixes required byerror_codes.cjs, breaking structured logging and alerting rules (conformance check USE-001).Changes
close_entity_helpers.cjs: AddedERR_APIimport; replaced barethrow commentErrorwith a wrapped error that includes theERR_APIprefix and preserves the original as{ cause: commentError }for stack trace retention.generate_safe_outputs_tools.cjs: AddedERR_CONFIGimport; prefixed boththrow new Error(msg)calls (missing source tools file, missing config file) withERR_CONFIG.