Conversation
The process.env.ANTHROPIC_API_KEY and process.env.GITHUB_TOKEN writes were removed in eb13672 but re-introduced during merge conflict resolution in c204ade. runReview() receives credentials as parameters and never reads process.env, so these writes are unnecessary and expose secrets to child processes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 Dialectic PR ReviewFramework: vanilla SummaryThe removal of environment variable bridging appears to be a breaking change that could cause the review-engine to fail. This needs verification that the downstream dependency has been updated accordingly. Issues🐛 Environment variables no longer set for review-engineFile: The code removes the bridging logic that sets process.env.ANTHROPIC_API_KEY and process.env.GITHUB_TOKEN from GitHub Actions inputs. If review-engine validates process.env directly as the comment indicates, this removal will cause the review-engine to fail due to missing environment variables. Suggestion: Verify that review-engine has been updated to accept these values through a different mechanism, or restore the environment variable bridging logic to maintain compatibility. Metadata
Powered by Dialectic PR Review |
Add safeError() helper that extracts only error.message instead of stringifying full error objects, which could contain Authorization headers or SDK internals. Applied across all logger.error/warn calls in github-api, claude-api, consensus-engine, cli, detector, and config-loader. Also removed debug log that dumped raw Claude response text on parse failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 Dialectic PR ReviewFramework: vanilla SummaryThe changes introduce a safeError utility for consistent error message handling, which is a good security practice. However, there are two potential breaking changes: the APIError constructor signature change and removal of environment variable bridging code that may cause runtime failures. Issues🐛 APIError constructor signature mismatchFile: The APIError constructor is being called with 2 arguments (status, message) but the original code passed 3 arguments (status, message, error). This change removes the original error object from the APIError, which may break error handling or debugging capabilities downstream. Suggestion: Verify that APIError constructor only expects 2 parameters, or preserve the original error object as a third parameter if the constructor supports it. 🐛 Environment variables no longer set for review engineFile: The code that sets process.env.ANTHROPIC_API_KEY and process.env.GITHUB_TOKEN has been removed. The comment indicates that review-engine validates process.env directly, so removing this bridging code may cause the review engine to fail when accessing these environment variables. Suggestion: Ensure that the review engine has been updated to accept these values through a different mechanism, or restore the environment variable setting if the engine still depends on process.env. Metadata
Powered by Dialectic PR Review |
Rewrite prompts and output formatting to restore the proven review output structure: numbered issues in STEP 1/2, Executive Summary with Critical/Important/Positive sections in STEP 3, bullet-list metrics, simplified consensus JSON (agreed_issues/rejected_issues/verdict/ false_positive_rate), and proper footer with tip. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 Dialectic PR Review✅ ✅ No significant issues found. Code looks good. |
1 participant
The process.env.ANTHROPIC_API_KEY and process.env.GITHUB_TOKEN writes were removed in eb13672 but re-introduced during merge conflict resolution in c204ade. runReview() receives credentials as parameters and never reads process.env, so these writes are unnecessary and expose secrets to child processes.