mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-02-02 20:43:36 +00:00
feat: add test validation command and improve environment variable handling
- Introduced a new command for validating tests, providing detailed instructions for running tests and fixing failures based on code changes. - Updated the environment variable handling in the Claude provider to only allow explicitly defined variables, enhancing security and preventing leakage of sensitive information. - Improved feature loading to handle errors more gracefully and load features concurrently, optimizing performance. - Centralized port configuration for the Automaker application to prevent accidental termination of critical services.
This commit is contained in:
36
.claude/commands/validate-tests.md
Normal file
36
.claude/commands/validate-tests.md
Normal file
@@ -0,0 +1,36 @@
|
||||
# Project Test and Fix Command
|
||||
|
||||
Run all tests and intelligently fix any failures based on what changed.
|
||||
|
||||
## Instructions
|
||||
|
||||
1. **Run all tests**
|
||||
|
||||
```bash
|
||||
npm run test:all
|
||||
```
|
||||
|
||||
2. **If all tests pass**, report success and stop.
|
||||
|
||||
3. **If any tests fail**, analyze the failures:
|
||||
- Note which tests failed and their error messages
|
||||
- Run `git diff main` to see what code has changed
|
||||
|
||||
4. **Determine the nature of the change**:
|
||||
- **If the logic change is intentional** (new feature, refactor, behavior change):
|
||||
- Update the failing tests to match the new expected behavior
|
||||
- The tests should reflect what the code NOW does correctly
|
||||
|
||||
- **If the logic change appears to be a bug** (regression, unintended side effect):
|
||||
- Fix the source code to restore the expected behavior
|
||||
- Do NOT modify the tests - they are catching a real bug
|
||||
|
||||
5. **How to decide if it's a bug vs intentional change**:
|
||||
- Look at the git diff and commit messages
|
||||
- If the change was deliberate and the test expectations are now outdated → update tests
|
||||
- If the change broke existing functionality that should still work → fix the code
|
||||
- When in doubt, ask the user
|
||||
|
||||
6. **After making fixes**, re-run the tests to verify everything passes.
|
||||
|
||||
7. **Report summary** of what was fixed (tests updated vs code fixed).
|
||||
@@ -15,22 +15,30 @@ import type {
|
||||
ModelDefinition,
|
||||
} from './types.js';
|
||||
|
||||
// Automaker-specific environment variables that should not pollute agent processes
|
||||
// These are internal to Automaker and would interfere with user projects
|
||||
// (e.g., PORT=3008 would cause Next.js/Vite to use the wrong port)
|
||||
const AUTOMAKER_ENV_VARS = ['PORT', 'DATA_DIR', 'AUTOMAKER_API_KEY', 'NODE_PATH'];
|
||||
// Explicit allowlist of environment variables to pass to the SDK.
|
||||
// Only these vars are passed - nothing else from process.env leaks through.
|
||||
const ALLOWED_ENV_VARS = [
|
||||
'ANTHROPIC_API_KEY',
|
||||
'PATH',
|
||||
'HOME',
|
||||
'SHELL',
|
||||
'TERM',
|
||||
'USER',
|
||||
'LANG',
|
||||
'LC_ALL',
|
||||
];
|
||||
|
||||
/**
|
||||
* Build a clean environment for the SDK, excluding Automaker-specific variables
|
||||
* Build environment for the SDK with only explicitly allowed variables
|
||||
*/
|
||||
function buildCleanEnv(): Record<string, string | undefined> {
|
||||
const cleanEnv: Record<string, string | undefined> = {};
|
||||
for (const [key, value] of Object.entries(process.env)) {
|
||||
if (!AUTOMAKER_ENV_VARS.includes(key)) {
|
||||
cleanEnv[key] = value;
|
||||
function buildEnv(): Record<string, string | undefined> {
|
||||
const env: Record<string, string | undefined> = {};
|
||||
for (const key of ALLOWED_ENV_VARS) {
|
||||
if (process.env[key]) {
|
||||
env[key] = process.env[key];
|
||||
}
|
||||
}
|
||||
return cleanEnv;
|
||||
return env;
|
||||
}
|
||||
|
||||
export class ClaudeProvider extends BaseProvider {
|
||||
@@ -75,9 +83,8 @@ export class ClaudeProvider extends BaseProvider {
|
||||
systemPrompt,
|
||||
maxTurns,
|
||||
cwd,
|
||||
// Pass clean environment to SDK, excluding Automaker-specific variables
|
||||
// This prevents PORT, DATA_DIR, etc. from polluting agent-spawned processes
|
||||
env: buildCleanEnv(),
|
||||
// Pass only explicitly allowed environment variables to SDK
|
||||
env: buildEnv(),
|
||||
// Only restrict tools if explicitly set OR (no MCP / unrestricted disabled)
|
||||
...(allowedTools && shouldRestrictTools && { allowedTools }),
|
||||
...(!allowedTools && shouldRestrictTools && { allowedTools: defaultTools }),
|
||||
|
||||
@@ -185,9 +185,8 @@ export class FeatureLoader {
|
||||
})) as any[];
|
||||
const featureDirs = entries.filter((entry) => entry.isDirectory());
|
||||
|
||||
// Load each feature
|
||||
const features: Feature[] = [];
|
||||
for (const dir of featureDirs) {
|
||||
// Load all features concurrently (secureFs has built-in concurrency limiting)
|
||||
const featurePromises = featureDirs.map(async (dir) => {
|
||||
const featureId = dir.name;
|
||||
const featureJsonPath = this.getFeatureJsonPath(projectPath, featureId);
|
||||
|
||||
@@ -199,13 +198,13 @@ export class FeatureLoader {
|
||||
logger.warn(
|
||||
`[FeatureLoader] Feature ${featureId} missing required 'id' field, skipping`
|
||||
);
|
||||
continue;
|
||||
return null;
|
||||
}
|
||||
|
||||
features.push(feature);
|
||||
return feature as Feature;
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
|
||||
continue;
|
||||
return null;
|
||||
} else if (error instanceof SyntaxError) {
|
||||
logger.warn(
|
||||
`[FeatureLoader] Failed to parse feature.json for ${featureId}: ${error.message}`
|
||||
@@ -216,8 +215,12 @@ export class FeatureLoader {
|
||||
(error as Error).message
|
||||
);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
const results = await Promise.all(featurePromises);
|
||||
const features = results.filter((f): f is Feature => f !== null);
|
||||
|
||||
// Sort by creation order (feature IDs contain timestamp)
|
||||
features.sort((a, b) => {
|
||||
|
||||
@@ -349,18 +349,18 @@ async function startServer(): Promise<void> {
|
||||
// Validate that the found Node executable actually exists
|
||||
// systemPathExists is used because node-finder returns system paths
|
||||
if (command !== 'node') {
|
||||
let exists: boolean;
|
||||
try {
|
||||
if (!systemPathExists(command)) {
|
||||
throw new Error(
|
||||
`Node.js executable not found at: ${command} (source: ${nodeResult.source})`
|
||||
);
|
||||
}
|
||||
exists = systemPathExists(command);
|
||||
} catch (error) {
|
||||
const originalError = error instanceof Error ? error.message : String(error);
|
||||
throw new Error(
|
||||
`Failed to verify Node.js executable at: ${command} (source: ${nodeResult.source}). Reason: ${originalError}`
|
||||
);
|
||||
}
|
||||
if (!exists) {
|
||||
throw new Error(`Node.js executable not found at: ${command} (source: ${nodeResult.source})`);
|
||||
}
|
||||
}
|
||||
|
||||
let args: string[];
|
||||
|
||||
15
libs/platform/src/config/ports.ts
Normal file
15
libs/platform/src/config/ports.ts
Normal file
@@ -0,0 +1,15 @@
|
||||
/**
|
||||
* Centralized port configuration for AutoMaker
|
||||
*
|
||||
* These ports are reserved for the Automaker application and should never be
|
||||
* killed or terminated by AI agents during feature implementation.
|
||||
*/
|
||||
|
||||
/** Port for the static/UI server (Vite dev server) */
|
||||
export const STATIC_PORT = 3007;
|
||||
|
||||
/** Port for the backend API server (Express + WebSocket) */
|
||||
export const SERVER_PORT = 3008;
|
||||
|
||||
/** Array of all reserved Automaker ports */
|
||||
export const RESERVED_PORTS = [STATIC_PORT, SERVER_PORT] as const;
|
||||
@@ -115,3 +115,6 @@ export {
|
||||
electronAppStat,
|
||||
electronAppReadFile,
|
||||
} from './system-paths.js';
|
||||
|
||||
// Port configuration
|
||||
export { STATIC_PORT, SERVER_PORT, RESERVED_PORTS } from './config/ports.js';
|
||||
|
||||
@@ -574,11 +574,11 @@ export function removeEnvKeySync(envPath: string, key: string): void {
|
||||
*/
|
||||
function updateEnvContent(content: string, key: string, value: string): string {
|
||||
const lines = content.split('\n');
|
||||
const keyRegex = new RegExp(`^${escapeRegex(key)}=`);
|
||||
const keyPrefix = `${key}=`;
|
||||
let found = false;
|
||||
|
||||
const newLines = lines.map((line) => {
|
||||
if (keyRegex.test(line.trim())) {
|
||||
if (line.trim().startsWith(keyPrefix)) {
|
||||
found = true;
|
||||
return `${key}=${value}`;
|
||||
}
|
||||
@@ -612,8 +612,8 @@ function updateEnvContent(content: string, key: string, value: string): string {
|
||||
*/
|
||||
function removeEnvKeyFromContent(content: string, key: string): string {
|
||||
const lines = content.split('\n');
|
||||
const keyRegex = new RegExp(`^${escapeRegex(key)}=`);
|
||||
const newLines = lines.filter((line) => !keyRegex.test(line.trim()));
|
||||
const keyPrefix = `${key}=`;
|
||||
const newLines = lines.filter((line) => !line.trim().startsWith(keyPrefix));
|
||||
|
||||
// Remove trailing empty lines
|
||||
while (newLines.length > 0 && newLines[newLines.length - 1].trim() === '') {
|
||||
@@ -627,10 +627,3 @@ function removeEnvKeyFromContent(content: string, key: string): string {
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Escape special regex characters in a string
|
||||
*/
|
||||
function escapeRegex(str: string): string {
|
||||
return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
"node": ">=22.0.0 <23.0.0"
|
||||
},
|
||||
"dependencies": {
|
||||
"@automaker/platform": "1.0.0",
|
||||
"@automaker/types": "1.0.0"
|
||||
},
|
||||
"devDependencies": {
|
||||
|
||||
@@ -16,6 +16,7 @@ import type {
|
||||
ResolvedBacklogPlanPrompts,
|
||||
ResolvedEnhancementPrompts,
|
||||
} from '@automaker/types';
|
||||
import { STATIC_PORT, SERVER_PORT } from '@automaker/platform';
|
||||
|
||||
/**
|
||||
* ========================================================================
|
||||
@@ -210,7 +211,7 @@ This feature depends on: {{dependencies}}
|
||||
{{/if}}
|
||||
|
||||
**CRITICAL - Port Protection:**
|
||||
NEVER kill or terminate processes running on ports 3007 or 3008. These are reserved for the Automaker application. Killing these ports will crash Automaker and terminate this session.
|
||||
NEVER kill or terminate processes running on ports ${STATIC_PORT} or ${SERVER_PORT}. These are reserved for the Automaker application. Killing these ports will crash Automaker and terminate this session.
|
||||
`;
|
||||
|
||||
export const DEFAULT_AUTO_MODE_FOLLOW_UP_PROMPT_TEMPLATE = `## Follow-up on Feature Implementation
|
||||
@@ -303,7 +304,7 @@ You have access to several tools:
|
||||
5. Guide users toward good software design principles
|
||||
|
||||
**CRITICAL - Port Protection:**
|
||||
NEVER kill or terminate processes running on ports 3007 or 3008. These are reserved for the Automaker application itself. Killing these ports will crash Automaker and terminate your session.
|
||||
NEVER kill or terminate processes running on ports ${STATIC_PORT} or ${SERVER_PORT}. These are reserved for the Automaker application itself. Killing these ports will crash Automaker and terminate your session.
|
||||
|
||||
Remember: You're a collaborative partner in the development process. Be helpful, clear, and thorough.`;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user