Skip to content
208 changes: 208 additions & 0 deletions components/ambient-ui/src/adapters/__tests__/mappers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,214 @@ describe('mapSdkSessionToDomain', () => {
expect(domain.createdAt).toBe('')
expect(domain.updatedAt).toBe('')
})

describe('new session fields', () => {
it('parses repos from valid JSON string', () => {
const repos = JSON.stringify([
{ url: 'https://github.com/org/repo1', branch: 'main', name: 'repo1', autoPush: true },
{ url: 'https://github.com/org/repo2', branch: null, name: null, autoPush: false },
])
const sdk = makeSdkSession({ repos })
const domain = mapSdkSessionToDomain(sdk)

expect(domain.repos).toHaveLength(2)
expect(domain.repos[0]).toEqual({
url: 'https://github.com/org/repo1',
branch: 'main',
name: 'repo1',
autoPush: true,
})
expect(domain.repos[1]).toEqual({
url: 'https://github.com/org/repo2',
branch: null,
name: null,
autoPush: false,
})
})

it('returns empty repos for empty string', () => {
const sdk = makeSdkSession({ repos: '' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.repos).toEqual([])
})

it('returns empty repos for invalid JSON', () => {
const sdk = makeSdkSession({ repos: 'not valid json' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.repos).toEqual([])
})

it('parses reconciled repos with all status variants', () => {
const reconciledRepos = JSON.stringify([
{ url: 'https://github.com/org/repo1', name: 'repo1', status: 'Cloning', currentActiveBranch: 'feat-1', defaultBranch: 'main', clonedAt: '2026-01-15T10:00:00Z' },
{ url: 'https://github.com/org/repo2', name: 'repo2', status: 'Ready', currentActiveBranch: 'main', defaultBranch: 'main', clonedAt: '2026-01-15T10:01:00Z' },
{ url: 'https://github.com/org/repo3', name: 'repo3', status: 'Failed', currentActiveBranch: null, defaultBranch: null, clonedAt: null },
])
const sdk = makeSdkSession({ reconciled_repos: reconciledRepos })
const domain = mapSdkSessionToDomain(sdk)

expect(domain.reconciledRepos).toHaveLength(3)
expect(domain.reconciledRepos[0]).toEqual({
url: 'https://github.com/org/repo1',
name: 'repo1',
status: 'Cloning',
currentActiveBranch: 'feat-1',
defaultBranch: 'main',
clonedAt: '2026-01-15T10:00:00Z',
})
expect(domain.reconciledRepos[1]!.status).toBe('Ready')
expect(domain.reconciledRepos[2]!.status).toBe('Failed')
expect(domain.reconciledRepos[2]!.currentActiveBranch).toBeNull()
expect(domain.reconciledRepos[2]!.clonedAt).toBeNull()
})

it('returns null status for invalid reconciled repo status', () => {
const reconciledRepos = JSON.stringify([
{ url: 'https://github.com/org/repo1', name: 'repo1', status: 'SomeBogusStatus' },
])
const sdk = makeSdkSession({ reconciled_repos: reconciledRepos })
const domain = mapSdkSessionToDomain(sdk)

expect(domain.reconciledRepos).toHaveLength(1)
expect(domain.reconciledRepos[0]!.status).toBeNull()
})

it('parses conditions array', () => {
const conditions = JSON.stringify([
{ type: 'Ready', status: 'True', reason: 'AllGood', message: 'Session is ready', lastTransitionTime: '2026-01-15T10:05:00Z' },
{ type: 'Progressing', status: 'False', reason: null, message: null, lastTransitionTime: null },
])
const sdk = makeSdkSession({ conditions })
const domain = mapSdkSessionToDomain(sdk)

expect(domain.conditions).toHaveLength(2)
expect(domain.conditions[0]).toEqual({
type: 'Ready',
status: 'True',
reason: 'AllGood',
message: 'Session is ready',
lastTransitionTime: '2026-01-15T10:05:00Z',
})
expect(domain.conditions[1]).toEqual({
type: 'Progressing',
status: 'False',
reason: null,
message: null,
lastTransitionTime: null,
})
})

it('returns Unknown for invalid condition status', () => {
const conditions = JSON.stringify([
{ type: 'Ready', status: 'Maybe' },
])
const sdk = makeSdkSession({ conditions })
const domain = mapSdkSessionToDomain(sdk)

expect(domain.conditions).toHaveLength(1)
expect(domain.conditions[0]!.status).toBe('Unknown')
})

it('parses environment variables from JSON string', () => {
const envVars = JSON.stringify({ NODE_ENV: 'production', API_URL: 'https://api.example.com' })
const sdk = makeSdkSession({ environment_variables: envVars })
const domain = mapSdkSessionToDomain(sdk)

expect(domain.environmentVariables).toEqual({
NODE_ENV: 'production',
API_URL: 'https://api.example.com',
})
})

it('parses labels from JSON string', () => {
const labels = JSON.stringify({ team: 'platform', tier: 'production' })
const sdk = makeSdkSession({ labels })
const domain = mapSdkSessionToDomain(sdk)

expect(domain.labels).toEqual({
team: 'platform',
tier: 'production',
})
})

it('returns empty object for invalid env vars JSON', () => {
const sdk = makeSdkSession({ environment_variables: '{broken' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.environmentVariables).toEqual({})
})

it('returns empty object for invalid labels JSON', () => {
const sdk = makeSdkSession({ labels: 'not-json' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.labels).toEqual({})
})

it('returns empty object for array-shaped env vars', () => {
const sdk = makeSdkSession({ environment_variables: '["a","b"]' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.environmentVariables).toEqual({})
})

it('returns empty object for array-shaped labels', () => {
const sdk = makeSdkSession({ labels: '["x"]' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.labels).toEqual({})
})

it('maps temperature, maxTokens, timeout from SDK numbers', () => {
const sdk = makeSdkSession({ llm_temperature: 0.5, llm_max_tokens: 8192, timeout: 7200 })
const domain = mapSdkSessionToDomain(sdk)

expect(domain.temperature).toBe(0.5)
expect(domain.maxTokens).toBe(8192)
expect(domain.timeout).toBe(7200)
})

it('returns null for zero temperature, maxTokens, timeout', () => {
const sdk = makeSdkSession({ llm_temperature: 0, llm_max_tokens: 0, timeout: 0 })
const domain = mapSdkSessionToDomain(sdk)

expect(domain.temperature).toBeNull()
expect(domain.maxTokens).toBeNull()
expect(domain.timeout).toBeNull()
})

it('maps workflowId from workflow_id', () => {
const sdk = makeSdkSession({ workflow_id: 'wf-42' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.workflowId).toBe('wf-42')
})

it('maps workflowId to null for empty string', () => {
const sdk = makeSdkSession({ workflow_id: '' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.workflowId).toBeNull()
})

it('maps prompt from SDK', () => {
const sdk = makeSdkSession({ prompt: 'Fix the bug in auth.ts' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.prompt).toBe('Fix the bug in auth.ts')
})

it('maps prompt to null for empty string', () => {
const sdk = makeSdkSession({ prompt: '' })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.prompt).toBeNull()
})

it('maps sdkRestartCount from sdk_restart_count', () => {
const sdk = makeSdkSession({ sdk_restart_count: 3 })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.sdkRestartCount).toBe(3)
})

it('defaults sdkRestartCount to 0 when sdk_restart_count is 0', () => {
const sdk = makeSdkSession({ sdk_restart_count: 0 })
const domain = mapSdkSessionToDomain(sdk)
expect(domain.sdkRestartCount).toBe(0)
})
})
})

describe('mapSdkProjectToDomain', () => {
Expand Down
91 changes: 90 additions & 1 deletion components/ambient-ui/src/adapters/mappers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import type { Session, Project } from 'ambient-sdk'
import type { DomainSession, DomainProject, DomainSessionMessage, SessionPhase, SessionEventType } from '@/domain/types'
import type {
DomainSession, DomainProject, DomainSessionMessage, SessionPhase, SessionEventType,
DomainRepo, DomainReconciledRepo, DomainCondition, ReconciledRepoStatus, ConditionStatus,
} from '@/domain/types'

const VALID_PHASES: ReadonlySet<string> = new Set<string>([
'Pending',
Expand Down Expand Up @@ -37,10 +40,85 @@ function parseAnnotations(raw: string): Record<string, string> {
}
}

function parseJsonArray(raw: string): unknown[] {
if (!raw) return []
try {
const parsed: unknown = JSON.parse(raw)
return Array.isArray(parsed) ? parsed : []
} catch {
return []
}
}

function parseJsonObject(raw: string): Record<string, string> {
if (!raw) return {}
try {
const parsed: unknown = JSON.parse(raw)
if (typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed)) {
const result: Record<string, string> = {}
for (const [key, value] of Object.entries(parsed as Record<string, unknown>)) {
result[key] = String(value)
}
return result
}
return {}
} catch {
return {}
}
}

const VALID_REPO_STATUSES: ReadonlySet<string> = new Set(['Cloning', 'Ready', 'Failed'])
const VALID_CONDITION_STATUSES: ReadonlySet<string> = new Set(['True', 'False', 'Unknown'])

function parseRepos(raw: string): DomainRepo[] {
return parseJsonArray(raw).map((item) => {
const r = item as Record<string, unknown>
return {
url: String(r.url ?? ''),
branch: r.branch ? String(r.branch) : null,
name: r.name ? String(r.name) : null,
autoPush: Boolean(r.autoPush),
}
})
}

function parseReconciledRepos(raw: string): DomainReconciledRepo[] {
return parseJsonArray(raw).map((item) => {
const r = item as Record<string, unknown>
const status = String(r.status ?? '')
return {
url: String(r.url ?? ''),
name: r.name ? String(r.name) : null,
status: VALID_REPO_STATUSES.has(status) ? (status as ReconciledRepoStatus) : null,
currentActiveBranch: r.currentActiveBranch ? String(r.currentActiveBranch) : null,
defaultBranch: r.defaultBranch ? String(r.defaultBranch) : null,
clonedAt: r.clonedAt ? String(r.clonedAt) : null,
}
})
}

function parseConditions(raw: string): DomainCondition[] {
return parseJsonArray(raw).map((item) => {
const c = item as Record<string, unknown>
const status = String(c.status ?? 'Unknown')
return {
type: String(c.type ?? ''),
status: VALID_CONDITION_STATUSES.has(status) ? (status as ConditionStatus) : 'Unknown',
reason: c.reason ? String(c.reason) : null,
message: c.message ? String(c.message) : null,
lastTransitionTime: c.lastTransitionTime ? String(c.lastTransitionTime) : null,
}
})
}
Comment on lines +73 to +112
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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard against non-object array entries before property access.

parseJsonArray can legitimately return arrays containing null or primitives (e.g. backend emits "[null]" or "[1,2]"). Here const r = item as Record<string, unknown> is a compile-time-only cast; at runtime r.url on a null item throws TypeError, and the .map isn't wrapped in try/catch — so a malformed repos/reconciled_repos/conditions payload crashes the render of the Resources/Config tabs.

🛡️ Suggested guard (apply to all three parsers)
+function isRecord(v: unknown): v is Record<string, unknown> {
+  return typeof v === 'object' && v !== null && !Array.isArray(v)
+}
+
 function parseRepos(raw: string): DomainRepo[] {
-  return parseJsonArray(raw).map((item) => {
-    const r = item as Record<string, unknown>
-    return {
+  return parseJsonArray(raw)
+    .filter(isRecord)
+    .map((r) => ({
       url: String(r.url ?? ''),
       branch: r.branch ? String(r.branch) : null,
       name: r.name ? String(r.name) : null,
       autoPush: Boolean(r.autoPush),
-    }
-  })
+    }))
 }
🤖 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 `@components/ambient-ui/src/adapters/mappers.ts` around lines 73 - 112, The
parser functions parseRepos, parseReconciledRepos, and parseConditions currently
cast each array item to Record<string, unknown> and access properties directly,
which throws if an item is null or a primitive; update each function to first
check that item is a non-null object (e.g., typeof item === "object" && item !==
null) before reading properties from it, and for invalid entries return sensible
defaults or skip them (e.g., map to null/empty fields or filter them out) so
malformed parseJsonArray outputs like [null] or [1,2] do not cause a runtime
TypeError; apply this guard consistently inside parseRepos,
parseReconciledRepos, and parseConditions when handling r / c and preserve
existing field coercions (String(...), Boolean(...), VALID_* checks).


function emptyToNull(value: string): string | null {
return value || null
}

function numberOrNull(value: number): number | null {
return value === 0 || value === undefined || value === null ? null : value
}
Comment on lines +118 to +120
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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

numberOrNull discards a valid temperature of 0.

temperature: 0 is a meaningful deterministic setting, but numberOrNull collapses it to null (same as unset). It's correct for maxTokens/timeout, but for temperature 0 should be preserved. Consider a dedicated nullable-numeric that only nulls undefined/null for temperature.

Also applies to: 132-132

🤖 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 `@components/ambient-ui/src/adapters/mappers.ts` around lines 118 - 120,
numberOrNull currently treats 0 as null which discards a valid temperature=0;
change the logic so numberOrNull returns null only for undefined or null (do not
collapse 0), and introduce a separate helper (e.g. numberNullIfZero) that
preserves the original behavior for fields that should treat 0 as unset
(maxTokens/timeout). Update callers: use numberOrNull for temperature mapping
and use numberNullIfZero for maxTokens/timeout (or rename functions to match
intent) while keeping the function names numberOrNull and the new helper
referenced in the mapper code.


export function mapSdkSessionToDomain(sdk: Session): DomainSession {
const annotations = parseAnnotations(sdk.annotations)
return {
Expand All @@ -51,11 +129,22 @@ export function mapSdkSessionToDomain(sdk: Session): DomainSession {
agentName: annotations['agent_name'] ?? null,
projectId: emptyToNull(sdk.project_id),
model: emptyToNull(sdk.llm_model),
temperature: numberOrNull(sdk.llm_temperature),
maxTokens: numberOrNull(sdk.llm_max_tokens),
timeout: numberOrNull(sdk.timeout),
workflowId: emptyToNull(sdk.workflow_id),
prompt: emptyToNull(sdk.prompt),
sdkRestartCount: sdk.sdk_restart_count ?? 0,
startTime: emptyToNull(sdk.start_time),
completionTime: emptyToNull(sdk.completion_time),
createdAt: sdk.created_at ?? '',
updatedAt: sdk.updated_at ?? '',
annotations,
labels: parseJsonObject(sdk.labels),
environmentVariables: parseJsonObject(sdk.environment_variables),
repos: parseRepos(sdk.repos),
reconciledRepos: parseReconciledRepos(sdk.reconciled_repos),
conditions: parseConditions(sdk.conditions),
}
}

Expand Down
Loading
Loading