feat(diagnose): add AI diagnosis with SSE streaming and rating#59
Open
feat(diagnose): add AI diagnosis with SSE streaming and rating#59
Conversation
- Add `--ai <question>` option to `diagnose` command
- Stream analysis from POST /diagnostic/v1/analyze via SSE with
real-time delta output, tool_call indicators, and error handling
- Collect and send client_info (cli_version, node_version, os) in context
- Extract sessionId from done event for rating submission
- Add interactive rating prompt (helpful/not_helpful/incorrect) via
POST /diagnostic/v1/sessions/{sessionId}/rating
- Refactor data collection into reusable collectDiagnosticData()
- Fix unused CLIError import in env-vars.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded AI streaming analysis to the diagnose CLI with interactive rating and JSON/stream modes; implemented streaming diagnostic APIs and session rating in the platform client; bumped package version; exposed CLI version at build time; removed one unused import. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/diagnose/index.ts`:
- Around line 166-169: When in JSON mode the handler early-returns after pushing
to jsonEvents, so sessionId (the variable consumed by outputJson()) is never
captured; update the event callback that pushes to jsonEvents to also extract
and store event.data.sessionId (when present) into the same sessionId variable
before returning. Apply the same change to the other JSON fast-path branches
referenced (the handlers around the "done" and the later event handlers) so any
event carrying sessionId sets it immediately, ensuring outputJson() emits the
correct sessionId.
- Around line 166-169: The handler that processes SSE events currently returns
early for json mode on error events and only logs errors in TTY, which lets the
command exit successfully; change the logic in the event callback that
references jsonEvents/events so that on event.type === 'error' you record the
error (e.g., push the error payload into jsonEvents or set an error flag)
instead of returning early, and after streaming completes (where the function
currently decides exit success) check for any recorded streamed errors and throw
a CLIError (include the error message/payload) so the command exits non-zero;
apply the same change to the other identical handlers around the json handling
at the referenced blocks (lines noted in the review: also applies to 186-188 and
229-230) and ensure TTY logging still emits the error but does not mask the
thrown CLIError.
- Around line 29-39: The ossMode fallback in collectDiagnosticData is never
reached because the caller invokes requireAuth(apiUrl) before computing ossMode,
causing early exit for API-key-linked projects; to fix, compute/determine
ossMode (or whether the project is API-key linked) before calling requireAuth so
collectDiagnosticData can run its degraded-path branches, or change the call
site to defer requireAuth until after collectDiagnosticData has been invoked;
update the caller flow that currently calls requireAuth(apiUrl) prior to
invoking collectDiagnosticData (and adjust any related logic) so
collectDiagnosticData's ossMode branches can execute when appropriate.
- Around line 229-230: The failure telemetry is misclassified because the AI
success path calls reportCliUsage('cli.diagnose.ai') but the shared catch block
always reports 'cli.diagnose'; update the code so the catch block reports the
same event name used for success when the AI path is active. Concretely,
introduce a local variable/eventName (e.g., const telemetryEvent =
'cli.diagnose.ai' when invoking the AI flow) or an isAiRun flag in the diagnose
command, use that variable when calling reportCliUsage in both the success
branch and the shared catch block (reportCliUsage(telemetryEvent, false)), and
ensure any existing non-AI path still uses 'cli.diagnose' unchanged.
- Around line 127-136: The current runtime package.json lookup in
src/commands/diagnose/index.ts (the block that sets cliVersion by reading
package.json using import.meta.url and join(... '../../../package.json')) fails
for tsup-bundled single-file outputs; replace this runtime filesystem traversal
with a build-time injected constant or a bundled data import: stop using
import.meta.url/readFileSync for cliVersion and either (a) add a DEFINE like
CLI_VERSION via tsup's define option and reference that constant where
cliVersion is set, or (b) generate a small version.json at build time and import
it (e.g. import version from './version.json') so the Diagnose command reads the
injected/bundled version instead of reading package.json at runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc8867ff-0f8c-4692-8fe3-ccf2ad2fb07f
📒 Files selected for processing (4)
package.jsonsrc/commands/deployments/env-vars.tssrc/commands/diagnose/index.tssrc/lib/api/platform.ts
- SSE parser: initialize currentEvent to 'delta', validate against known event types before casting - SSE parser: flush remaining buffer and multi-byte sequences after stream ends - CLI version: inject via tsup define (process.env.CLI_VERSION) instead of runtime package.json path traversal that breaks in bundled output - sessionId: capture from done event before JSON early-return so it's included in JSON output - Stream errors: record as CLIError and throw after streaming completes instead of silently succeeding - Telemetry: use consistent usageEvent variable for both success and failure paths (cli.diagnose.ai vs cli.diagnose) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/lib/api/platform.ts (2)
225-225: EncodesessionIdbefore embedding it in the URL path.Dynamic path segments should be URL-encoded to avoid malformed requests when IDs contain reserved characters.
Proposed fix
- await platformFetch(`/diagnostic/v1/sessions/${sessionId}/rating`, { + await platformFetch(`/diagnostic/v1/sessions/${encodeURIComponent(sessionId)}/rating`, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/api/platform.ts` at line 225, The call building the path for platformFetch embeds sessionId directly which can break URLs if the ID contains reserved characters; update the call that uses platformFetch and the sessionId variable to URL-encode the dynamic segment (e.g., replace `/diagnostic/v1/sessions/${sessionId}/rating` with a path that uses encodeURIComponent(sessionId) when constructing the string) so the sessionId is safely encoded before sending the request.
156-158: Function docstring is stale versus actual return type.The comment says it returns raw
Response, but the function returnsPromise<void>(Line 163). This can cause incorrect caller assumptions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/api/platform.ts` around lines 156 - 158, The docstring for streamDiagnosticAnalysisViaSSE is stale: it claims the function returns a raw Response while the implementation returns Promise<void>; update the function comment to accurately describe the actual return type and behavior (e.g., "Returns Promise<void>; streaming handled via onEvent and caller receives errors via thrown exceptions or callback") or alternatively change the function signature to return the raw Response if you intend that behavior—edit the comment near the streamDiagnosticAnalysisViaSSE definition to match whichever approach you choose so the docstring and the function (return type Promise<void> or Response) are consistent.tsup.config.ts (1)
4-4: Avoid CWD-coupledpackage.jsonresolution in build config.Line 4 uses a relative path string (
'./package.json'), which can break when build is executed from a different working directory.Proposed fix
-const pkg = JSON.parse(readFileSync('./package.json', 'utf-8')) as { version: string }; +const pkg = JSON.parse( + readFileSync(new URL('./package.json', import.meta.url), 'utf-8'), +) as { version: string };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsup.config.ts` at line 4, The build config currently reads package.json via a CWD-dependent relative path in the const pkg assignment; change it to resolve package.json relative to the config file itself (e.g., using fileURLToPath(import.meta.url) + dirname or new URL('./package.json', import.meta.url)) and then readFileSync that resolved path before JSON.parse, so pkg loading no longer depends on the current working directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/api/platform.ts`:
- Around line 179-190: processLine currently leaves currentEvent unchanged when
an unknown "event:" line is seen, causing a following "data:" line to be emitted
with a stale type; update processLine so that when an "event:" value is not in
VALID_EVENTS it explicitly resets currentEvent (e.g., to undefined/null) and
returns, and ensure the "data:" branch only calls onEvent({ type: currentEvent,
data }) when currentEvent is a valid DiagnosticSSEEvent['type']; reference
processLine, currentEvent, VALID_EVENTS, DiagnosticSSEEvent, and onEvent when
making the change.
---
Nitpick comments:
In `@src/lib/api/platform.ts`:
- Line 225: The call building the path for platformFetch embeds sessionId
directly which can break URLs if the ID contains reserved characters; update the
call that uses platformFetch and the sessionId variable to URL-encode the
dynamic segment (e.g., replace `/diagnostic/v1/sessions/${sessionId}/rating`
with a path that uses encodeURIComponent(sessionId) when constructing the
string) so the sessionId is safely encoded before sending the request.
- Around line 156-158: The docstring for streamDiagnosticAnalysisViaSSE is
stale: it claims the function returns a raw Response while the implementation
returns Promise<void>; update the function comment to accurately describe the
actual return type and behavior (e.g., "Returns Promise<void>; streaming handled
via onEvent and caller receives errors via thrown exceptions or callback") or
alternatively change the function signature to return the raw Response if you
intend that behavior—edit the comment near the streamDiagnosticAnalysisViaSSE
definition to match whichever approach you choose so the docstring and the
function (return type Promise<void> or Response) are consistent.
In `@tsup.config.ts`:
- Line 4: The build config currently reads package.json via a CWD-dependent
relative path in the const pkg assignment; change it to resolve package.json
relative to the config file itself (e.g., using fileURLToPath(import.meta.url) +
dirname or new URL('./package.json', import.meta.url)) and then readFileSync
that resolved path before JSON.parse, so pkg loading no longer depends on the
current working directory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a703fe01-e830-420d-bd22-c4e4dc4da3ff
📒 Files selected for processing (3)
src/commands/diagnose/index.tssrc/lib/api/platform.tstsup.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/commands/diagnose/index.ts
Align with backend OpenAPI spec: session_id instead of sessionId, tool_name instead of toolName. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/commands/diagnose/index.ts (1)
105-112:⚠️ Potential issue | 🟠 MajorMove auth gating after OSS-mode detection to preserve degraded diagnostics.
At Line 105, unconditional
requireAuth(apiUrl)prevents theossModefallback logic from ever executing, so partial diagnostics cannot run for API-key-linked OSS projects.Suggested fix
- await requireAuth(apiUrl); const config = getProjectConfig(); if (!config) throw new ProjectNotLinkedError(); const projectId = config.project_id; const projectName = config.project_name; const ossMode = config.project_id === 'oss-project'; + if (!ossMode) { + await requireAuth(apiUrl); + } trackDiagnose(opts.ai ? 'ai' : 'report', config);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/diagnose/index.ts` around lines 105 - 112, The call to requireAuth(apiUrl) is currently unconditional and blocks the ossMode fallback; instead, first call getProjectConfig() and compute ossMode (using config.project_id === 'oss-project') and run trackDiagnose(opts.ai ? 'ai' : 'report', config), then only call requireAuth(apiUrl) if needed for non-OSS flows; ensure ProjectNotLinkedError is still thrown when config is missing and preserve existing behavior for API-key-linked projects while allowing degraded diagnostics when ossMode is true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/commands/diagnose/index.ts`:
- Around line 105-112: The call to requireAuth(apiUrl) is currently
unconditional and blocks the ossMode fallback; instead, first call
getProjectConfig() and compute ossMode (using config.project_id ===
'oss-project') and run trackDiagnose(opts.ai ? 'ai' : 'report', config), then
only call requireAuth(apiUrl) if needed for non-OSS flows; ensure
ProjectNotLinkedError is still thrown when config is missing and preserve
existing behavior for API-key-linked projects while allowing degraded
diagnostics when ossMode is true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5bb8c19b-6aa2-406b-a0b8-eaae53b87473
📒 Files selected for processing (1)
src/commands/diagnose/index.ts
Set currentEvent to null when event type is not in VALID_EVENTS, and skip data lines when currentEvent is null to prevent emitting events with a stale type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/api/platform.ts`:
- Line 224: Interpolate an encoded sessionId into the platformFetch path to
avoid reserved-character issues: replace the direct `${sessionId}` usage in the
call to platformFetch(`/diagnostic/v1/sessions/${sessionId}/rating`, ...) by
wrapping sessionId with encodeURIComponent (i.e., use
encodeURIComponent(sessionId)) so the URL path segment is safely encoded; update
the invocation in platformFetch where the sessionId variable is used.
- Around line 155-158: The JSDoc for streamDiagnosticAnalysis is wrong: it
claims the function returns a raw Response but the signature is Promise<void>;
update the comment for the function (streamDiagnosticAnalysis) to reflect the
actual return contract (e.g., state that it returns Promise<void> which resolves
when streaming setup completes and that the raw Response is not returned), or
alternatively modify the function signature to return Promise<Response> if you
intend to return the raw Response—pick one and make the JSDoc and the function
signature consistent.
- Around line 179-193: processLine currently dispatches each 'data:' line
immediately, which breaks SSE multi-line messages; change it to accumulate
consecutive 'data:' lines into a buffer (e.g., currentData string/array) when
encountering lines that start with 'data:' (append raw content, join with '\n'
between entries), and only attempt JSON.parse and call onEvent({ type:
currentEvent, data }) when a blank line (empty or whitespace-only) is seen;
after dispatching reset currentData and currentEvent appropriately and ensure
malformed JSON is caught and ignored. Update the same logic in the other handler
block referenced (lines ~204-213) so both places use currentEvent and
currentData for proper SSE framing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0bb867a2-3281-41c4-8828-469819f53857
📒 Files selected for processing (1)
src/lib/api/platform.ts
- Filter out metrics with no data points (avoid null values) - Strip extra advisor fields (scanId, scanType, etc.), only send summary + collectorErrors as string[] - Stringify all db values to match spec types (string not number) - Explicitly pick log error fields (timestamp, message, source) - Improve platformFetch error message to include response message field - Add INSFORGE_DEBUG env var for request logging Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace non-hex placeholder IDs (oss-project, oss-org) with properly formatted UUIDs for OSS/self-hosted linking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/lib/api/platform.ts (2)
225-236:⚠️ Potential issue | 🟠 MajorEncode
sessionIdbefore inserting it into the URL path.Reserved characters in
sessionIdcan change the request path and send the rating to the wrong endpoint.Suggested fix
- await platformFetch(`/diagnostic/v1/sessions/${sessionId}/rating`, { + await platformFetch(`/diagnostic/v1/sessions/${encodeURIComponent(sessionId)}/rating`, { method: 'POST', body: JSON.stringify(body), }, apiUrl);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/api/platform.ts` around lines 225 - 236, The sessionId is inserted into the request URL without encoding, so reserved characters could break the path; update the URL construction in rateDiagnosticSession to use an encoded sessionId (e.g., call encodeURIComponent(sessionId)) when building `/diagnostic/v1/sessions/${...}/rating` so the platformFetch request always targets the intended endpoint.
188-202:⚠️ Potential issue | 🟠 MajorSSE framing is still incomplete.
This still emits on every
data:line instead of buffering until the blank-line delimiter, so multi-line events are fragmented or lost. That was the earlier SSE-spec issue and it remains unresolved here.Also applies to: 205-222
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/api/platform.ts` around lines 188 - 202, processLine currently emits on every `data:` line, fragmenting multi-line SSE messages; change it to buffer consecutive `data:` lines and only parse/emit when a blank-line delimiter is received: add a `dataBuffer` string (or array) scoped with `currentEvent` in the same module, on `data:` append the raw line (preserving newline between lines), on blank line (line.trim() === '') join the buffered lines into a single string, parse JSON once and call `onEvent({ type: currentEvent, data })`, then clear `dataBuffer` and `currentEvent`; ensure malformed JSON is handled the same and apply the same fix to the other identical block referenced around lines 205-222 so both occurrences (the `processLine` implementations) behave identically.src/commands/diagnose/index.ts (2)
102-113:⚠️ Potential issue | 🟠 MajorOSS-linked projects still fail before the degraded path can run.
requireAuth(apiUrl)executes before the project config is read, so API-key-linked/self-hosted projects exit immediately and never reachcollectDiagnosticData(..., ossMode, ...). That keepsinsforge diagnosebroken for direct links.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/diagnose/index.ts` around lines 102 - 113, The command calls requireAuth(apiUrl) before reading project config, which causes API-key/self-hosted projects to exit early; change the order so getProjectConfig() is called first (call getProjectConfig() and compute ossMode using config.project_id), then only call requireAuth(apiUrl) when ossMode is false (i.e., for non-OSS linked projects) so collectDiagnosticData(..., ossMode, ...) and the degraded OSS path run for direct links; update references in the action handler around requireAuth, getProjectConfig, ossMode, trackDiagnose and collectDiagnosticData accordingly.
210-213:⚠️ Potential issue | 🟠 MajorUse
sessionIdinstead ofsession_idin the done event handler.The done event data uses camelCase
sessionId. Readingevent.data.session_id(snake_case) will beundefined, silently breaking rating submission and JSON output.Suggested fix
if (event.type === 'done') { - sessionId = event.data.session_id as string | undefined; + sessionId = event.data.sessionId as string | undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/diagnose/index.ts` around lines 210 - 213, In the done event handler use the camelCase property: replace reading event.data.session_id with event.data.sessionId so the existing sessionId variable (captured earlier) is assigned correctly when event.type === 'done'; update the assignment in the block that checks event.type === 'done' (the code referencing sessionId and event.data) to use event.data.sessionId to ensure rating submission and JSON output receive the real sessionId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/projects/link.ts`:
- Around line 77-84: The diff replaced the OSS sentinel value with UUID-shaped
IDs which breaks existing checks that look for config.project_id ===
'oss-project'; instead add an explicit boolean flag (e.g., isOSS or
isSelfHosted) to the ProjectConfig type and set it when constructing
projectConfig in link.ts, then update the consumers that currently check
config.project_id === 'oss-project' (notably the logic in diagnose/index.ts and
lib/analytics.ts) to check the new flag (config.isOSS) so behavior remains
consistent without relying on a sentinel ID; ensure ProjectConfig declaration,
the creation site (projectConfig), and all callers are migrated together.
In `@src/lib/api/platform.ts`:
- Around line 29-34: The debug logging block gated by process.env.INSFORGE_DEBUG
is leaking sensitive data (Authorization bearer tokens and full request bodies);
update the logging in the platform debug section to redact auth-like headers
(e.g., any header key matching /authorization|auth/i or value starting with
"Bearer ") and to avoid printing full bodies by either masking large/JSON bodies
or printing only a truncated/summary string; modify the code that inspects
headers and options.body (the same place that currently logs headers and body)
to create a sanitizedHeaders object and a safeBody string before passing them to
console.error so tokens and PII are not emitted.
---
Duplicate comments:
In `@src/commands/diagnose/index.ts`:
- Around line 102-113: The command calls requireAuth(apiUrl) before reading
project config, which causes API-key/self-hosted projects to exit early; change
the order so getProjectConfig() is called first (call getProjectConfig() and
compute ossMode using config.project_id), then only call requireAuth(apiUrl)
when ossMode is false (i.e., for non-OSS linked projects) so
collectDiagnosticData(..., ossMode, ...) and the degraded OSS path run for
direct links; update references in the action handler around requireAuth,
getProjectConfig, ossMode, trackDiagnose and collectDiagnosticData accordingly.
- Around line 210-213: In the done event handler use the camelCase property:
replace reading event.data.session_id with event.data.sessionId so the existing
sessionId variable (captured earlier) is assigned correctly when event.type ===
'done'; update the assignment in the block that checks event.type === 'done'
(the code referencing sessionId and event.data) to use event.data.sessionId to
ensure rating submission and JSON output receive the real sessionId.
In `@src/lib/api/platform.ts`:
- Around line 225-236: The sessionId is inserted into the request URL without
encoding, so reserved characters could break the path; update the URL
construction in rateDiagnosticSession to use an encoded sessionId (e.g., call
encodeURIComponent(sessionId)) when building
`/diagnostic/v1/sessions/${...}/rating` so the platformFetch request always
targets the intended endpoint.
- Around line 188-202: processLine currently emits on every `data:` line,
fragmenting multi-line SSE messages; change it to buffer consecutive `data:`
lines and only parse/emit when a blank-line delimiter is received: add a
`dataBuffer` string (or array) scoped with `currentEvent` in the same module, on
`data:` append the raw line (preserving newline between lines), on blank line
(line.trim() === '') join the buffered lines into a single string, parse JSON
once and call `onEvent({ type: currentEvent, data })`, then clear `dataBuffer`
and `currentEvent`; ensure malformed JSON is handled the same and apply the same
fix to the other identical block referenced around lines 205-222 so both
occurrences (the `processLine` implementations) behave identically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9ff1fd8e-d025-423d-b77e-cabd407f0a97
📒 Files selected for processing (3)
src/commands/diagnose/index.tssrc/commands/projects/link.tssrc/lib/api/platform.ts
Comment on lines
77
to
84
| const projectConfig: ProjectConfig = { | ||
| project_id: 'oss-project', | ||
| project_id: 'fa4e0000-1234-5678-90ab-0e02b2c3d479', | ||
| project_name: 'oss-project', | ||
| org_id: 'oss-org', | ||
| appkey: 'oss', | ||
| region: 'local', | ||
| org_id: 'fa4e0001-1234-5678-90ab-0e02b2c3d479', | ||
| appkey: 'ossfkey', | ||
| region: 'us-test', | ||
| api_key: opts.apiKey, | ||
| oss_host: opts.apiBaseUrl.replace(/\/$/, ''), // remove trailing slash if any |
There was a problem hiding this comment.
Don’t change the OSS sentinel without migrating its consumers.
These UUID placeholders no longer match the existing config.project_id === 'oss-project' checks in src/commands/diagnose/index.ts and src/lib/analytics.ts, so direct-linked projects will now be treated like platform-linked ones. If you need UUID-shaped IDs here, add an explicit OSS/self-hosted flag to ProjectConfig and switch those callers to that instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/projects/link.ts` around lines 77 - 84, The diff replaced the
OSS sentinel value with UUID-shaped IDs which breaks existing checks that look
for config.project_id === 'oss-project'; instead add an explicit boolean flag
(e.g., isOSS or isSelfHosted) to the ProjectConfig type and set it when
constructing projectConfig in link.ts, then update the consumers that currently
check config.project_id === 'oss-project' (notably the logic in
diagnose/index.ts and lib/analytics.ts) to check the new flag (config.isOSS) so
behavior remains consistent without relying on a sentinel ID; ensure
ProjectConfig declaration, the creation site (projectConfig), and all callers
are migrated together.
Comment on lines
+29
to
+34
| if (process.env.INSFORGE_DEBUG) { | ||
| console.error(`[DEBUG] ${options.method ?? 'GET'} ${url}`); | ||
| console.error(`[DEBUG] Headers: ${JSON.stringify(headers, null, 2)}`); | ||
| if (options.body) { | ||
| console.error(`[DEBUG] Body: ${typeof options.body === 'string' ? options.body : JSON.stringify(options.body)}`); | ||
| } |
There was a problem hiding this comment.
Redact auth headers and diagnostic bodies from debug logs.
INSFORGE_DEBUG currently prints the bearer token verbatim and can dump full diagnostic payloads to stderr. That is enough to leak credentials and user data into CI or support logs.
Suggested fix
const url = `${baseUrl}${path}`;
if (process.env.INSFORGE_DEBUG) {
+ const debugHeaders = {
+ ...headers,
+ ...(headers.Authorization ? { Authorization: 'Bearer [REDACTED]' } : {}),
+ };
console.error(`[DEBUG] ${options.method ?? 'GET'} ${url}`);
- console.error(`[DEBUG] Headers: ${JSON.stringify(headers, null, 2)}`);
+ console.error(`[DEBUG] Headers: ${JSON.stringify(debugHeaders, null, 2)}`);
if (options.body) {
- console.error(`[DEBUG] Body: ${typeof options.body === 'string' ? options.body : JSON.stringify(options.body)}`);
+ console.error('[DEBUG] Body: [redacted]');
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/api/platform.ts` around lines 29 - 34, The debug logging block gated
by process.env.INSFORGE_DEBUG is leaking sensitive data (Authorization bearer
tokens and full request bodies); update the logging in the platform debug
section to redact auth-like headers (e.g., any header key matching
/authorization|auth/i or value starting with "Bearer ") and to avoid printing
full bodies by either masking large/JSON bodies or printing only a
truncated/summary string; modify the code that inspects headers and options.body
(the same place that currently logs headers and body) to create a
sanitizedHeaders object and a safeBody string before passing them to
console.error so tokens and PII are not emitted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--ai <question>option toinsforge diagnosefor cloud-powered AI analysisPOST /diagnostic/v1/analyzevia SSE with real-time text outputdelta(streamed text),tool_call,tool_result,done,errorPOST /diagnostic/v1/sessions/{sessionId}/ratingclient_info(CLI version, Node.js version, OS) alongside diagnostic contextcollectDiagnosticData()used by both standard report and AI modesCLIErrorimport inenv-vars.tsUsage
Test plan
diagnose --ai "test question"and verify SSE streaming output--jsonand verify structured event outputdiagnose(no--ai) still works unchanged🤖 Generated with Claude Code
Note
Add AI diagnosis with SSE streaming and post-analysis rating to the
diagnosecommand--ai <question>flag to thediagnosecommand in src/commands/diagnose/index.ts that streams AI analysis of collected diagnostics via Server-Sent Events.streamDiagnosticAnalysis(POSTs to/diagnostic/v1/analyze, parses SSEdelta/tool_call/done/errorevents) andrateDiagnosticSession(POSTs to/diagnostic/v1/sessions/{sessionId}/rating) in src/lib/api/platform.ts.collectDiagnosticDatahelper that concurrently gathers metrics, advisor summary, DB checks, and logs, normalizing results and accumulating errors for both AI and standard output flows.@clack/promptsand submits it; usage is tracked under distinct event namescli.diagnosevscli.diagnose.ai.process.env.CLI_VERSIONat build time via tsup.config.ts so the CLI version is included in the diagnostic context sent to the AI.Macroscope summarized d5224b5.
Summary by CodeRabbit
New Features
Chores