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
48 changes: 48 additions & 0 deletions src/constants/outputStyles.protoName.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { expect, test } from 'bun:test'

import { resolveOutputStyle } from './outputStyles.js'

// `settings.outputStyle` is a free-form `z.string()` with no enum, and the
// style maps are plain object literals, so the name reaches a bare index. The
// resolved value flows into the model-facing system prompt
// (`# Output Style: ${config.name}\n${config.prompt}`) and the output-style
// system reminder, so a non-config must resolve to null rather than be rendered.
const STYLES = {
default: { name: 'Default', prompt: 'be concise' },
explanatory: { name: 'Explanatory', prompt: 'explain more' },
}

const PROTO_NAMES = [
'constructor',
'__proto__',
'toString',
'valueOf',
'hasOwnProperty',
'isPrototypeOf',
]

test('resolves a real style by name', () => {
expect(resolveOutputStyle(STYLES, 'explanatory')).toEqual({
name: 'Explanatory',
prompt: 'explain more',
})
})

test('returns null for an unconfigured style name', () => {
expect(resolveOutputStyle(STYLES, 'nonexistent-style')).toBeNull()
})

test('returns null for Object.prototype member names', () => {
// Before the fix each of these resolved to an inherited member. `?? null`
// did not neutralize it — the Object constructor is not nullish — so
// `outputStyle: "constructor"` injected "# Output Style: Object\nundefined"
// into the system prompt instead of falling back to the default.
for (const name of PROTO_NAMES) {
expect(resolveOutputStyle(STYLES, name)).toBeNull()
}
})

test('returns null for an explicitly null entry', () => {
// The map type allows null values; those are not usable configs either.
expect(resolveOutputStyle({ broken: null }, 'broken')).toBeNull()
})
22 changes: 21 additions & 1 deletion src/constants/outputStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,27 @@ export async function getOutputStyleConfig(): Promise<OutputStyleConfig | null>
const outputStyle = (settings?.outputStyle ||
DEFAULT_OUTPUT_STYLE_NAME) as string

return allStyles[outputStyle] ?? null
return resolveOutputStyle(allStyles, outputStyle)
}

/**
* Look up a style by name, returning null for anything that is not a real,
* configured style.
*
* `settings.outputStyle` is a free-form string with no enum, and the style maps
* are plain objects, so a bare index resolves inherited Object.prototype
* members. A trailing `?? null` does not neutralize that — `styles.constructor`
* is the Object constructor, which is not nullish — so the "unknown style falls
* back to the default" contract is skipped and a function is handed on as if it
* were a config.
*
* exported for testing
*/
export function resolveOutputStyle<T>(
styles: Record<string, T | null>,
name: string,
): T | null {
return Object.hasOwn(styles, name) ? (styles[name] ?? null) : null
}

export function hasCustomOutputStyle(): boolean {
Expand Down
18 changes: 13 additions & 5 deletions src/utils/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import { sanitizeToolNameForAnalytics } from 'src/services/analytics/metadata.js
import type { AgentId } from 'src/types/ids.js'
import { companionIntroText } from '../buddy/prompt.js'
import { NO_CONTENT_MESSAGE } from '../constants/messages.js'
import { OUTPUT_STYLE_CONFIG } from '../constants/outputStyles.js'
import {
OUTPUT_STYLE_CONFIG,
resolveOutputStyle,
} from '../constants/outputStyles.js'
import { isAutoMemoryEnabled } from '../memdir/paths.js'
import {
checkStatsigFeatureGate_CACHED_MAY_BE_STALE,
Expand Down Expand Up @@ -2751,10 +2754,15 @@ Read the team config to discover your teammates' names. Check the task list peri
])
}
case 'output_style': {
const outputStyle =
OUTPUT_STYLE_CONFIG[
attachment.style as keyof typeof OUTPUT_STYLE_CONFIG
]
// Own-property lookup: OUTPUT_STYLE_CONFIG is a plain object and
// `attachment.style` carries the free-form settings value, so a bare
// index resolves inherited Object.prototype members. Those are truthy, so
// the guard below would pass and the reminder would announce a style that
// does not exist ("Object output style is active").
const outputStyle = resolveOutputStyle(
OUTPUT_STYLE_CONFIG,
attachment.style,
)
if (!outputStyle) {
return []
}
Expand Down