Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,7 @@
language: "en-US"

tone_instructions: |
You are a Senior Software Engineer and Security Specialist.
When reviewing, focus on:
1. **Security**: Ensure no secrets are leaked and input validation is robust (especially for URLs and ports).
2. **Performance**: Look for inefficient async operations (e.g., Promise.all vs. Promise.allSettled).
3. **TUI/UX**: Since this is a CLI tool, ensure the interactive flows are intuitive and handle edge cases like cancellations.
4. **Test Coverage**: Ensure all new logic is covered by vitest unit tests.
Senior Engineer & Security Specialist. Review for: 1. Security (no leaks, URL/port validation). 2. Performance (prefer Promise.allSettled). 3. TUI/UX (intuitive CLI flows, handle cancellations). 4. Tests (full vitest coverage for new logic).

reviews:
profile: "assertive"
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
- name: Checkout code
uses: actions/checkout@v4
with:
token: ${{ secrets.GH_PAT }}
fetch-depth: 0

- name: Setup Node.js
uses: actions/setup-node@v4
Expand Down
193 changes: 0 additions & 193 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
"dockerode": "^4.0.0",
"ink": "^4.4.1",
"nodemailer": "^8.0.7",
"ora": "^8.0.1",
"react": "^18.2.0"
},
"devDependencies": {
Expand Down
7 changes: 7 additions & 0 deletions src/__tests__/health.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ vi.mock('../utils/logger', () => ({
},
}));

vi.mock('../ui/spinner', () => ({
createSpinner: vi.fn(() => ({
start: vi.fn().mockReturnThis(),
stop: vi.fn().mockReturnThis(),
})),
}));
Comment on lines +12 to +17
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add fail method to spinner mock for error-path coverage.

The health command uses spinner.fail() in its catch block (line 16 in health.ts), but the mock only provides start and stop. If you add error-path tests, the mock will be incomplete.

♻️ Extend the mock
 vi.mock('../ui/spinner', () => ({
   createSpinner: vi.fn(() => ({
     start: vi.fn().mockReturnThis(),
     stop: vi.fn().mockReturnThis(),
+    fail: vi.fn().mockReturnThis(),
   })),
 }));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/health.test.ts` around lines 12 - 17, The spinner mock created
in the test only implements start and stop but the health command calls
spinner.fail() on errors; update the mock returned by createSpinner in
src/__tests__/health.test.ts to also include a fail method (e.g.,
vi.fn().mockReturnThis()) so tests exercising the error path won't throw due to
a missing method; ensure the mock's createSpinner remains a vi.fn and
consistently returns an object with start, stop, and fail that each return this.


describe('health command', () => {
let program: Command;

Expand Down
7 changes: 7 additions & 0 deletions src/__tests__/logs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ vi.mock('../utils/logger', () => ({
},
}));

vi.mock('../ui/spinner', () => ({
createSpinner: vi.fn(() => ({
start: vi.fn().mockReturnThis(),
stop: vi.fn().mockReturnThis(),
})),
}));
Comment on lines +12 to +17
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add fail method to spinner mock for error-path coverage.

The logs command uses spinner.fail() in its catch block (line 16 in logs.ts), but the mock only provides start and stop.

♻️ Extend the mock
 vi.mock('../ui/spinner', () => ({
   createSpinner: vi.fn(() => ({
     start: vi.fn().mockReturnThis(),
     stop: vi.fn().mockReturnThis(),
+    fail: vi.fn().mockReturnThis(),
   })),
 }));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
vi.mock('../ui/spinner', () => ({
createSpinner: vi.fn(() => ({
start: vi.fn().mockReturnThis(),
stop: vi.fn().mockReturnThis(),
})),
}));
vi.mock('../ui/spinner', () => ({
createSpinner: vi.fn(() => ({
start: vi.fn().mockReturnThis(),
stop: vi.fn().mockReturnThis(),
fail: vi.fn().mockReturnThis(),
})),
}));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/logs.test.ts` around lines 12 - 17, The spinner mock in the
test only implements start and stop but the logs command uses spinner.fail() in
its catch path; update the vi.mock for createSpinner in
src/__tests__/logs.test.ts to include a fail method (e.g., fail:
vi.fn().mockReturnThis()) so the mock supports createSpinner().start(), .stop(),
and .fail(), ensuring the error-path in the logs command can be exercised
without throwing undefined method errors.


describe('logs command', () => {
let program: Command;

Expand Down
4 changes: 4 additions & 0 deletions src/__tests__/show.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ vi.mock('../ui/spinner', () => ({
createSpinner: vi.fn(() => ({
start: vi.fn().mockReturnThis(),
stop: vi.fn().mockReturnThis(),
fail: vi.fn().mockReturnThis(),
warn: vi.fn().mockReturnThis(),
info: vi.fn().mockReturnThis(),
update: vi.fn().mockReturnThis(),
})),
}));
vi.mock('../ui/table', () => ({
Expand Down
15 changes: 13 additions & 2 deletions src/commands/health.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
import { Command } from 'commander';
import { logger } from '../utils/logger';
import { createSpinner } from '../ui/spinner';

export const registerHealthCommand = (program: Command) => {
program
.command('health <target>')
.description('Show health status for pods or containers')
.action((target) => {
logger.info(`Showing health for ${target}...`);
.action(async (target) => {
const spinner = createSpinner(`Checking health for ${target}...`).start();
try {
// TODO: Implement actual health check logic
spinner.stop(`Health check for ${target} complete`);
Comment on lines +9 to +13
logger.info(`Showing health for ${target}...`);
} catch (error) {
const errorMessage = (error as Error).message;
spinner.fail(`Health check for ${target} failed: ${errorMessage}`);
logger.error(`Health check for ${target} failed: ${errorMessage}`, error);
throw error;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
};
15 changes: 13 additions & 2 deletions src/commands/logs.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
import { Command } from 'commander';
import { logger } from '../utils/logger';
import { createSpinner } from '../ui/spinner';

export const registerLogsCommand = (program: Command) => {
program
.command('logs <name>')
.description('Show logs for a container or pod')
.action((name) => {
logger.info(`Showing logs for ${name}...`);
.action(async (name) => {
const spinner = createSpinner(`Fetching logs for ${name}...`).start();
try {
// TODO: Implement actual log fetching logic
spinner.stop(`Logs for ${name} fetched`);
Comment on lines +9 to +13
logger.info(`Showing logs for ${name}...`);
} catch (error) {
const errorMessage = (error as Error).message;
spinner.fail(`Failed to fetch logs for ${name}: ${errorMessage}`);
logger.error(`Failed to fetch logs for ${name}: ${errorMessage}`, error);
throw error;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
};
Loading
Loading