diff --git a/.changeset/client-configs-custom-base-only.md b/.changeset/client-configs-custom-base-only.md new file mode 100644 index 000000000..89bc3eafa --- /dev/null +++ b/.changeset/client-configs-custom-base-only.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Make no remote client configuration request unless `CUSTOM_API_BASE_URL` is set. diff --git a/.changeset/fewer-long-context-surveys.md b/.changeset/fewer-long-context-surveys.md new file mode 100644 index 000000000..9c19249b4 --- /dev/null +++ b/.changeset/fewer-long-context-surveys.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Show the long-context feedback survey at most once per session, and make both survey kinds share one cooldown. diff --git a/.changeset/fs-glob-segment-boundary.md b/.changeset/fs-glob-segment-boundary.md new file mode 100644 index 000000000..807151b58 --- /dev/null +++ b/.changeset/fs-glob-segment-boundary.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Glob filters on the server file endpoints now follow standard glob syntax: `**/` matches whole path segments (so `a/**/b` no longer matches `a/xxb`), and brace sets and character classes are expanded instead of matched literally. diff --git a/.changeset/fs-list-allow-ignored-globs.md b/.changeset/fs-list-allow-ignored-globs.md new file mode 100644 index 000000000..51ac70361 --- /dev/null +++ b/.changeset/fs-list-allow-ignored-globs.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Add `allow_ignored_globs` to the `fs:list` server endpoint to list named gitignored paths while the rest of gitignore still applies. diff --git a/.changeset/sanitize-foreground-bash-output.md b/.changeset/sanitize-foreground-bash-output.md new file mode 100644 index 000000000..d7dcecbb3 --- /dev/null +++ b/.changeset/sanitize-foreground-bash-output.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Strip terminal escape sequences from foreground Bash tool output so they no longer change the terminal state. diff --git a/apps/pythinker-code/src/tui/components/messages/shell-execution.ts b/apps/pythinker-code/src/tui/components/messages/shell-execution.ts index 71f6322c7..b48042bf6 100644 --- a/apps/pythinker-code/src/tui/components/messages/shell-execution.ts +++ b/apps/pythinker-code/src/tui/components/messages/shell-execution.ts @@ -3,6 +3,7 @@ import { Container, Text } from '@pymodel/pi-tui'; import { currentTheme } from '#/tui/theme'; import type { ToolCallBlockData, ToolResultBlockData } from '#/tui/types'; +import { sanitizeShellOutput } from '#/tui/utils/shell-output'; import type { ResultRenderer } from './tool-renderers/types'; import { isSpilledToolOutput, PREVIEW_LINES } from './tool-renderers/types'; @@ -59,8 +60,10 @@ export class ShellExecutionComponent extends Container { expandHint: boolean, ): void { if (!result.output) return; + // Untrusted bytes: sanitize the whole buffer, not each chunk, so escape + // sequences split across live-output chunks cannot reach the terminal. this.addChild( - new TruncatedOutputComponent(result.output, { + new TruncatedOutputComponent(sanitizeShellOutput(result.output), { expanded, isError: result.is_error ?? false, maxLines: PREVIEW_LINES, diff --git a/apps/pythinker-code/src/tui/controllers/survey-controller.ts b/apps/pythinker-code/src/tui/controllers/survey-controller.ts index 4d14d8b95..bfb0c8589 100644 --- a/apps/pythinker-code/src/tui/controllers/survey-controller.ts +++ b/apps/pythinker-code/src/tui/controllers/survey-controller.ts @@ -8,7 +8,9 @@ import { getSurveyPopupConfig, peekSurveyPopupConfig, peekSurveyPopupConfigFresh, + resolveSurveyPopupConfig, type SurveyPopupConfig, + type SurveyPopupPayload, } from '#/utils/survey-popup-config'; import { readSurveyLastShownTime, writeSurveyLastShownTime } from '#/utils/survey-state-store'; import { currentPythinkerRegion } from '#/utils/region'; @@ -94,7 +96,7 @@ function joinModels(models: ReadonlySet): string | undefined { } export interface SurveyControllerDeps { - readonly config?: () => SurveyPopupConfig; + readonly config?: () => SurveyPopupPayload; readonly monotonicNow?: () => number; readonly wallNow?: () => number; readonly random?: () => number; @@ -145,7 +147,8 @@ export class SurveyController { private userTurnsAtLastShown: number | undefined; private appearanceCount = 0; private globalLastShownAt: number | undefined; - private longContextRollConsumed = false; + private readonly longContextRollConsumedModels = new Set(); + private longContextShownThisMount = false; private generation = 0; private idleSince: number | undefined; private openedAt = 0; @@ -196,7 +199,8 @@ export class SurveyController { this.lastShownAt = undefined; this.userTurnsAtLastShown = undefined; this.appearanceCount = 0; - this.longContextRollConsumed = false; + this.longContextRollConsumedModels.clear(); + this.longContextShownThisMount = false; this.idleSince = undefined; this.stickySample = undefined; this.toolCallCount = 0; @@ -444,6 +448,13 @@ export class SurveyController { return false; } + private currentConfig(): SurveyPopupConfig { + return resolveSurveyPopupConfig( + (this.deps.config ?? defaultDeps.config)(), + resolveKfcModelId(this.host.state.appState), + ); + } + private evaluate(): void { if (this.machine.phase !== 'closed') return; if (!this.configReady) return; @@ -463,9 +474,12 @@ export class SurveyController { ) { return; } - const config = (this.deps.config ?? defaultDeps.config)(); - const verdict = evaluateSurveyGate({ ...this.gateInputs(), config }); - if (verdict.longContextRollConsumed === true) this.longContextRollConsumed = true; + const config = this.currentConfig(); + const inputs = this.gateInputs(); + const verdict = evaluateSurveyGate({ ...inputs, config }); + if (verdict.longContextRollConsumed === true) { + this.longContextRollConsumedModels.add(inputs.longContext.kfcModelId); + } if (!verdict.show) return; this.open(verdict.survey, config); } @@ -508,7 +522,12 @@ export class SurveyController { ...shared, cumulativeTokens: appState.cumulativeTokens ?? 0, virtualContextTokens: appState.contextTokens, - mountRollConsumed: this.longContextRollConsumed, + mountRollConsumed: this.longContextRollConsumedModels.has(shared.kfcModelId), + mountSurveyShown: this.longContextShownThisMount, + msSinceGlobalLastShown: + this.globalLastShownAt === undefined + ? undefined + : this.wallNow() - this.globalLastShownAt, drawMountRoll: () => (this.deps.random ?? defaultDeps.random)(), }, }; @@ -564,7 +583,7 @@ export class SurveyController { this.openedEditorText = this.host.state.editor.getText(); this.lastShownAt = shownAt; this.userTurnsAtLastShown = this.userTurnCount; - if (survey !== 'session') return; + if (survey === 'long_context') this.longContextShownThisMount = true; this.globalLastShownAt = this.wallNow(); try { (this.deps.writeGlobalLastShown ?? defaultDeps.writeGlobalLastShown)( @@ -598,7 +617,7 @@ export class SurveyController { response: effect.response, }, this.appearanceSnapshot?.fields ?? this.environmentFields(), - this.appearanceConfig ?? (this.deps.config ?? defaultDeps.config)(), + this.appearanceConfig ?? this.currentConfig(), ); const sessionId = this.appearanceSnapshot?.sessionId ?? ''; if (sessionId.length > 0) { diff --git a/apps/pythinker-code/src/tui/utils/survey-policy.ts b/apps/pythinker-code/src/tui/utils/survey-policy.ts index 6ba8b517a..c57053cdc 100644 --- a/apps/pythinker-code/src/tui/utils/survey-policy.ts +++ b/apps/pythinker-code/src/tui/utils/survey-policy.ts @@ -11,6 +11,7 @@ export type SurveyKind = 'session' | 'long_context'; export type SurveyGateSkipReason = | 'mount-roll-consumed' + | 'mount-survey-shown' | 'survey-active' | 'turn-in-progress' | 'idle-too-short' @@ -60,6 +61,8 @@ export interface LongContextArmGateInput extends SharedArmGateInput { readonly cumulativeTokens: number; readonly virtualContextTokens: number; readonly mountRollConsumed: boolean; + readonly mountSurveyShown: boolean; + readonly msSinceGlobalLastShown: number | undefined; readonly drawMountRoll: () => number; } @@ -141,6 +144,7 @@ function evaluateSessionArm(input: SurveyGateInput): SurveyGateVerdict { export function evaluateLongContextArm(input: SurveyGateInput): SurveyGateVerdict { const { longContext, config } = input; + if (longContext.mountSurveyShown) return { show: false, reason: 'mount-survey-shown' }; if (longContext.mountRollConsumed) return { show: false, reason: 'mount-roll-consumed' }; if (longContext.phase !== 'closed') return { show: false, reason: 'survey-active' }; if (longContext.turnInProgress) return { show: false, reason: 'turn-in-progress' }; @@ -179,6 +183,12 @@ export function evaluateLongContextArm(input: SurveyGateInput): SurveyGateVerdic if (counter < config.long_context_survey_threshold) { return { show: false, reason: 'below-threshold' }; } + if ( + longContext.msSinceGlobalLastShown !== undefined && + longContext.msSinceGlobalLastShown < config.min_time_between_global_feedback_ms + ) { + return { show: false, reason: 'global-cooldown' }; + } if (longContext.drawMountRoll() >= config.long_context_probability) { return { show: false, reason: 'sampled-out', longContextRollConsumed: true }; } diff --git a/apps/pythinker-code/src/utils/client-configs.ts b/apps/pythinker-code/src/utils/client-configs.ts index 953c9352b..2b5d75439 100644 --- a/apps/pythinker-code/src/utils/client-configs.ts +++ b/apps/pythinker-code/src/utils/client-configs.ts @@ -4,7 +4,7 @@ import { z } from 'zod'; import { getCacheDir } from '#/utils/paths'; import { readJsonFile, writeJsonFile } from '#/utils/persistence'; -import { currentPythinkerProfile, currentPythinkerRegion } from '#/utils/region'; +import { currentPythinkerRegion } from '#/utils/region'; /** * Generic client for the public client-configs endpoint: @@ -12,6 +12,10 @@ import { currentPythinkerProfile, currentPythinkerRegion } from '#/utils/region' * `{ name, config: }`, where the payload shape is config-specific * and validated by the caller-supplied schema. * + * The endpoint exists only on a custom API base (`CUSTOM_API_BASE_URL`). + * Without one there is nothing to ask, so no request is made and no + * token leaves the machine. + * * Each named config is cached for a day, in two layers: an in-process map * (the only layer the synchronous peek can see) and a JSON file under the * CLI cache dir (survives restarts, so the TTL holds across processes). An @@ -25,15 +29,11 @@ const CLIENT_CONFIGS_PATH = '/client_configs'; const CONFIG_CACHE_TTL_MS = 24 * 60 * 60 * 1000; const FETCH_TIMEOUT_MS = 5000; -/** The endpoint's API base: the env override keeps winning (custom/internal - envs); otherwise the active region profile, so a global login's token is - not sent to the mainland-China deployment. */ -function clientConfigsBaseUrl(): string { +/** The endpoint's API base, or undefined when no custom base is set. */ +function clientConfigsBaseUrl(): string | undefined { const override = process.env['CUSTOM_API_BASE_URL']?.trim(); - if (override !== undefined && override.length > 0) { - return override.replace(/\/+$/, ''); - } - return currentPythinkerProfile().apiBase.replace(/\/+$/, ''); + if (override === undefined || override.length === 0) return undefined; + return override.replace(/\/+$/, ''); } /** Cache entries are partitioned by region so a login switch never serves @@ -165,6 +165,8 @@ export async function fetchClientConfig( schema: S, options: ClientConfigFetchOptions = {}, ): Promise | undefined> { + const baseUrl = clientConfigsBaseUrl(); + if (baseUrl === undefined) return undefined; const fetchFn = options.fetchImpl ?? fetch; const headers: Record = { accept: 'application/json', @@ -174,7 +176,7 @@ export async function fetchClientConfig( headers['authorization'] = `Bearer ${options.accessToken}`; } try { - const response = await fetchFn(`${clientConfigsBaseUrl()}${CLIENT_CONFIGS_PATH}`, { + const response = await fetchFn(`${baseUrl}${CLIENT_CONFIGS_PATH}`, { method: 'POST', headers, body: JSON.stringify({ name }), diff --git a/apps/pythinker-code/src/utils/survey-popup-config.ts b/apps/pythinker-code/src/utils/survey-popup-config.ts index 373cffd10..2cbe1c572 100644 --- a/apps/pythinker-code/src/utils/survey-popup-config.ts +++ b/apps/pythinker-code/src/utils/survey-popup-config.ts @@ -42,6 +42,17 @@ export const DEFAULT_SURVEY_POPUP_CONFIG: SurveyPopupConfig = { long_context_trigger_mode: 'virtual_context', }; +export type SurveyPopupOverride = Partial>; + +export interface SurveyPopupPayload extends SurveyPopupConfig { + model_overrides: Record; +} + +export const DEFAULT_SURVEY_POPUP_PAYLOAD: SurveyPopupPayload = { + ...DEFAULT_SURVEY_POPUP_CONFIG, + model_overrides: {}, +}; + const FIELD_SCHEMAS = { probability: z.number().min(0).max(1), on_for_models: z.array(z.string()), @@ -55,7 +66,19 @@ const FIELD_SCHEMAS = { long_context_trigger_mode: z.enum(['cumulative', 'virtual_context']), } satisfies Record; -const surveyPopupConfigSchema = z.unknown().transform((raw): Partial => { +const surveyPopupOverrideSchema = z.object(FIELD_SCHEMAS).omit({ on_for_models: true }).partial(); + +function parseModelOverrides(raw: unknown): Record { + if (typeof raw !== 'object' || raw === null) return {}; + const overrides: Record = {}; + for (const [model, entry] of Object.entries(raw)) { + const parsed = surveyPopupOverrideSchema.safeParse(entry); + if (parsed.success) overrides[model] = parsed.data; + } + return overrides; +} + +const surveyPopupConfigSchema = z.unknown().transform((raw): Partial => { if (typeof raw !== 'object' || raw === null) return {}; const record = raw as Record; const partial: Record = {}; @@ -65,23 +88,35 @@ const surveyPopupConfigSchema = z.unknown().transform((raw): Partial; + if (record['model_overrides'] !== undefined) { + partial['model_overrides'] = parseModelOverrides(record['model_overrides']); + } + return partial as Partial; }); -function withDefaults(partial: Partial | undefined): SurveyPopupConfig { - return { ...DEFAULT_SURVEY_POPUP_CONFIG, ...partial }; +function withDefaults(partial: Partial | undefined): SurveyPopupPayload { + return { ...DEFAULT_SURVEY_POPUP_PAYLOAD, ...partial }; } export async function getSurveyPopupConfig( options: ClientConfigFetchOptions = {}, -): Promise { +): Promise { return withDefaults(await getClientConfig(CONFIG_NAME, surveyPopupConfigSchema, options)); } -export function peekSurveyPopupConfig(now?: number): SurveyPopupConfig { +export function peekSurveyPopupConfig(now?: number): SurveyPopupPayload { return withDefaults(peekClientConfig(CONFIG_NAME, surveyPopupConfigSchema, now)); } +export function resolveSurveyPopupConfig( + payload: SurveyPopupPayload, + kfcModelId: string | undefined, +): SurveyPopupConfig { + const { model_overrides: modelOverrides, ...base } = payload; + const override = kfcModelId === undefined ? undefined : modelOverrides[kfcModelId]; + return override === undefined ? base : { ...base, ...override }; +} + export function peekSurveyPopupConfigFresh(now?: number): boolean { return peekClientConfig(CONFIG_NAME, surveyPopupConfigSchema, now) !== undefined; } diff --git a/apps/pythinker-code/test/tui/components/messages/shell-execution.test.ts b/apps/pythinker-code/test/tui/components/messages/shell-execution.test.ts index c61c43573..11c54924b 100644 --- a/apps/pythinker-code/test/tui/components/messages/shell-execution.test.ts +++ b/apps/pythinker-code/test/tui/components/messages/shell-execution.test.ts @@ -65,6 +65,24 @@ describe('ShellExecutionComponent', () => { expect(output).toContain('step20'); }); + it('strips terminal control sequences from captured output', () => { + const component = new ShellExecutionComponent({ + result: { + tool_call_id: 'call_shell', + output: 'before\u001B]0;pwned\u0007\u001B[2Jafter\u001B[?1049h', + is_error: false, + }, + expanded: true, + }); + + const output = component.render(100).map(strip).join('\n'); + expect(output).toContain('beforeafter'); + expect(output).not.toContain('\u001B]0;'); + expect(output).not.toContain('\u001B[2J'); + expect(output).not.toContain('\u001B[?1049h'); + expect(output).not.toContain('\u0007'); + }); + it('does not count trailing empty lines toward the preview cap', () => { const component = new ShellExecutionComponent({ result: { diff --git a/apps/pythinker-code/test/tui/controllers/survey-controller.test.ts b/apps/pythinker-code/test/tui/controllers/survey-controller.test.ts index da1e0c326..89d7d9ec6 100644 --- a/apps/pythinker-code/test/tui/controllers/survey-controller.test.ts +++ b/apps/pythinker-code/test/tui/controllers/survey-controller.test.ts @@ -7,7 +7,7 @@ import { type SurveyHost, } from '#/tui/controllers/survey-controller'; import type { TranscriptEntry } from '#/tui/types'; -import { DEFAULT_SURVEY_POPUP_CONFIG } from '#/utils/survey-popup-config'; +import { DEFAULT_SURVEY_POPUP_PAYLOAD } from '#/utils/survey-popup-config'; const mocks = vi.hoisted(() => ({ getSurveyPopupConfig: vi.fn(() => Promise.resolve(undefined)), @@ -451,7 +451,7 @@ describe('SurveyController long-context arm', () => { dynamic_workflow_run_count: 0, ...DEFAULT_SNAPSHOT, }); - expect(harness.writes).toEqual([]); + expect(harness.writes).toEqual([1_700_000_000_000]); }); it('reports the responded and abandoned states under the long_context_survey name', async () => { @@ -541,7 +541,7 @@ describe('SurveyController long-context arm', () => { it('compares the cumulative counter when the trigger mode is cumulative', async () => { const harness = createHarness({ config: () => ({ - ...DEFAULT_SURVEY_POPUP_CONFIG, + ...DEFAULT_SURVEY_POPUP_PAYLOAD, long_context_trigger_mode: 'cumulative', }), }); @@ -565,7 +565,7 @@ describe('SurveyController long-context arm', () => { it('closes the arm on a non-positive effective threshold and produces no events', async () => { const harness = createHarness({ - config: () => ({ ...DEFAULT_SURVEY_POPUP_CONFIG, long_context_survey_threshold: 0 }), + config: () => ({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, long_context_survey_threshold: 0 }), }); harness.state.appState.contextTokens = 500_000; await harness.flush(); @@ -580,7 +580,7 @@ describe('SurveyController long-context arm', () => { it('leaves the session arm running when the threshold closes the long-context arm', async () => { const harness = createHarness({ - config: () => ({ ...DEFAULT_SURVEY_POPUP_CONFIG, long_context_survey_threshold: 0 }), + config: () => ({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, long_context_survey_threshold: 0 }), }); harness.state.appState.contextTokens = 500_000; await harness.flush(); @@ -664,7 +664,7 @@ describe('SurveyController long-context arm', () => { 'long_context_survey', expect.objectContaining({ event_type: 'appeared', appearance_index: 1 }), ); - expect(harness.writes).toEqual([]); + expect(harness.writes).toEqual([1_700_000_000_000]); }); it('does not spend the roll while an active prompt suppresses the evaluation', async () => { @@ -712,7 +712,7 @@ describe('SurveyController long-context arm', () => { ); }); - it('ignores the persisted global cooldown that gates the session arm', async () => { + it('honors the shared persisted cooldown without spending the roll', async () => { const harness = createHarness({ readGlobalLastShown: async () => 1_700_000_000_000 - 1000, }); @@ -723,14 +723,21 @@ describe('SurveyController long-context arm', () => { harness.controller.notifyTurnEnded(); harness.elapse(2000); + expect(harness.container.children).toHaveLength(0); + expect(harness.track).not.toHaveBeenCalled(); + + harness.clock.wall += 100_000_000; + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); expect(harness.track).toHaveBeenCalledWith( 'long_context_survey', expect.objectContaining({ event_type: 'appeared' }), ); - expect(harness.writes).toEqual([]); + expect(harness.writes).toEqual([1_700_000_000_000 + 100_000_000]); }); - it('does not suppress the session arm through the persisted cooldown after a long-context appearance', async () => { + it('writes the shared global cooldown on a long-context appearance', async () => { const harness = createHarness(); harness.state.appState.contextTokens = 250_000; await harness.flush(); @@ -742,18 +749,49 @@ describe('SurveyController long-context arm', () => { 'long_context_survey', expect.objectContaining({ event_type: 'appeared' }), ); + expect(harness.writes).toEqual([1_700_000_000_000]); harness.clock.mono += 600; harness.controller.handlePreInput(ESC); - expect(harness.writes).toEqual([]); + expect(harness.container.children).toHaveLength(0); + harness.track.mockClear(); harness.clock.mono += 3_600_000; harness.runTurns(10); harness.elapse(2000); + expect(harness.track).not.toHaveBeenCalled(); + + harness.clock.wall += 100_000_000; + harness.runTurns(1); + harness.elapse(2000); expect(harness.track).toHaveBeenCalledWith( 'feedback_survey', expect.objectContaining({ event_type: 'appeared' }), ); - expect(harness.writes).toEqual([1_700_000_000_000]); + }); + + it('keeps the long-context arm closed across a remount inside the shared cooldown', async () => { + let stored: number | undefined; + const harness = createHarness({ + readGlobalLastShown: async () => stored, + writeGlobalLastShown: (wallTime) => { + stored = wallTime; + }, + }); + harness.state.appState.contextTokens = 250_000; + await harness.flush(); + + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); + expect(harness.track).toHaveBeenCalledTimes(1); + + harness.controller.reset(); + await harness.flush(); + harness.track.mockClear(); + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); + expect(harness.track).not.toHaveBeenCalled(); }); }); @@ -816,7 +854,7 @@ describe('SurveyController kfc model gate', () => { it('opens for a kfc user when the real id is listed', async () => { const harness = createHarness({ - config: () => ({ ...DEFAULT_SURVEY_POPUP_CONFIG, on_for_models: ['k2', 'k3'] }), + config: () => ({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, on_for_models: ['k2', 'k3'] }), }); useModel(harness); await harness.flush(); @@ -831,7 +869,7 @@ describe('SurveyController kfc model gate', () => { it('stays closed for a kfc user whose real id is not listed', async () => { const harness = createHarness({ - config: () => ({ ...DEFAULT_SURVEY_POPUP_CONFIG, on_for_models: ['k3-256k'] }), + config: () => ({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, on_for_models: ['k3-256k'] }), }); useModel(harness); await harness.flush(); @@ -857,7 +895,7 @@ describe('SurveyController kfc model gate', () => { it('stays closed for a self-hosted model that happens to share the listed id', async () => { const harness = createHarness({ - config: () => ({ ...DEFAULT_SURVEY_POPUP_CONFIG, on_for_models: ['k3'] }), + config: () => ({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, on_for_models: ['k3'] }), }); useModel(harness, { providerBaseUrl: GATEWAY_BASE_URL }); await harness.flush(); @@ -882,7 +920,7 @@ describe('SurveyController kfc model gate', () => { it('stays closed on a concrete list when the alias cannot be resolved', async () => { const harness = createHarness({ - config: () => ({ ...DEFAULT_SURVEY_POPUP_CONFIG, on_for_models: ['k2'] }), + config: () => ({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, on_for_models: ['k2'] }), }); await harness.flush(); harness.appear(); @@ -913,7 +951,7 @@ describe('SurveyController kfc model gate', () => { it('prefers the entry baseUrl over the provider baseUrl when the entry is managed', async () => { const harness = createHarness({ - config: () => ({ ...DEFAULT_SURVEY_POPUP_CONFIG, on_for_models: ['k3'] }), + config: () => ({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, on_for_models: ['k3'] }), }); useModel(harness, { entryBaseUrl: MANAGED_BASE_URL, providerBaseUrl: GATEWAY_BASE_URL }); await harness.flush(); @@ -928,7 +966,7 @@ describe('SurveyController kfc model gate', () => { it('prefers the entry baseUrl over the provider baseUrl when the entry is self-hosted', async () => { const harness = createHarness({ - config: () => ({ ...DEFAULT_SURVEY_POPUP_CONFIG, on_for_models: ['k3'] }), + config: () => ({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, on_for_models: ['k3'] }), }); useModel(harness, { entryBaseUrl: GATEWAY_BASE_URL, providerBaseUrl: MANAGED_BASE_URL }); await harness.flush(); @@ -939,6 +977,277 @@ describe('SurveyController kfc model gate', () => { }); }); +describe('SurveyController model overrides', () => { + const MANAGED_BASE_URL = 'https://api.example.com/v1/v1'; + const savedBaseUrl = process.env['CUSTOM_API_BASE_URL']; + + beforeEach(() => { + process.env['CUSTOM_API_BASE_URL'] = MANAGED_BASE_URL; + }); + + afterEach(() => { + if (savedBaseUrl === undefined) { + delete process.env['CUSTOM_API_BASE_URL']; + } else { + process.env['CUSTOM_API_BASE_URL'] = savedBaseUrl; + } + }); + + function useManagedModel(harness: Harness, model = 'k3'): void { + harness.state.appState.model = 'main'; + harness.state.appState.availableModels = { + main: { provider: 'openai', model, maxContextSize: 256_000 }, + }; + harness.state.appState.availableProviders = { + 'openai': { type: 'pythinker', baseUrl: MANAGED_BASE_URL }, + }; + } + + it('opens for the targeted model under its override probability when the base samples out', async () => { + const harness = createHarness({ + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + probability: 0, + model_overrides: { k3: { probability: 1 } }, + }), + }); + useManagedModel(harness); + await harness.flush(); + harness.appear(); + + expect(harness.track).toHaveBeenCalledWith( + 'feedback_survey', + expect.objectContaining({ event_type: 'appeared', config_probability: 1 }), + ); + }); + + it('keeps the base policy for models without an override entry', async () => { + const harness = createHarness({ + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + probability: 0, + model_overrides: { k3: { probability: 1 } }, + }), + }); + useManagedModel(harness, 'other-model'); + await harness.flush(); + harness.appear(); + + expect(harness.container.children).toHaveLength(0); + expect(harness.track).not.toHaveBeenCalled(); + }); + + it('applies the override on the next evaluation after the model switches mid-session', async () => { + const harness = createHarness({ + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + probability: 0, + model_overrides: { k3: { probability: 1 } }, + }), + }); + await harness.flush(); + harness.appear(); + expect(harness.container.children).toHaveLength(0); + + useManagedModel(harness); + harness.runTurns(1); + harness.elapse(2000); + expect(harness.container.children).not.toHaveLength(0); + expect(harness.track).toHaveBeenCalledWith( + 'feedback_survey', + expect.objectContaining({ event_type: 'appeared', pfc_model_id: 'k3' }), + ); + }); + + it('honors the override global cooldown against the shared persisted clock', async () => { + const harness = createHarness({ + readGlobalLastShown: async () => 1_700_000_000_000 - 1000, + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + model_overrides: { k3: { min_time_between_global_feedback_ms: 0 } }, + }), + }); + useManagedModel(harness); + await harness.flush(); + harness.appear(); + + expect(harness.container.children).not.toHaveLength(0); + expect(harness.track).toHaveBeenCalledWith( + 'feedback_survey', + expect.objectContaining({ + event_type: 'appeared', + config_min_time_between_global_feedback_ms: 0, + }), + ); + }); + + it('keeps on_for_models as the gate even when an override entry matches', async () => { + const harness = createHarness({ + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + on_for_models: ['someone-else'], + model_overrides: { k3: { probability: 1 } }, + }), + }); + useManagedModel(harness); + await harness.flush(); + harness.appear(); + + expect(harness.container.children).toHaveLength(0); + expect(harness.track).not.toHaveBeenCalled(); + }); + + it('targets the long-context arm per model through the override', async () => { + const targeted = createHarness({ + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + model_overrides: { k3: { long_context_probability: 0 } }, + }), + }); + useManagedModel(targeted); + targeted.state.appState.contextTokens = 250_000; + await targeted.flush(); + + targeted.controller.notifyTurnStarted(true); + targeted.controller.notifyTurnEnded(); + targeted.elapse(2000); + expect(targeted.container.children).toHaveLength(0); + expect(targeted.track).not.toHaveBeenCalled(); + + const untargeted = createHarness({ + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + model_overrides: { k3: { long_context_probability: 0 } }, + }), + }); + untargeted.state.appState.contextTokens = 250_000; + await untargeted.flush(); + + untargeted.controller.notifyTurnStarted(true); + untargeted.controller.notifyTurnEnded(); + untargeted.elapse(2000); + expect(untargeted.track).toHaveBeenCalledWith( + 'long_context_survey', + expect.objectContaining({ event_type: 'appeared' }), + ); + }); + + it('treats the long-context roll as consumed per model across a mid-session switch', async () => { + const harness = createHarness({ + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + model_overrides: { k3: { long_context_probability: 0 } }, + }), + }); + useManagedModel(harness); + harness.state.appState.contextTokens = 250_000; + await harness.flush(); + + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); + expect(harness.container.children).toHaveLength(0); + + useManagedModel(harness, 'other-model'); + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); + expect(harness.track).toHaveBeenCalledWith( + 'long_context_survey', + expect.objectContaining({ event_type: 'appeared', pfc_model_id: 'other-model' }), + ); + + useManagedModel(harness); + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); + expect( + harness.track.mock.calls.filter( + (call) => (call[1] as { event_type?: string }).event_type === 'appeared', + ), + ).toHaveLength(1); + }); + + it('closes the long-context arm for the rest of the mount after the first appearance', async () => { + const harness = createHarness({ + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + model_overrides: { k3: { long_context_probability: 1 } }, + }), + }); + useManagedModel(harness); + harness.state.appState.contextTokens = 250_000; + await harness.flush(); + + const appeared = () => + harness.track.mock.calls.filter( + (call) => (call[1] as { event_type?: string }).event_type === 'appeared', + ).length; + + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); + expect(appeared()).toBe(1); + harness.clock.mono += 600; + harness.controller.handlePreInput(ESC); + + useManagedModel(harness, 'other-model'); + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); + expect(appeared()).toBe(1); + + useManagedModel(harness); + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); + expect(appeared()).toBe(1); + }); + + it('honors the override global cooldown on the long-context arm', async () => { + const harness = createHarness({ + readGlobalLastShown: async () => 1_700_000_000_000 - 1000, + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + model_overrides: { k3: { min_time_between_global_feedback_ms: 0 } }, + }), + }); + useManagedModel(harness); + harness.state.appState.contextTokens = 250_000; + await harness.flush(); + + harness.controller.notifyTurnStarted(true); + harness.controller.notifyTurnEnded(); + harness.elapse(2000); + expect(harness.track).toHaveBeenCalledWith( + 'long_context_survey', + expect.objectContaining({ event_type: 'appeared' }), + ); + }); + + it('reports the merged effective config in the policy snapshot', async () => { + const harness = createHarness({ + config: () => ({ + ...DEFAULT_SURVEY_POPUP_PAYLOAD, + model_overrides: { k3: { probability: 0.5, min_user_turns_before_feedback: 1 } }, + }), + }); + useManagedModel(harness); + await harness.flush(); + harness.appear(); + + expect(harness.track).toHaveBeenCalledWith( + 'feedback_survey', + expect.objectContaining({ + event_type: 'appeared', + config_probability: 0.5, + config_min_user_turns_before_feedback: 1, + config_min_time_before_feedback_ms: 600_000, + }), + ); + }); +}); + describe('SurveyController interaction', () => { it('selects a rating after the digit debounce and walks pending → thanks → closed', async () => { const harness = createHarness(); @@ -1888,7 +2197,7 @@ describe('SurveyController event payload', () => { }); it('snapshots the config that produced the appearance, not a later refresh', async () => { - let cloudConfig = { ...DEFAULT_SURVEY_POPUP_CONFIG, probability: 0.5 }; + let cloudConfig = { ...DEFAULT_SURVEY_POPUP_PAYLOAD, probability: 0.5 }; const harness = createHarness({ config: () => cloudConfig }); await harness.flush(); harness.appear(); @@ -2005,7 +2314,7 @@ describe('SurveyController event payload', () => { it('defers the evaluation that triggers a refresh so a stale policy cannot open the survey', async () => { let region = 'region-a'; - let config = { ...DEFAULT_SURVEY_POPUP_CONFIG, probability: 1 }; + let config = { ...DEFAULT_SURVEY_POPUP_PAYLOAD, probability: 1 }; const refreshConfig = vi.fn(() => { config = { ...config, probability: 0 }; }); @@ -2034,7 +2343,7 @@ describe('SurveyController event payload', () => { it('evaluates with the refreshed policy on the next turn after the deferred evaluation', async () => { let region = 'region-a'; - let config = { ...DEFAULT_SURVEY_POPUP_CONFIG, probability: 1, on_for_models: ['other-model'] }; + let config = { ...DEFAULT_SURVEY_POPUP_PAYLOAD, probability: 1, on_for_models: ['other-model'] }; const refreshConfig = vi.fn(() => { config = { ...config, on_for_models: ['*'] }; }); @@ -2094,7 +2403,7 @@ describe('SurveyController event payload', () => { it('applies the cloud model gate at evaluation time', async () => { const harness = createHarness({ - config: () => ({ ...DEFAULT_SURVEY_POPUP_CONFIG, on_for_models: ['other-model'] }), + config: () => ({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, on_for_models: ['other-model'] }), }); await harness.flush(); harness.appear(); diff --git a/apps/pythinker-code/test/tui/pythinker-tui-message-flow.test.ts b/apps/pythinker-code/test/tui/pythinker-tui-message-flow.test.ts index 1455fa953..028e0b49d 100644 --- a/apps/pythinker-code/test/tui/pythinker-tui-message-flow.test.ts +++ b/apps/pythinker-code/test/tui/pythinker-tui-message-flow.test.ts @@ -9523,10 +9523,16 @@ describe('PythinkerTUI session rating survey', () => { ); vi.useRealTimers(); - await new Promise((resolve) => { - setImmediate(resolve); + const stateFile = join(homeDir, 'feedback-survey-state.json'); + await vi.waitFor(() => { + expect(existsSync(stateFile)).toBe(true); }); - expect(existsSync(join(homeDir, 'feedback-survey-state.json'))).toBe(false); + const persisted = JSON.parse(await readFile(stateFile, 'utf-8')) as { + version: number; + last_shown_time: number; + }; + expect(persisted.version).toBe(1); + expect(typeof persisted.last_shown_time).toBe('number'); } finally { vi.useRealTimers(); vi.restoreAllMocks(); diff --git a/apps/pythinker-code/test/tui/utils/survey-policy.test.ts b/apps/pythinker-code/test/tui/utils/survey-policy.test.ts index e385149ed..7e94e9da5 100644 --- a/apps/pythinker-code/test/tui/utils/survey-policy.test.ts +++ b/apps/pythinker-code/test/tui/utils/survey-policy.test.ts @@ -65,6 +65,8 @@ function passingLongContext( cumulativeTokens: 0, virtualContextTokens: 0, mountRollConsumed: false, + mountSurveyShown: false, + msSinceGlobalLastShown: undefined, drawMountRoll: () => 0, ...overrides, }; @@ -212,6 +214,7 @@ describe('evaluateLongContextArm', () => { it.each<[Partial, string]>([ [{ mountRollConsumed: true }, 'mount-roll-consumed'], + [{ mountSurveyShown: true }, 'mount-survey-shown'], [{ phase: 'open' }, 'survey-active'], [{ phase: 'pending' }, 'survey-active'], [{ phase: 'thanks' }, 'survey-active'], @@ -229,10 +232,18 @@ describe('evaluateLongContextArm', () => { [{ telemetryDisabled: true }, 'telemetry-disabled'], [{ lastUserMessageStartsOrderedList: true }, 'ordered-list-ambiguity'], [{ virtualContextTokens: 199_999 }, 'below-threshold'], + [{ msSinceGlobalLastShown: 99_999_999 }, 'global-cooldown'], ])('skips with %j → %s', (overrides, reason) => { expect(arm(overrides)).toEqual({ show: false, reason }); }); + it('checks the mount cap first: a shown survey reports mount-survey-shown, not later reasons', () => { + expect(arm({ mountSurveyShown: true, mountRollConsumed: true, phase: 'open' })).toEqual({ + show: false, + reason: 'mount-survey-shown', + }); + }); + it('checks the mount latch first: a spent roll reports mount-roll-consumed, not later reasons', () => { expect(arm({ mountRollConsumed: true, phase: 'open', telemetryDisabled: true })).toEqual({ show: false, @@ -289,6 +300,23 @@ describe('evaluateLongContextArm', () => { expect(drawMountRoll).not.toHaveBeenCalled(); }); + it('does not draw the dice inside the shared global cooldown, so the roll stays unspent', () => { + const drawMountRoll = vi.fn(() => 0); + expect(arm({ msSinceGlobalLastShown: 1000, drawMountRoll })).toEqual({ + show: false, + reason: 'global-cooldown', + }); + expect(drawMountRoll).not.toHaveBeenCalled(); + }); + + it('shows once the shared global cooldown has elapsed', () => { + expect(arm({ msSinceGlobalLastShown: 100_000_000 })).toEqual({ + show: true, + survey: 'long_context', + longContextRollConsumed: true, + }); + }); + it('does not draw the dice while transiently suppressed by an active prompt', () => { const drawMountRoll = vi.fn(() => 0); expect(arm({ promptActive: true, drawMountRoll })).toEqual({ @@ -412,14 +440,16 @@ describe('evaluateSurveyGate (arbitration)', () => { }); }); - it('shows the long-context survey inside the persisted cooldown that still gates the session arm', () => { + it('blocks both arms inside the shared persisted cooldown', () => { expect( - evaluateSurveyGate(gate({ msSinceGlobalLastShown: 1000 }, CONFIG, ELIGIBLE)), - ).toEqual({ - show: true, - survey: 'long_context', - longContextRollConsumed: true, - }); + evaluateSurveyGate( + gate( + { msSinceGlobalLastShown: 1000 }, + CONFIG, + { ...ELIGIBLE, msSinceGlobalLastShown: 1000 }, + ), + ), + ).toEqual({ show: false, reason: 'global-cooldown' }); }); it('shows nothing when the long-context arm is ineligible and the session arm is sampled out', () => { diff --git a/apps/pythinker-code/test/utils/client-configs.test.ts b/apps/pythinker-code/test/utils/client-configs.test.ts index 301ab025a..262c8e2bf 100644 --- a/apps/pythinker-code/test/utils/client-configs.test.ts +++ b/apps/pythinker-code/test/utils/client-configs.test.ts @@ -32,7 +32,12 @@ function jsonResponse(body: unknown, status = 200): Response { }); } +beforeEach(() => { + vi.stubEnv('CUSTOM_API_BASE_URL', 'https://api.example.test/coding/v1'); +}); + afterEach(() => { + vi.unstubAllEnvs(); resetClientConfigCache(); }); @@ -367,7 +372,21 @@ describe('region awareness', () => { refreshPythinkerRegion(); }); - it('fetches from the active region profile and partitions the cache by region', async () => { + it('makes no request and sends no token without a custom API base', async () => { + vi.stubEnv('CUSTOM_API_BASE_URL', ''); + const fetchImpl = vi.fn(async () => jsonResponse(ENVELOPE)); + + const data = await getClientConfig('estimated_cache_duration', configSchema, { + accessToken: 'secret-token', + fetchImpl: fetchImpl as typeof fetch, + cacheFile: null, + }); + + expect(data).toBeUndefined(); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + + it('partitions the cache by region', async () => { const fetchImpl = vi.fn(async () => jsonResponse(ENVELOPE)); const data = await getClientConfig('estimated_cache_duration', configSchema, { @@ -377,7 +396,7 @@ describe('region awareness', () => { expect(data).toEqual(CONFIG); expect(fetchImpl).toHaveBeenCalledWith( - expect.stringContaining('https://api.example.ai/coding/v1/client_configs'), + expect.stringContaining('https://api.example.test/coding/v1/client_configs'), expect.anything(), ); expect(peekClientConfig('estimated_cache_duration', configSchema)).toEqual(CONFIG); diff --git a/apps/pythinker-code/test/utils/recommended-effort-config.test.ts b/apps/pythinker-code/test/utils/recommended-effort-config.test.ts index 04d901515..4e5ba120d 100644 --- a/apps/pythinker-code/test/utils/recommended-effort-config.test.ts +++ b/apps/pythinker-code/test/utils/recommended-effort-config.test.ts @@ -2,7 +2,7 @@ import { mkdtemp, readFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { getRecommendedEffortConfig, @@ -32,7 +32,12 @@ async function makeCacheFile(): Promise { return join(dir, 'cache.json'); } +beforeEach(() => { + vi.stubEnv('CUSTOM_API_BASE_URL', 'https://api.example.test/coding/v1'); +}); + afterEach(async () => { + vi.unstubAllEnvs(); resetRecommendedEffortConfigCache(); await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); }); diff --git a/apps/pythinker-code/test/utils/survey-popup-config.test.ts b/apps/pythinker-code/test/utils/survey-popup-config.test.ts index 0b1a051d5..3ee8bc18f 100644 --- a/apps/pythinker-code/test/utils/survey-popup-config.test.ts +++ b/apps/pythinker-code/test/utils/survey-popup-config.test.ts @@ -2,13 +2,16 @@ import { mkdtemp, readFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { DEFAULT_SURVEY_POPUP_CONFIG, + DEFAULT_SURVEY_POPUP_PAYLOAD, getSurveyPopupConfig, peekSurveyPopupConfig, resetSurveyPopupConfigCache, + resolveSurveyPopupConfig, + type SurveyPopupPayload, } from '#/utils/survey-popup-config'; const CLOUD_CONFIG = { @@ -41,7 +44,12 @@ async function makeCacheFile(): Promise { return join(dir, 'cache.json'); } +beforeEach(() => { + vi.stubEnv('CUSTOM_API_BASE_URL', 'https://api.example.test/coding/v1'); +}); + afterEach(async () => { + vi.unstubAllEnvs(); resetSurveyPopupConfigCache(); await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); }); @@ -72,7 +80,7 @@ describe('getSurveyPopupConfig', () => { cacheFile: null, }); - expect(result).toEqual(CLOUD_CONFIG); + expect(result).toEqual({ ...CLOUD_CONFIG, model_overrides: {} }); expect(fetchImpl).toHaveBeenCalledWith( expect.stringContaining('/client_configs'), expect.objectContaining({ @@ -92,7 +100,7 @@ describe('getSurveyPopupConfig', () => { cacheFile: null, }); - expect(result).toEqual({ ...DEFAULT_SURVEY_POPUP_CONFIG, probability: 0.5 }); + expect(result).toEqual({ ...DEFAULT_SURVEY_POPUP_PAYLOAD, probability: 0.5 }); }); it('drops only the invalid field and keeps the rest', async () => { @@ -118,6 +126,7 @@ describe('getSurveyPopupConfig', () => { probability: DEFAULT_SURVEY_POPUP_CONFIG.probability, on_for_models: DEFAULT_SURVEY_POPUP_CONFIG.on_for_models, long_context_trigger_mode: DEFAULT_SURVEY_POPUP_CONFIG.long_context_trigger_mode, + model_overrides: {}, }); }); @@ -140,7 +149,7 @@ describe('getSurveyPopupConfig', () => { cacheFile: null, }); - expect(result).toEqual(DEFAULT_SURVEY_POPUP_CONFIG); + expect(result).toEqual(DEFAULT_SURVEY_POPUP_PAYLOAD); }); it('accepts zero pacing values as the documented no-limit semantics', async () => { @@ -163,7 +172,7 @@ describe('getSurveyPopupConfig', () => { }); expect(result).toEqual({ - ...DEFAULT_SURVEY_POPUP_CONFIG, + ...DEFAULT_SURVEY_POPUP_PAYLOAD, min_time_before_feedback_ms: 0, min_user_turns_before_feedback: 0, min_time_between_feedback_ms: 0, @@ -227,7 +236,7 @@ describe('getSurveyPopupConfig', () => { cacheFile: null, }); - expect(result).toEqual(DEFAULT_SURVEY_POPUP_CONFIG); + expect(result).toEqual(DEFAULT_SURVEY_POPUP_PAYLOAD); }); it('falls back to the defaults when the fetch fails', async () => { @@ -240,7 +249,7 @@ describe('getSurveyPopupConfig', () => { cacheFile: null, }); - expect(result).toEqual(DEFAULT_SURVEY_POPUP_CONFIG); + expect(result).toEqual(DEFAULT_SURVEY_POPUP_PAYLOAD); }); it('serves the in-process cache within a day and refetches after it', async () => { @@ -253,7 +262,7 @@ describe('getSurveyPopupConfig', () => { now: now + 60_000, cacheFile: null, }); - expect(cached).toEqual(CLOUD_CONFIG); + expect(cached).toEqual({ ...CLOUD_CONFIG, model_overrides: {} }); expect(fetchImpl).toHaveBeenCalledTimes(1); await getSurveyPopupConfig({ @@ -282,7 +291,7 @@ describe('getSurveyPopupConfig', () => { now: now + 60_000, cacheFile, }); - expect(result).toEqual(CLOUD_CONFIG); + expect(result).toEqual({ ...CLOUD_CONFIG, model_overrides: {} }); }); it('ignores a stale disk cache and falls back to the defaults when the refetch fails', async () => { @@ -298,13 +307,13 @@ describe('getSurveyPopupConfig', () => { now: now + 25 * 60 * 60 * 1000, cacheFile, }); - expect(result).toEqual(DEFAULT_SURVEY_POPUP_CONFIG); + expect(result).toEqual(DEFAULT_SURVEY_POPUP_PAYLOAD); }); }); describe('peekSurveyPopupConfig', () => { it('returns the defaults while the cache is cold', () => { - expect(peekSurveyPopupConfig()).toEqual(DEFAULT_SURVEY_POPUP_CONFIG); + expect(peekSurveyPopupConfig()).toEqual(DEFAULT_SURVEY_POPUP_PAYLOAD); }); it('sees the fetched config once the cache is warm', async () => { @@ -313,7 +322,139 @@ describe('peekSurveyPopupConfig', () => { await getSurveyPopupConfig({ fetchImpl: fetchImpl as typeof fetch, now, cacheFile: null }); - expect(peekSurveyPopupConfig(now + 60_000)).toEqual(CLOUD_CONFIG); - expect(peekSurveyPopupConfig(now + 25 * 60 * 60 * 1000)).toEqual(DEFAULT_SURVEY_POPUP_CONFIG); + expect(peekSurveyPopupConfig(now + 60_000)).toEqual({ ...CLOUD_CONFIG, model_overrides: {} }); + expect(peekSurveyPopupConfig(now + 25 * 60 * 60 * 1000)).toEqual(DEFAULT_SURVEY_POPUP_PAYLOAD); + }); +}); + +describe('model_overrides parsing', () => { + it('parses override entries keyed by model id', async () => { + const fetchImpl = vi.fn(async () => + jsonResponse({ + name: 'survey_popup', + config: { + ...CLOUD_CONFIG, + model_overrides: { k3: { probability: 0.5, min_user_turns_before_feedback: 1 } }, + }, + }), + ); + + const result = await getSurveyPopupConfig({ + fetchImpl: fetchImpl as typeof fetch, + cacheFile: null, + }); + + expect(result.model_overrides).toEqual({ + k3: { probability: 0.5, min_user_turns_before_feedback: 1 }, + }); + }); + + it('drops an entire override entry when any of its fields is invalid', async () => { + const fetchImpl = vi.fn(async () => + jsonResponse({ + name: 'survey_popup', + config: { + ...CLOUD_CONFIG, + model_overrides: { k3: { probability: 'often', min_time_between_feedback_ms: 60_000 } }, + }, + }), + ); + + const result = await getSurveyPopupConfig({ + fetchImpl: fetchImpl as typeof fetch, + cacheFile: null, + }); + + expect(result.model_overrides).toEqual({}); + }); + + it('keeps sibling entries when one entry is invalid', async () => { + const fetchImpl = vi.fn(async () => + jsonResponse({ + name: 'survey_popup', + config: { + ...CLOUD_CONFIG, + model_overrides: { + k3: { probability: 'often' }, + 'other-model': { probability: 0.5 }, + }, + }, + }), + ); + + const result = await getSurveyPopupConfig({ + fetchImpl: fetchImpl as typeof fetch, + cacheFile: null, + }); + + expect(result.model_overrides).toEqual({ 'other-model': { probability: 0.5 } }); + }); + + it('strips unknown keys inside an override entry', async () => { + const fetchImpl = vi.fn(async () => + jsonResponse({ + name: 'survey_popup', + config: { + ...CLOUD_CONFIG, + model_overrides: { k3: { probability: 0.5, on_for_models: ['nobody'], future_knob: 1 } }, + }, + }), + ); + + const result = await getSurveyPopupConfig({ + fetchImpl: fetchImpl as typeof fetch, + cacheFile: null, + }); + + expect(result.model_overrides).toEqual({ k3: { probability: 0.5 } }); + }); + + it('drops non-object entries and treats a non-object table as empty', async () => { + const fetchImpl = vi.fn(async () => + jsonResponse({ + name: 'survey_popup', + config: { ...CLOUD_CONFIG, model_overrides: { k3: 42, 'other-model': { probability: 0.5 } } }, + }), + ); + + const result = await getSurveyPopupConfig({ + fetchImpl: fetchImpl as typeof fetch, + cacheFile: null, + }); + expect(result.model_overrides).toEqual({ 'other-model': { probability: 0.5 } }); + + resetSurveyPopupConfigCache(); + const stringTable = vi.fn(async () => + jsonResponse({ name: 'survey_popup', config: { ...CLOUD_CONFIG, model_overrides: 'yes' } }), + ); + const fallback = await getSurveyPopupConfig({ + fetchImpl: stringTable as typeof fetch, + cacheFile: null, + }); + expect(fallback.model_overrides).toEqual({}); + }); +}); + +describe('resolveSurveyPopupConfig', () => { + const PAYLOAD: SurveyPopupPayload = { + ...CLOUD_CONFIG, + long_context_trigger_mode: 'virtual_context', + model_overrides: { k3: { probability: 0.5, min_user_turns_before_feedback: 1 } }, + }; + + it('returns the flat base config when no override matches the model', () => { + expect(resolveSurveyPopupConfig(PAYLOAD, 'k2')).toEqual(CLOUD_CONFIG); + }); + + it('returns the flat base config when the model id is undefined', () => { + expect(resolveSurveyPopupConfig(PAYLOAD, undefined)).toEqual(CLOUD_CONFIG); + }); + + it('merges the matching override over the base and keeps untouched base fields', () => { + expect(resolveSurveyPopupConfig(PAYLOAD, 'k3')).toEqual({ + ...CLOUD_CONFIG, + probability: 0.5, + min_user_turns_before_feedback: 1, + }); }); }); diff --git a/docs/reference/server-api.md b/docs/reference/server-api.md index 8274cc832..c9ebd7b73 100644 --- a/docs/reference/server-api.md +++ b/docs/reference/server-api.md @@ -1706,6 +1706,7 @@ Lists the entries of a session workspace directory, optionally recursing into su | `show_hidden` | body | boolean | Include dotfiles. Default `false` | | `follow_gitignore` | body | boolean | Skip gitignored paths. Default `true` | | `exclude_globs` | body | string[] | Additional globs to skip | +| `allow_ignored_globs` | body | string[] | Globs to show even when gitignored (e.g. `.tmp`). Ignored ancestor directories of a match are kept so the match stays reachable. Allowing dot-paths also requires `show_hidden: true` | | `sort` | body | string | `type_first` (default) / `name_asc` / `name_desc` / `mtime_desc` / `size_desc` | | `include_git_status` | body | boolean | Attach each entry's git status. Default `false` | diff --git a/packages/agent-core-v2/src/workspace/workspaceFs/fs.ts b/packages/agent-core-v2/src/workspace/workspaceFs/fs.ts index 36ab1c907..43dc65e7f 100644 --- a/packages/agent-core-v2/src/workspace/workspaceFs/fs.ts +++ b/packages/agent-core-v2/src/workspace/workspaceFs/fs.ts @@ -77,6 +77,7 @@ export const fsListRequestSchema = z.object({ show_hidden: z.boolean().default(false), follow_gitignore: z.boolean().default(true), exclude_globs: z.array(z.string()).optional(), + allow_ignored_globs: z.array(z.string()).optional(), sort: fsListSortSchema.default('type_first'), include_git_status: z.boolean().default(false), }); diff --git a/packages/agent-core-v2/src/workspace/workspaceFs/fsService.ts b/packages/agent-core-v2/src/workspace/workspaceFs/fsService.ts index 82d1f2f9c..b68432683 100644 --- a/packages/agent-core-v2/src/workspace/workspaceFs/fsService.ts +++ b/packages/agent-core-v2/src/workspace/workspaceFs/fsService.ts @@ -62,6 +62,7 @@ import { computeFuzzyScore, computeMatchPositions, evaluateSuggestCandidate, + globCanMatchBelow, matchesAnyGlob, type RgJsonRecord, rgPath, @@ -74,6 +75,8 @@ import { } from './internal/fsSearch'; const SEARCH_HARD_CAP = 500; +const PROBE_MAX_DEPTH = 6; +const PROBE_MAX_ENTRIES = 500; const GREP_TIMEOUT_MS = 30_000; const SUGGEST_TIMEOUT_MS = 10_000; const SUGGEST_WALK_ABORTED = new Error('suggest walk aborted'); @@ -142,8 +145,8 @@ export class WorkspaceFsService implements IWorkspaceFsService { let topStat: HostFileStat; try { topStat = await this.hostFs.stat(abs); - } catch (err) { - throw mapFsError(err, req.path); + } catch (error) { + throw mapFsError(error, req.path); } if (!topStat.isDirectory) { throw new Error2(ErrorCodes.FS_PATH_NOT_FOUND, `path not found: ${req.path}`, { @@ -155,6 +158,7 @@ export class WorkspaceFsService implements IWorkspaceFsService { const items: FsEntry[] = []; const childrenByPath: Record = {}; + const probeBudget = { visited: 0 }; let truncated = false; interface QueueEntry { @@ -176,9 +180,9 @@ export class WorkspaceFsService implements IWorkspaceFsService { let names: readonly string[]; try { names = (await this.hostFs.readdir(this.absOf(entry.relPath))).map((e) => e.name); - } catch (err) { + } catch (error) { if (entry.relPath === (rel === '.' ? '' : rel)) { - throw mapFsError(err, req.path); + throw mapFsError(error, req.path); } continue; } @@ -187,12 +191,21 @@ export class WorkspaceFsService implements IWorkspaceFsService { for (const name of names) { if (!req.show_hidden && isHidden(name)) continue; const childRel = entry.relPath === '' ? name : `${entry.relPath}/${name}`; + let ancestorOfAllowed = false; if (gitignore && (gitignore.ignores(childRel) || gitignore.ignores(`${childRel}/`))) { - continue; + if (req.allow_ignored_globs === undefined) continue; + if (!matchesAnyGlob(childRel, req.allow_ignored_globs)) { + if (!globCanMatchBelow(childRel, req.allow_ignored_globs)) continue; + ancestorOfAllowed = true; + } } if (req.exclude_globs && matchesAnyGlob(childRel, req.exclude_globs)) continue; const st = await this.hostFs.lstat(this.absOf(childRel)).catch(() => undefined); - if (st === undefined) continue; + if (st === undefined || (ancestorOfAllowed && !st.isDirectory)) continue; + if ( + ancestorOfAllowed && + !(await this.hasAllowedDescendant(childRel, req.allow_ignored_globs!, req.exclude_globs, req.show_hidden, probeBudget)) + ) continue; visible.push({ name, relPath: childRel, stat: st }); } @@ -227,6 +240,38 @@ export class WorkspaceFsService implements IWorkspaceFsService { return response; } + private async hasAllowedDescendant( + relDir: string, + globs: readonly string[], + excludeGlobs: readonly string[] | undefined, + showHidden: boolean, + budget: { visited: number }, + ): Promise { + const walk = async (rel: string, depth: number): Promise => { + if (depth > PROBE_MAX_DEPTH || budget.visited >= PROBE_MAX_ENTRIES) return true; + let names: readonly string[]; + try { + names = (await this.hostFs.readdir(this.absOf(rel))).map((e) => e.name); + } catch { + return false; + } + for (const name of names) { + if (budget.visited >= PROBE_MAX_ENTRIES) return true; + budget.visited += 1; + if (!showHidden && isHidden(name)) continue; + const childRel = `${rel}/${name}`; + if (excludeGlobs !== undefined && matchesAnyGlob(childRel, excludeGlobs)) continue; + if (matchesAnyGlob(childRel, globs)) return true; + const st = await this.hostFs.lstat(this.absOf(childRel)).catch(() => undefined); + if (st?.isDirectory === true && globCanMatchBelow(childRel, globs)) { + if (await walk(childRel, depth + 1)) return true; + } + } + return false; + }; + return walk(relDir, 1); + } + async read(req: FsReadRequest): Promise { const abs = await this.resolveWithin(req.path); const rel = this.toRel(abs); @@ -234,8 +279,8 @@ export class WorkspaceFsService implements IWorkspaceFsService { let st: HostFileStat; try { st = await this.hostFs.stat(abs); - } catch (err) { - throw mapFsError(err, req.path); + } catch (error) { + throw mapFsError(error, req.path); } if (st.isDirectory) { throw new Error2(ErrorCodes.FS_IS_DIRECTORY, `path is a directory: ${req.path}`, { @@ -333,9 +378,9 @@ export class WorkspaceFsService implements IWorkspaceFsService { }); results[p] = sub.items; if (sub.truncated) truncatedPaths.push(p); - } catch (err) { - if (err instanceof Error2 && err.code === ErrorCodes.FS_PATH_ESCAPES) throw err; - partialErrors[p] = toWireError(err); + } catch (error) { + if (error instanceof Error2 && error.code === ErrorCodes.FS_PATH_ESCAPES) throw error; + partialErrors[p] = toWireError(error); } }), ); @@ -352,8 +397,8 @@ export class WorkspaceFsService implements IWorkspaceFsService { let st: HostFileStat; try { st = await this.hostFs.lstat(abs); - } catch (err) { - throw mapFsError(err, req.path); + } catch (error) { + throw mapFsError(error, req.path); } const name = rel === '.' ? this.path.basename(this.workDir) : this.path.basename(abs); return buildFsEntry(rel, name, st, true); @@ -387,8 +432,8 @@ export class WorkspaceFsService implements IWorkspaceFsService { const rel = this.toRel(abs); try { await this.hostFs.mkdir(abs, { recursive: req.recursive }); - } catch (err) { - const code = errnoCode(err); + } catch (error) { + const code = errnoCode(error); if (code === 'EEXIST') { throw new Error2(ErrorCodes.FS_ALREADY_EXISTS, `path already exists: ${req.path}`, { details: { path: req.path }, @@ -399,7 +444,7 @@ export class WorkspaceFsService implements IWorkspaceFsService { details: { path: req.path }, }); } - throw err; + throw error; } const st = await this.hostFs.lstat(abs); return buildFsEntry(rel, this.path.basename(abs), st, false); @@ -411,8 +456,8 @@ export class WorkspaceFsService implements IWorkspaceFsService { let st: HostFileStat; try { st = await this.hostFs.lstat(abs); - } catch (err) { - throw mapFsError(err, relPath); + } catch (error) { + throw mapFsError(error, relPath); } return { absolute: abs, relative: rel, isDirectory: st.isDirectory }; } @@ -423,8 +468,8 @@ export class WorkspaceFsService implements IWorkspaceFsService { let st: HostFileStat; try { st = await this.hostFs.stat(abs); - } catch (err) { - throw mapFsError(err, relPath); + } catch (error) { + throw mapFsError(error, relPath); } if (st.isDirectory) { throw new Error2(ErrorCodes.FS_IS_DIRECTORY, `path is a directory: ${relPath}`, { @@ -541,8 +586,8 @@ export class WorkspaceFsService implements IWorkspaceFsService { if (resolution !== null) { try { return await this.suggestWithRg(query, cap, controller.signal, resolution.path, roots); - } catch (err) { - if (controller.signal.aborted) throw err; + } catch (error) { + if (controller.signal.aborted) throw error; this.telemetry.track2('fs_suggest_node_fallback', { reason: 'rg_error' }); return await this.suggestWithNode(query, cap, controller.signal, roots); } @@ -603,8 +648,8 @@ export class WorkspaceFsService implements IWorkspaceFsService { let entries: readonly HostDirEntry[]; try { entries = await this.hostFs.readdir(root.dir); - } catch (err) { - throw mapFsError(err, root.dir); + } catch (error) { + throw mapFsError(error, root.dir); } const visible: { name: string; kind: TopEntry['kind'] }[] = []; for (const entry of entries) { @@ -823,8 +868,8 @@ export class WorkspaceFsService implements IWorkspaceFsService { top.push(this.displayCandidate(root, candidate)); }); } - } catch (err) { - if (err !== SUGGEST_WALK_ABORTED) throw err; + } catch (error) { + if (error !== SUGGEST_WALK_ABORTED) throw error; } const items = top.drain().map((candidate) => ({ path: candidate.path, @@ -1119,9 +1164,9 @@ export class WorkspaceFsService implements IWorkspaceFsService { for (let i = 0; i < 256; i++) { try { const real = await this.hostFs.realpath(current); - return tail.length === 0 ? real : this.path.join(real, ...tail.reverse()); - } catch (err) { - if (!isMissingPathError(err)) throw err; + return tail.length === 0 ? real : this.path.join(real, ...tail.toReversed()); + } catch (error) { + if (!isMissingPathError(error)) throw error; const parent = this.path.dirname(current); if (parent === current) return abs; tail.push(this.path.basename(current)); @@ -1268,7 +1313,7 @@ class RgJsonAccumulator { const buf = this.fileBuf.get(p); if (buf === undefined) return; if (buf.matches.length > 0 && buf.pending.length > 0) { - const last = buf.matches[buf.matches.length - 1]!; + const last = buf.matches.at(-1)!; last.after = buf.pending.slice(0, this.req.context_lines); } if (buf.matches.length > 0) { diff --git a/packages/agent-core-v2/src/workspace/workspaceFs/internal/fsSearch.ts b/packages/agent-core-v2/src/workspace/workspaceFs/internal/fsSearch.ts index 483022343..1444ca481 100644 --- a/packages/agent-core-v2/src/workspace/workspaceFs/internal/fsSearch.ts +++ b/packages/agent-core-v2/src/workspace/workspaceFs/internal/fsSearch.ts @@ -1,3 +1,5 @@ +import picomatch from 'picomatch'; + import type { FsGrepRequest } from '../fs'; export function computeFuzzyScore(name: string, queryLower: string): number { @@ -39,37 +41,46 @@ export function computeMatchPositions( } export function matchesAnyGlob(rel: string, globs: readonly string[]): boolean { + const valid = globs.filter((g) => g !== ''); + return valid.length > 0 && picomatch.isMatch(rel, valid, { dot: true, nonegate: true }); +} + +export function globCanMatchBelow(rel: string, globs: readonly string[]): boolean { + const relSegments = rel.split('/'); for (const g of globs) { - if (globToRegExp(g).test(rel)) return true; + const parts = picomatch.scan(g, { parts: true, nonegate: true }).parts ?? []; + if (parts.length === 0) { + if (g === '**' || g.includes('/')) return true; + continue; + } + if (parts.some((part) => part.includes('/'))) return true; + if (globSegmentsMatchPrefix(parts, relSegments)) return true; } return false; } -function globToRegExp(glob: string): RegExp { - let re = '^'; - let i = 0; - while (i < glob.length) { - const ch = glob[i]!; - if (ch === '*' && glob[i + 1] === '*') { - re += '.*'; - i += 2; - if (glob[i] === '/') i++; - } else if (ch === '*') { - re += '[^/]*'; - i++; - } else if (ch === '?') { - re += '[^/]'; - i++; - } else if (/[.+^${}()|[\]\\]/.test(ch)) { - re += `\\${ch}`; - i++; - } else { - re += ch; - i++; +function globSegmentsMatchPrefix(globSegments: readonly string[], relSegments: readonly string[]): boolean { + const g = globSegments.length; + const r = relSegments.length; + const width = r + 1; + const dp = new Uint8Array((g + 1) * width); + for (let gi = g; gi >= 0; gi--) { + const head = gi < g ? globSegments[gi]! : undefined; + const headMatch = head !== undefined && head !== '**' && head !== '' ? picomatch(head, { dot: true, nonegate: true }) : undefined; + for (let ri = r; ri >= 0; ri--) { + const at = gi * width + ri; + if (ri === r) { + dp[at] = 1; + } else if (head === '**') { + dp[at] = dp[(gi + 1) * width + ri]! | dp[gi * width + ri + 1]!; + } else if (headMatch === undefined) { + dp[at] = 0; + } else { + dp[at] = headMatch(relSegments[ri]!) ? dp[(gi + 1) * width + ri + 1]! : 0; + } } } - re += '$'; - return new RegExp(re); + return dp[0] === 1; } export function compileGrepPattern(req: FsGrepRequest): RegExp { @@ -79,7 +90,7 @@ export function compileGrepPattern(req: FsGrepRequest): RegExp { } function escapeRegExp(s: string): string { - return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return s.replaceAll(/[.*+?^${}()|[\]\\]/g, '\\$&'); } export function stripTrailingNewline(s: string): string { @@ -188,7 +199,7 @@ function matchSuggestName(name: string, queryLower: string): SuggestMatch | null if (positions === null) return null; const nameLower = name.toLowerCase(); const tier = nameLower === queryLower ? 3 : nameLower.startsWith(queryLower) ? 2 : 1; - const span = positions[positions.length - 1]! - positions[0]! + 1; + const span = positions.at(-1)! - positions[0]! + 1; return { tier, span, positions }; } @@ -227,7 +238,7 @@ function matchSuggestPath(path: string, querySegments: readonly string[]): Sugge : lastSeg === pathSegments.length - 1 && lastSegPrefix ? 2 : 1; - const span = positions[positions.length - 1]! - positions[0]! + 1; + const span = positions.at(-1)! - positions[0]! + 1; return { tier, span, positions }; } @@ -239,7 +250,7 @@ export function evaluateSuggestCandidate( const segments = relPath.split('/'); if (segments.some((s) => VCS_METADATA_DIRS.has(s))) return null; if (!query.showHidden && segments.some((s) => s.startsWith('.'))) return null; - const name = segments[segments.length - 1]!; + const name = segments.at(-1)!; const pathMode = query.pathSegments.length > 0; const match = pathMode ? matchSuggestPath(relPath, query.pathSegments) @@ -302,7 +313,7 @@ export class SuggestTopHeap { } drain(): SuggestCandidate[] { - return this.heap.slice().sort(compareSuggestCandidates); + return this.heap.slice().toSorted(compareSuggestCandidates); } private siftUp(index: number): void { diff --git a/packages/agent-core-v2/test/workspace/workspaceFs/fsSearch.test.ts b/packages/agent-core-v2/test/workspace/workspaceFs/fsSearch.test.ts index 8617f16b0..d532a4668 100644 --- a/packages/agent-core-v2/test/workspace/workspaceFs/fsSearch.test.ts +++ b/packages/agent-core-v2/test/workspace/workspaceFs/fsSearch.test.ts @@ -4,6 +4,7 @@ import { compileGrepPattern, computeFuzzyScore, computeMatchPositions, + globCanMatchBelow, matchesAnyGlob, rgPath, stripTrailingNewline, @@ -45,6 +46,79 @@ describe('matchesAnyGlob', () => { expect(matchesAnyGlob('src/a.ts', ['**/*.ts'])).toBe(true); expect(matchesAnyGlob('src/a.js', ['**/*.ts'])).toBe(false); }); + + it('preserves segment boundaries around recursive wildcards', () => { + expect(matchesAnyGlob('ignored-dir/keep.txt', ['ignored-dir/**/keep.txt'])).toBe(true); + expect(matchesAnyGlob('ignored-dir/sub/keep.txt', ['ignored-dir/**/keep.txt'])).toBe(true); + expect(matchesAnyGlob('ignored-dir/notkeep.txt', ['ignored-dir/**/keep.txt'])).toBe(false); + expect(matchesAnyGlob('keep.txt', ['**/keep.txt'])).toBe(true); + expect(matchesAnyGlob('notkeep.txt', ['**/keep.txt'])).toBe(false); + expect(matchesAnyGlob('a/xxb', ['a/**/b'])).toBe(false); + }); + + it('treats globstars inside a path segment as within-segment wildcards', () => { + expect(matchesAnyGlob('foo/keep.txt', ['foo**/keep.txt'])).toBe(true); + expect(matchesAnyGlob('fooX/keep.txt', ['foo**/keep.txt'])).toBe(true); + expect(matchesAnyGlob('foo/sub/keep.txt', ['foo**/keep.txt'])).toBe(false); + expect(matchesAnyGlob('foobar', ['foo**'])).toBe(true); + expect(matchesAnyGlob('foo/bar', ['foo**'])).toBe(false); + }); +}); + +describe('globCanMatchBelow', () => { + it('detects a literal-prefix ancestor of a nested glob', () => { + expect(globCanMatchBelow('ignored-dir', ['ignored-dir/keep.txt'])).toBe(true); + expect(globCanMatchBelow('ignored-dir', ['ignored-dir/**'])).toBe(true); + expect(globCanMatchBelow('a', ['a/b/c'])).toBe(true); + expect(globCanMatchBelow('a/b', ['a/b/c'])).toBe(true); + }); + + it('detects ancestors across wildcard segments', () => { + expect(globCanMatchBelow('ignored-a', ['ignored-*/keep.txt'])).toBe(true); + expect(globCanMatchBelow('ignored-dir/sub', ['ignored-dir/**/keep.txt'])).toBe(true); + expect(globCanMatchBelow('ignored-dir', ['ignored-dir/**/keep.txt'])).toBe(true); + expect(globCanMatchBelow('src', ['**/keep.txt'])).toBe(true); + }); + + it('rejects non-ancestors and non-matching segments', () => { + expect(globCanMatchBelow('ignored', ['ignored-dir/keep.txt'])).toBe(false); + expect(globCanMatchBelow('other-dir', ['ignored-dir/keep.txt'])).toBe(false); + expect(globCanMatchBelow('src', ['*.txt'])).toBe(false); + expect(globCanMatchBelow('ignored-b', ['ignored-a/keep.txt'])).toBe(false); + expect(globCanMatchBelow('ignored-dir/sub', ['ignored-dir/nested/keep.txt'])).toBe(false); + }); + + it('handles brace and single-segment constructs via whole-pattern parsing', () => { + expect(globCanMatchBelow('ignored-a', ['{ignored-a/keep.txt,other}'])).toBe(true); + expect(globCanMatchBelow('a', ['{a,b}/keep.txt'])).toBe(true); + expect(globCanMatchBelow('c', ['{a,b}/keep.txt'])).toBe(false); + expect(globCanMatchBelow('src', ['keep.txt'])).toBe(false); + expect(globCanMatchBelow('anything', ['**'])).toBe(true); + expect(globCanMatchBelow('a', ['{a/b,c}/x'])).toBe(true); + }); + + it('treats empty patterns as non-matching', () => { + expect(matchesAnyGlob('keep.txt', [''])).toBe(false); + expect(matchesAnyGlob('a.ts', ['', '*.ts'])).toBe(true); + expect(globCanMatchBelow('src', [''])).toBe(false); + }); + + it('treats leading exclamation marks as literal characters', () => { + expect(matchesAnyGlob('!foo', ['!foo'])).toBe(true); + expect(matchesAnyGlob('foo', ['!foo'])).toBe(false); + expect(matchesAnyGlob('bar', ['!foo'])).toBe(false); + expect(globCanMatchBelow('!foo', ['!foo/keep.txt'])).toBe(true); + }); + + it('treats empty path segments as non-matching without throwing', () => { + expect(globCanMatchBelow('a', ['a//b'])).toBe(true); + expect(globCanMatchBelow('a/b', ['a//b'])).toBe(false); + }); + + it('handles deeply segmented globs without exhausting the call stack', () => { + expect(globCanMatchBelow('a/b', [`${'**/'.repeat(5000)}keep.txt`])).toBe(true); + expect(globCanMatchBelow('a/b', [`${'x/'.repeat(5000)}keep.txt`])).toBe(false); + }); }); describe('compileGrepPattern', () => { diff --git a/packages/agent-gateway/test/fs.test.ts b/packages/agent-gateway/test/fs.test.ts index 241a38ff9..a76ed6660 100644 --- a/packages/agent-gateway/test/fs.test.ts +++ b/packages/agent-gateway/test/fs.test.ts @@ -201,11 +201,147 @@ describe('server-v2 /api/v1 fs routes', () => { const id = await createSession(); const body = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', {}); expect(body.code).toBe(0); - const names = body.data.items.map((i) => i.name).sort(); + const names = body.data.items.map((i) => i.name).toSorted(); expect(names).toEqual(['a.txt', 'b.txt']); expect(body.data.truncated).toBe(false); }); + it('fs:list keeps gitignored paths hidden unless allowed via allow_ignored_globs', async () => { + await writeFile(join(work!, '.gitignore'), 'ignored-dir/\n.tmp/\n'); + await mkdir(join(work!, 'ignored-dir')); + await mkdir(join(work!, '.tmp')); + await writeFile(join(work!, '.tmp/keep.txt'), ''); + await writeFile(join(work!, 'visible.txt'), ''); + const id = await createSession(); + + const filtered = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', {}); + const filteredNames = filtered.data.items.map((i) => i.name); + expect(filteredNames).not.toContain('ignored-dir'); + expect(filteredNames).not.toContain('.tmp'); + expect(filteredNames).toContain('visible.txt'); + + const allowed = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + show_hidden: true, + allow_ignored_globs: ['ignored-dir', '.tmp', '.tmp/**'], + }); + const allowedNames = allowed.data.items.map((i) => i.name); + expect(allowedNames).toContain('ignored-dir'); + expect(allowedNames).toContain('.tmp'); + expect(allowedNames).toContain('visible.txt'); + + const children = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + path: '.tmp', + show_hidden: true, + allow_ignored_globs: ['.tmp', '.tmp/**'], + }); + expect(children.data.items.map((i) => i.name)).toContain('keep.txt'); + }); + + it('fs:list traverses ignored ancestor directories of allowed matches', async () => { + await writeFile(join(work!, '.gitignore'), 'ignored-dir/\n'); + await mkdir(join(work!, 'ignored-dir/nested'), { recursive: true }); + await writeFile(join(work!, 'ignored-dir/nested/keep.txt'), ''); + await writeFile(join(work!, 'ignored-dir/drop.txt'), ''); + await writeFile(join(work!, 'visible.txt'), ''); + const id = await createSession(); + const globs = ['ignored-dir/nested/keep.txt']; + + const root = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + allow_ignored_globs: globs, + }); + const rootNames = root.data.items.map((i) => i.name); + expect(rootNames).toContain('ignored-dir'); + expect(rootNames).toContain('visible.txt'); + + const mid = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + path: 'ignored-dir', + allow_ignored_globs: globs, + }); + expect(mid.data.items.map((i) => i.name)).toEqual(['nested']); + + const leaf = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + path: 'ignored-dir/nested', + allow_ignored_globs: globs, + }); + expect(leaf.data.items.map((i) => i.name)).toEqual(['keep.txt']); + + const recursive = await postFs<{ + items: FsEntryWire[]; + truncated: boolean; + children_by_path?: Record; + }>(id, 'list', { depth: 3, allow_ignored_globs: globs }); + const descendants = Object.values(recursive.data.children_by_path ?? {}) + .flat() + .map((i) => i.path); + expect(descendants).toContain('ignored-dir/nested'); + expect(descendants).toContain('ignored-dir/nested/keep.txt'); + expect(descendants).not.toContain('ignored-dir/drop.txt'); + }); + + it('fs:list does not surface a non-directory prefix of an allowed glob', async () => { + await writeFile(join(work!, '.gitignore'), 'data.bin\n'); + await writeFile(join(work!, 'data.bin'), ''); + const id = await createSession(); + + const body = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + allow_ignored_globs: ['data.bin/keep.txt'], + }); + expect(body.data.items.map((i) => i.name)).not.toContain('data.bin'); + }); + + it('fs:list traverses ignored ancestors across wildcard glob segments', async () => { + await writeFile(join(work!, '.gitignore'), 'ignored-a/\nignored-b/\nignored-dir/\nother-dir/\n'); + await mkdir(join(work!, 'ignored-a'), { recursive: true }); + await mkdir(join(work!, 'ignored-b'), { recursive: true }); + await mkdir(join(work!, 'ignored-dir/sub'), { recursive: true }); + await mkdir(join(work!, 'other-dir'), { recursive: true }); + await writeFile(join(work!, 'ignored-a/keep.txt'), ''); + await writeFile(join(work!, 'ignored-b/drop.txt'), ''); + await writeFile(join(work!, 'ignored-dir/sub/keep.txt'), ''); + await writeFile(join(work!, 'ignored-dir/sub/notkeep.txt'), ''); + await writeFile(join(work!, 'ignored-dir/drop.txt'), ''); + await writeFile(join(work!, 'other-dir/drop.txt'), ''); + const id = await createSession(); + + const wildcard = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + allow_ignored_globs: ['ignored-*/keep.txt'], + }); + const wildcardNames = wildcard.data.items.map((i) => i.name); + expect(wildcardNames).toContain('ignored-a'); + expect(wildcardNames).not.toContain('ignored-b'); + expect(wildcardNames).not.toContain('other-dir'); + + const mid = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + path: 'ignored-dir', + allow_ignored_globs: ['ignored-dir/**/keep.txt'], + }); + expect(mid.data.items.map((i) => i.name)).toEqual(['sub']); + + const leaf = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + path: 'ignored-dir/sub', + allow_ignored_globs: ['ignored-dir/**/keep.txt'], + }); + expect(leaf.data.items.map((i) => i.name)).toEqual(['keep.txt']); + + const brace = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + allow_ignored_globs: ['{ignored-a/keep.txt,other}'], + }); + const braceNames = brace.data.items.map((i) => i.name); + expect(braceNames).toContain('ignored-a'); + expect(braceNames).not.toContain('ignored-b'); + }); + + it('fs:list tolerates empty glob patterns', async () => { + await writeFile(join(work!, 'a.txt'), ''); + const id = await createSession(); + const body = await postFs<{ items: FsEntryWire[]; truncated: boolean }>(id, 'list', { + exclude_globs: [''], + allow_ignored_globs: [''], + }); + expect(body.code).toBe(0); + expect(body.data.items.map((i) => i.name)).toContain('a.txt'); + }); + it('fs:mkdir creates a directory and rejects duplicates', async () => { const id = await createSession(); const created = await postFs(id, 'mkdir', { path: 'sub' }); @@ -580,13 +716,13 @@ describe('server-v2 /api/v1 fs routes', () => { expect(body.code).toBe(0); expect(body.data.items.map((i) => i.path)).toContain('kappa.ts'); - const workAliases = [work!, await realpath(work!)]; - expect((await listWorkspaces()).some((w) => workAliases.includes(w.root))).toBe(false); + const workAliases = new Set([work!, await realpath(work!)]); + expect((await listWorkspaces()).some((w) => workAliases.has(w.root))).toBe(false); expect( server!.core.accessor .get(IWorkspaceInstanceManager) .list() - .some((w) => workAliases.includes(w.root)), + .some((w) => workAliases.has(w.root)), ).toBe(false); const again = await postRootSuggest<{ items: SuggestItemWire[] }>({ @@ -594,7 +730,7 @@ describe('server-v2 /api/v1 fs routes', () => { query: 'kappa', }); expect(again.code).toBe(0); - expect((await listWorkspaces()).some((w) => workAliases.includes(w.root))).toBe(false); + expect((await listWorkspaces()).some((w) => workAliases.has(w.root))).toBe(false); }); it('fs:suggest matches the workspace route for the same single root', async () => {