Skip to content

Commit 90ec75a

Browse files
committed
fix: address CodeRabbit review comments
- husky pre-commit: respect existing NVM_DIR with parameter expansion fallback - base.ts: gate error emission on emitResponseChunks to avoid surfacing internal "thinking" operation errors to users Signed-off-by: ibolton336 <ibolton@redhat.com>
1 parent a1160d2 commit 90ec75a

8 files changed

Lines changed: 288 additions & 69 deletions

File tree

.husky/pre-commit

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,7 @@
22
#
33
# Auto-fix any list errors in staged files
44
#
5-
npx lint-staged
5+
export NVM_DIR="${NVM_DIR:-$HOME/.nvm}"
6+
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
67

8+
npx lint-staged

agentic/src/nodes/base.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,13 @@ export abstract class BaseNode extends KaiWorkflowEventEmitter {
156156
errorStack: err instanceof Error ? err.stack : undefined,
157157
},
158158
);
159-
this.emitWorkflowMessage({
160-
id: messageId,
161-
type: KaiWorkflowMessageType.Error,
162-
data: `Failed to get llm response - ${String(err)}`,
163-
});
159+
if (emitResponseChunks) {
160+
this.emitWorkflowMessage({
161+
id: messageId,
162+
type: KaiWorkflowMessageType.Error,
163+
data: `Failed to get llm response - ${String(err)}`,
164+
});
165+
}
164166
} finally {
165167
if (timeoutId !== undefined) {
166168
clearTimeout(timeoutId);

tests/e2e/pages/vscode-web.page.ts

Lines changed: 257 additions & 56 deletions
Large diffs are not rendered by default.

tests/e2e/pages/vscode.page.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,11 @@ export abstract class VSCode {
199199
await runAnalysisBtnLocator.click();
200200

201201
console.log('Waiting for analysis progress indicator...');
202-
await expect(analysisView.getByText('Analysis Progress').first()).toBeVisible();
202+
await expect(
203+
analysisView.getByRole('button', {
204+
name: 'Analyzing...',
205+
})
206+
).toBeDisabled();
203207
console.log('Analysis started successfully');
204208
} catch (error) {
205209
console.log('Error running analysis:', error);
@@ -699,7 +703,8 @@ export abstract class VSCode {
699703
public async executeTerminalCommand(
700704
command: string,
701705
expectedOutput?: string | RegExp,
702-
outputShouldBeVisible: boolean = true
706+
outputShouldBeVisible: boolean = true,
707+
timeout: number = 30_000
703708
): Promise<void> {
704709
if (!this.repoDir || !this.branch) {
705710
throw new Error('executeTerminalCommand requires repoDir and branch to be set');
@@ -720,7 +725,7 @@ export abstract class VSCode {
720725
path: pathlib.join(SCREENSHOTS_FOLDER, `last-command.png`),
721726
});
722727
if (outputShouldBeVisible) {
723-
await expect(this.window.getByText(expectedOutput).first()).toBeVisible();
728+
await expect(this.window.getByText(expectedOutput).first()).toBeVisible({ timeout });
724729
} else {
725730
try {
726731
await expect(this.window.getByText(expectedOutput).first()).not.toBeVisible();

tests/e2e/tests/base/plugin-settings.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ test.describe.serial('Plugin Settings - Analyze on Save', { tag: ['@tier1'] }, (
2727
const profileName = `plugins-settings-${generateRandomString()}`;
2828

2929
test.beforeAll(async ({ testRepoData }) => {
30+
test.setTimeout(600_000);
3031
if (getDefaultProviderConfig() === LLEMULATOR_PROVIDER) {
3132
await loadLlemulatorResponses({
3233
reset: true,
@@ -111,7 +112,7 @@ test.describe.serial('Plugin Settings - Analyze on Save', { tag: ['@tier1'] }, (
111112
});
112113

113114
test.beforeEach(async function () {
114-
test.setTimeout(300000);
115+
test.setTimeout(600_000);
115116
const testName = test.info().title.replace(/[_"'\s]/g, '');
116117
console.log(`Starting ${testName} at ${new Date()}`);
117118

@@ -198,14 +199,17 @@ test.describe.serial('Plugin Settings - Analyze on Save', { tag: ['@tier1'] }, (
198199
await tabManager.saveTabFile(FILES_NAMES[1]);
199200
const rejectChangesBtn = vscodeApp.getWindow().getByText('Reject All Changes');
200201
// the diff should still be there when auto accept on save is disabled
202+
await vscodeApp.waitDefault();
201203
await rejectChangesBtn.click();
204+
await vscodeApp.assertNotification('Changes rejected and document saved', { timeout: 30_000 });
202205
await tabManager.saveTabFile(FILES_NAMES[1]);
203206
await tabManager.closeTabByName(FILES_NAMES[1]);
204207
await vscodeApp.executeTerminalCommand('git status', 'Changes not staged for commit', false);
205208
await vscodeApp.executeQuickCommand('View: Close All Editors');
206209
});
207210

208211
test('Exclude diagnostic sources in agent mode', async ({ testRepoData }) => {
212+
test.skip(!!process.env.WEB_ENV, 'Skipping test that requires a VS Code restart in web mode.');
209213
test.setTimeout(600000);
210214
const repoInfo = testRepoData['coolstore'];
211215

tests/e2e/tests/ccm/llm-proxy.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ test.describe.serial(
7070
).toBeVisible({ timeout: 30000 });
7171
});
7272

73+
test.beforeEach(async function () {
74+
test.setTimeout(300_000);
75+
const testName = test.info().title.replace(/[_"'\s]/g, '');
76+
console.log(`Starting ${testName} at ${new Date()}`);
77+
});
78+
7379
test('Hub LLM proxy configuration and notifications', async () => {
7480
console.log('Configuring Hub connection with profile sync enabled...');
7581
const authEnabled = !!(process.env.TEST_HUB_USERNAME && process.env.TEST_HUB_PASSWORD);

tests/e2e/utilities/utils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ export const extensionDependencies: Record<string, string[]> = {
9898
// Other Configuration
9999
// ============================================================================
100100
export const extensionShortName = process.env.TEST_CATEGORY || 'Konveyor';
101+
export const extensionLongName = process.env.TEST_CATEGORY_LONG || 'Konveyor';
101102

102103
// Function to get the analysis view title based on extension short name
103104
export function getAnalysisViewTitle(): string {

vscode/core/src/utilities/ModifiedFiles/handleFileResponse.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,7 @@ export async function handleFileResponse(
213213
try {
214214
const diskBytes = await vscode.workspace.fs.readFile(uri);
215215
finalContent = new TextDecoder().decode(diskBytes);
216-
logger.info(
217-
`Using on-disk content for solution server (captures in-place edits): ${path}`,
218-
);
216+
logger.info(`Using on-disk content for solution server (captures in-place edits): ${path}`);
219217
} catch (readError) {
220218
logger.warn(`Could not read disk content, using original content: ${path}`, readError);
221219
}

0 commit comments

Comments
 (0)