Skip to content

Commit b2fab80

Browse files
quanruclaude
andauthored
feat(core): unify processCacheConfig between CLI and Agent (#1320)
* feat(core,cli): unify processCacheConfig and support cache strategy - Unified processCacheConfig between CLI and Agent to handle MIDSCENE_CACHE env variable consistently - Added strategy parameter to support YAML configurations with separate cache and strategy properties - Added strategy property to AgentOpt interface for proper type support - Updated CLI to pass strategy parameter when processing cache config - Added comprehensive unit tests for processCacheConfig (18 tests) - Verified read-only cache strategy prevents cache file updates This ensures backward compatibility with MIDSCENE_CACHE environment variable while supporting new cache strategy configurations (read-only, read-write, write-only). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * feat(core): unify processCacheConfig between CLI and Agent This commit unifies the cache configuration processing logic between CLI and Agent to ensure consistent handling of the MIDSCENE_CACHE environment variable and cache strategies. Changes: - Moved processCacheConfig to core/utils.ts for shared use - Added support for cache strategies (read-only, read-write, write-only) - Implemented auto-generation of cache IDs for CLI/YAML scenarios - Added validation in Agent to reject cache: true without explicit ID - Maintained backward compatibility with legacy cacheId format - Added comprehensive unit tests for both CLI and Agent The key design decision is that processCacheConfig utility function auto-generates IDs (needed for CLI/YAML), while Agent's processCacheConfig method validates and rejects invalid configs (needed for direct API usage). This ensures: 1. CLI respects MIDSCENE_CACHE environment variable 2. YAML scripts can use cache: true with auto-generated IDs 3. Direct Agent API usage requires explicit cache IDs for safety 4. Read-only cache strategy prevents unintended cache updates All tests passing: - 30 Agent tests - 18 CLI processCacheConfig tests - 7 CLI create-yaml-player tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix(core): mark unused parameter with underscore prefix Fix TypeScript warning for unused 'key' parameter in replacerForPageObject. The parameter is required by JSON.stringify's replacer function signature but not used in this implementation. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6b50c79 commit b2fab80

8 files changed

Lines changed: 407 additions & 97 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ CLAUDE.md
121121
.claude
122122
**/.claude
123123
**/CLAUDE.md
124+
AGENTS.md
124125
.cursor/rules/nx-rules.mdc
125126
.github/instructions/nx.instructions.md
126127
.gemini-clipboard

‎packages/cli/src/create-yaml-player.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,11 @@ export async function createYamlPlayer(
122122
webTarget,
123123
{
124124
...preference,
125-
cache: processCacheConfig(yamlScript.agent?.cache, fileName),
125+
cache: processCacheConfig(
126+
yamlScript.agent?.cache,
127+
fileName,
128+
fileName,
129+
),
126130
},
127131
options?.browser,
128132
);
@@ -151,7 +155,11 @@ export async function createYamlPlayer(
151155

152156
const agent = new AgentOverChromeBridge({
153157
closeNewTabsAfterDisconnect: webTarget.closeNewTabsAfterDisconnect,
154-
cache: processCacheConfig(yamlScript.agent?.cache, fileName),
158+
cache: processCacheConfig(
159+
yamlScript.agent?.cache,
160+
fileName,
161+
fileName,
162+
),
155163
});
156164

157165
if (webTarget.bridgeMode === 'newTabWithUrl') {
@@ -178,7 +186,11 @@ export async function createYamlPlayer(
178186
if (typeof yamlScript.android !== 'undefined') {
179187
const androidTarget = yamlScript.android;
180188
const agent = await agentFromAdbDevice(androidTarget?.deviceId, {
181-
cache: processCacheConfig(yamlScript.agent?.cache, fileName),
189+
cache: processCacheConfig(
190+
yamlScript.agent?.cache,
191+
fileName,
192+
fileName,
193+
),
182194
});
183195

184196
if (androidTarget?.launch) {
@@ -258,7 +270,11 @@ export async function createYamlPlayer(
258270
debug('creating agent from device', device);
259271
const agent = createAgent(device, {
260272
...yamlScript.agent,
261-
cache: processCacheConfig(yamlScript.agent?.cache, fileName),
273+
cache: processCacheConfig(
274+
yamlScript.agent?.cache,
275+
fileName,
276+
fileName,
277+
),
262278
});
263279

264280
freeFn.push({
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
target:
22
serve: ./tests/server_root
33
url: index.html
4+
agent:
5+
cache: true
6+
strategy: read-only
47
tasks:
8+
- name: click title
9+
flow:
10+
- aiTap: click the title
511
- name: check title
612
flow:
713
- aiAssert: the content title is "My App"

‎packages/cli/tests/unit-test/create-yaml-player.test.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@ import { createYamlPlayer, launchServer } from '@/create-yaml-player';
33
import type { MidsceneYamlScript, MidsceneYamlScriptEnv } from '@midscene/core';
44
import { beforeEach, describe, expect, test, vi } from 'vitest';
55

6+
// Mock the global config manager to control environment variables
7+
vi.mock('@midscene/shared/env', () => ({
8+
MIDSCENE_CACHE: 'MIDSCENE_CACHE',
9+
globalConfigManager: {
10+
getEnvConfigInBoolean: vi.fn(),
11+
},
12+
}));
13+
614
// Mock dependencies
715
vi.mock('node:fs', () => ({
816
readFileSync: vi.fn(),
@@ -33,6 +41,8 @@ import { ScriptPlayer, parseYamlScript } from '@midscene/core/yaml';
3341
import { createServer } from 'http-server';
3442

3543
describe('create-yaml-player', () => {
44+
const mockFilePath = '/test/script.yml';
45+
3646
beforeEach(() => {
3747
vi.clearAllMocks();
3848
});
@@ -70,8 +80,6 @@ describe('create-yaml-player', () => {
7080
});
7181

7282
describe('createYamlPlayer', () => {
73-
const mockFilePath = '/test/script.yml';
74-
7583
test('should create player with web target', async () => {
7684
const mockScript: MidsceneYamlScript = {
7785
web: {
Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
import type { Cache } from '@midscene/core';
2+
import { processCacheConfig } from '@midscene/core/utils';
3+
import { beforeEach, describe, expect, test, vi } from 'vitest';
4+
5+
// Mock the global config manager to control environment variables
6+
vi.mock('@midscene/shared/env', () => ({
7+
MIDSCENE_CACHE: 'MIDSCENE_CACHE',
8+
globalConfigManager: {
9+
getEnvConfigInBoolean: vi.fn(),
10+
},
11+
}));
12+
13+
import { globalConfigManager } from '@midscene/shared/env';
14+
15+
describe('processCacheConfig in CLI', () => {
16+
beforeEach(() => {
17+
vi.clearAllMocks();
18+
});
19+
20+
describe('Basic cache configuration', () => {
21+
test('should return cache object with ID when cache config is provided with ID', () => {
22+
const cacheConfig: Cache = {
23+
id: 'test-cache-id',
24+
strategy: 'read-write',
25+
};
26+
const result = processCacheConfig(cacheConfig, 'fallback-id');
27+
28+
expect(result).toEqual({
29+
id: 'test-cache-id',
30+
strategy: 'read-write',
31+
});
32+
});
33+
34+
test('should auto-generate ID when cache config is true', () => {
35+
const result = processCacheConfig(true, 'fallback-id');
36+
37+
expect(result).toEqual({
38+
id: 'fallback-id',
39+
});
40+
});
41+
42+
test('should auto-generate ID when cache config object has no ID', () => {
43+
const cacheConfig: Cache = { strategy: 'read-only' };
44+
const result = processCacheConfig(cacheConfig, 'fallback-id');
45+
46+
expect(result).toEqual({
47+
id: 'fallback-id',
48+
strategy: 'read-only',
49+
});
50+
});
51+
52+
test('should return undefined when cache config is false', () => {
53+
const result = processCacheConfig(false, 'fallback-id');
54+
55+
expect(result).toBeUndefined();
56+
});
57+
58+
test('should return undefined when cache config is undefined', () => {
59+
const result = processCacheConfig(undefined, 'fallback-id');
60+
61+
expect(result).toBeUndefined();
62+
});
63+
});
64+
65+
describe('Environment variable support (MIDSCENE_CACHE)', () => {
66+
test('should enable legacy cacheId when MIDSCENE_CACHE is true', () => {
67+
vi.mocked(globalConfigManager.getEnvConfigInBoolean).mockReturnValue(
68+
true,
69+
);
70+
71+
const result = processCacheConfig(
72+
undefined,
73+
'fallback-id',
74+
'legacy-cache-id',
75+
);
76+
77+
expect(globalConfigManager.getEnvConfigInBoolean).toHaveBeenCalledWith(
78+
'MIDSCENE_CACHE',
79+
);
80+
expect(result).toEqual({
81+
id: 'legacy-cache-id',
82+
});
83+
});
84+
85+
test('should disable legacy cacheId when MIDSCENE_CACHE is false', () => {
86+
vi.mocked(globalConfigManager.getEnvConfigInBoolean).mockReturnValue(
87+
false,
88+
);
89+
90+
const result = processCacheConfig(
91+
undefined,
92+
'fallback-id',
93+
'legacy-cache-id',
94+
);
95+
96+
expect(globalConfigManager.getEnvConfigInBoolean).toHaveBeenCalledWith(
97+
'MIDSCENE_CACHE',
98+
);
99+
expect(result).toBeUndefined();
100+
});
101+
102+
test('should prefer new cache config over legacy cacheId', () => {
103+
vi.mocked(globalConfigManager.getEnvConfigInBoolean).mockReturnValue(
104+
true,
105+
);
106+
107+
const cacheConfig: Cache = { id: 'new-cache-id', strategy: 'read-write' };
108+
const result = processCacheConfig(
109+
cacheConfig,
110+
'fallback-id',
111+
'legacy-cache-id',
112+
);
113+
114+
expect(globalConfigManager.getEnvConfigInBoolean).not.toHaveBeenCalled();
115+
expect(result).toEqual({
116+
id: 'new-cache-id',
117+
strategy: 'read-write',
118+
});
119+
});
120+
121+
test('should prefer new cache config over legacy cacheId even when env is false', () => {
122+
vi.mocked(globalConfigManager.getEnvConfigInBoolean).mockReturnValue(
123+
false,
124+
);
125+
126+
const cacheConfig: Cache = { id: 'new-cache-id', strategy: 'read-write' };
127+
const result = processCacheConfig(
128+
cacheConfig,
129+
'fallback-id',
130+
'legacy-cache-id',
131+
);
132+
133+
expect(globalConfigManager.getEnvConfigInBoolean).not.toHaveBeenCalled();
134+
expect(result).toEqual({
135+
id: 'new-cache-id',
136+
strategy: 'read-write',
137+
});
138+
});
139+
});
140+
141+
describe('Strategy handling', () => {
142+
test('should preserve strategy in cache config', () => {
143+
const strategies = ['read-only', 'read-write', 'write-only'] as const;
144+
145+
strategies.forEach((strategy) => {
146+
const cacheConfig = { id: 'test-cache', strategy };
147+
const result = processCacheConfig(cacheConfig, 'fallback-id');
148+
149+
expect(result).toEqual({
150+
id: 'test-cache',
151+
strategy,
152+
});
153+
});
154+
});
155+
156+
test('should add default strategy when not provided', () => {
157+
const cacheConfig = { id: 'test-cache' };
158+
const result = processCacheConfig(cacheConfig, 'fallback-id');
159+
160+
expect(result).toEqual({
161+
id: 'test-cache',
162+
});
163+
});
164+
});
165+
166+
describe('Fallback ID generation', () => {
167+
test('should use provided fallback ID', () => {
168+
const result = processCacheConfig(true, 'my-custom-fallback-id');
169+
170+
expect(result).toEqual({
171+
id: 'my-custom-fallback-id',
172+
});
173+
});
174+
175+
test('should use fallback ID when cache object missing ID', () => {
176+
const cacheConfig: Cache = { strategy: 'read-only' };
177+
const result = processCacheConfig(cacheConfig, 'my-custom-fallback-id');
178+
179+
expect(result).toEqual({
180+
id: 'my-custom-fallback-id',
181+
strategy: 'read-only',
182+
});
183+
});
184+
});
185+
186+
describe('CLI-specific scenarios', () => {
187+
test('should handle YAML script cache configuration from file name', () => {
188+
// Simulate a scenario where cache config comes from YAML script
189+
const fileName = 'test-script';
190+
const yamlCacheConfig = { id: 'yaml-defined-cache' };
191+
192+
const result = processCacheConfig(yamlCacheConfig, fileName);
193+
194+
expect(result).toEqual({
195+
id: 'yaml-defined-cache',
196+
});
197+
});
198+
199+
test('should generate cache ID from file name when YAML cache is true', () => {
200+
const fileName = 'test-script';
201+
const yamlCacheConfig = true;
202+
203+
const result = processCacheConfig(yamlCacheConfig, fileName);
204+
205+
expect(result).toEqual({
206+
id: 'test-script',
207+
});
208+
});
209+
210+
test('should handle complex YAML cache configuration', () => {
211+
const fileName = 'complex-test';
212+
const yamlCacheConfig: Cache = {
213+
id: 'complex-cache',
214+
strategy: 'read-only',
215+
// Additional properties that might be in YAML
216+
customProp: 'should-be-preserved',
217+
} as Cache;
218+
219+
const result = processCacheConfig(yamlCacheConfig, fileName);
220+
221+
expect(result).toEqual({
222+
id: 'complex-cache',
223+
strategy: 'read-only',
224+
customProp: 'should-be-preserved',
225+
});
226+
});
227+
});
228+
229+
describe('Backward compatibility', () => {
230+
test('should handle legacy cacheId with environment variable correctly', () => {
231+
vi.mocked(globalConfigManager.getEnvConfigInBoolean).mockReturnValue(
232+
true,
233+
);
234+
235+
// Simulate old-style cache configuration
236+
const result = processCacheConfig(
237+
undefined,
238+
'fallback-id',
239+
'my-legacy-cache',
240+
);
241+
242+
expect(result).toEqual({
243+
id: 'my-legacy-cache',
244+
});
245+
});
246+
247+
test('should ignore legacy cacheId when environment variable is not set', () => {
248+
vi.mocked(globalConfigManager.getEnvConfigInBoolean).mockReturnValue(
249+
false,
250+
);
251+
252+
const result = processCacheConfig(
253+
undefined,
254+
'fallback-id',
255+
'my-legacy-cache',
256+
);
257+
258+
expect(result).toBeUndefined();
259+
});
260+
});
261+
});

0 commit comments

Comments
 (0)