mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 03:35:09 +02:00
chore: merge origin/main, resolve CHANGELOG conflict, regen SKILL.md
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -5,11 +5,21 @@
|
||||
- **Review findings now actually get fixed, not just listed.** `/review` and `/ship` used to print informational findings (dead code, test gaps, N+1 queries) and then ignore them. Now every finding gets action: obvious mechanical fixes are applied automatically, and genuinely ambiguous issues are batched into a single question instead of 8 separate prompts. You see `[AUTO-FIXED] file:line Problem → what was done` for each auto-fix.
|
||||
- **You control the line between "just fix it" and "ask me first."** Dead code, stale comments, N+1 queries get auto-fixed. Security issues, race conditions, design decisions get surfaced for your call. The classification lives in one place (`review/checklist.md`) so both `/review` and `/ship` stay in sync.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **`$B js "const x = await fetch(...); return x.status"` now works.** The `js` command used to wrap everything as an expression — so `const`, semicolons, and multi-line code all broke. It now detects statements and uses a block wrapper, just like `eval` already did.
|
||||
- **Clicking a dropdown option no longer hangs forever.** If an agent sees `@e3 [option] "Admin"` in a snapshot and runs `click @e3`, gstack now auto-selects that option instead of hanging on an impossible Playwright click. The right thing just happens.
|
||||
- **When click is the wrong tool, gstack tells you.** Clicking an `<option>` via CSS selector used to time out with a cryptic Playwright error. Now you get: `"Use 'browse select' instead of 'click' for dropdown options."`
|
||||
|
||||
### For contributors
|
||||
|
||||
- Gate Classification → Severity Classification rename (severity determines presentation order, not whether you see a prompt).
|
||||
- Fix-First Heuristic section added to `review/checklist.md` — the canonical AUTO-FIX vs ASK classification.
|
||||
- New validation test: `Fix-First Heuristic exists in checklist and is referenced by review + ship`.
|
||||
- Extracted `needsBlockWrapper()` and `wrapForEvaluate()` helpers in `read-commands.ts` — shared by both `js` and `eval` commands (DRY).
|
||||
- Added `getRefRole()` to `BrowserManager` — exposes ARIA role for ref selectors without changing `resolveRef` return type.
|
||||
- Click handler auto-routes `[role=option]` refs to `selectOption()` via parent `<select>`, with DOM `tagName` check to avoid blocking custom listbox components.
|
||||
- 6 new tests: multi-line js, semicolons, statement keywords, simple expressions, option auto-routing, CSS option error guidance.
|
||||
|
||||
## 0.4.4 — 2026-03-16
|
||||
|
||||
|
||||
@@ -208,6 +208,15 @@ export class BrowserManager {
|
||||
return { selector };
|
||||
}
|
||||
|
||||
/** Get the ARIA role for a ref selector, or null for CSS selectors / unknown refs. */
|
||||
getRefRole(selector: string): string | null {
|
||||
if (selector.startsWith('@e') || selector.startsWith('@c')) {
|
||||
const entry = this.refMap.get(selector.slice(1));
|
||||
return entry?.role ?? null;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
getRefCount(): number {
|
||||
return this.refMap.size;
|
||||
}
|
||||
|
||||
@@ -17,6 +17,24 @@ function hasAwait(code: string): boolean {
|
||||
return /\bawait\b/.test(stripped);
|
||||
}
|
||||
|
||||
/** Detect whether code needs a block wrapper {…} vs expression wrapper (…) inside an async IIFE. */
|
||||
function needsBlockWrapper(code: string): boolean {
|
||||
const trimmed = code.trim();
|
||||
if (trimmed.split('\n').length > 1) return true;
|
||||
if (/\b(const|let|var|function|class|return|throw|if|for|while|switch|try)\b/.test(trimmed)) return true;
|
||||
if (trimmed.includes(';')) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
/** Wrap code for page.evaluate(), using async IIFE with block or expression body as needed. */
|
||||
function wrapForEvaluate(code: string): string {
|
||||
if (!hasAwait(code)) return code;
|
||||
const trimmed = code.trim();
|
||||
return needsBlockWrapper(trimmed)
|
||||
? `(async()=>{\n${code}\n})()`
|
||||
: `(async()=>(${trimmed}))()`;
|
||||
}
|
||||
|
||||
// Security: Path validation to prevent path traversal attacks
|
||||
const SAFE_DIRECTORIES = ['/tmp', process.cwd()];
|
||||
|
||||
@@ -124,7 +142,7 @@ export async function handleReadCommand(
|
||||
case 'js': {
|
||||
const expr = args[0];
|
||||
if (!expr) throw new Error('Usage: browse js <expression>');
|
||||
const wrapped = hasAwait(expr) ? `(async()=>(${expr}))()` : expr;
|
||||
const wrapped = wrapForEvaluate(expr);
|
||||
const result = await page.evaluate(wrapped);
|
||||
return typeof result === 'object' ? JSON.stringify(result, null, 2) : String(result ?? '');
|
||||
}
|
||||
@@ -135,14 +153,8 @@ export async function handleReadCommand(
|
||||
validateReadPath(filePath);
|
||||
if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`);
|
||||
const code = fs.readFileSync(filePath, 'utf-8');
|
||||
if (hasAwait(code)) {
|
||||
const trimmed = code.trim();
|
||||
const isSingleExpr = trimmed.split('\n').length === 1;
|
||||
const wrapped = isSingleExpr ? `(async()=>(${trimmed}))()` : `(async()=>{\n${code}\n})()`;
|
||||
const result = await page.evaluate(wrapped);
|
||||
return typeof result === 'object' ? JSON.stringify(result, null, 2) : String(result ?? '');
|
||||
}
|
||||
const result = await page.evaluate(code);
|
||||
const wrapped = wrapForEvaluate(code);
|
||||
const result = await page.evaluate(wrapped);
|
||||
return typeof result === 'object' ? JSON.stringify(result, null, 2) : String(result ?? '');
|
||||
}
|
||||
|
||||
|
||||
@@ -44,11 +44,48 @@ export async function handleWriteCommand(
|
||||
case 'click': {
|
||||
const selector = args[0];
|
||||
if (!selector) throw new Error('Usage: browse click <selector>');
|
||||
|
||||
// Auto-route: if ref points to a real <option> inside a <select>, use selectOption
|
||||
const role = bm.getRefRole(selector);
|
||||
if (role === 'option') {
|
||||
const resolved = await bm.resolveRef(selector);
|
||||
if ('locator' in resolved) {
|
||||
const optionInfo = await resolved.locator.evaluate(el => {
|
||||
if (el.tagName !== 'OPTION') return null; // custom [role=option], not real <option>
|
||||
const option = el as HTMLOptionElement;
|
||||
const select = option.closest('select');
|
||||
if (!select) return null;
|
||||
return { value: option.value, text: option.text };
|
||||
});
|
||||
if (optionInfo) {
|
||||
await resolved.locator.locator('xpath=ancestor::select').selectOption(optionInfo.value, { timeout: 5000 });
|
||||
return `Selected "${optionInfo.text}" (auto-routed from click on <option>) → now at ${page.url()}`;
|
||||
}
|
||||
// Real <option> with no parent <select> or custom [role=option] — fall through to normal click
|
||||
}
|
||||
}
|
||||
|
||||
const resolved = await bm.resolveRef(selector);
|
||||
if ('locator' in resolved) {
|
||||
await resolved.locator.click({ timeout: 5000 });
|
||||
} else {
|
||||
await page.click(resolved.selector, { timeout: 5000 });
|
||||
try {
|
||||
if ('locator' in resolved) {
|
||||
await resolved.locator.click({ timeout: 5000 });
|
||||
} else {
|
||||
await page.click(resolved.selector, { timeout: 5000 });
|
||||
}
|
||||
} catch (err: any) {
|
||||
// Enhanced error guidance: clicking <option> elements always fails (not visible / timeout)
|
||||
const isOption = 'locator' in resolved
|
||||
? await resolved.locator.evaluate(el => el.tagName === 'OPTION').catch(() => false)
|
||||
: await page.evaluate(
|
||||
(sel: string) => document.querySelector(sel)?.tagName === 'OPTION',
|
||||
(resolved as { selector: string }).selector
|
||||
).catch(() => false);
|
||||
if (isOption) {
|
||||
throw new Error(
|
||||
`Cannot click <option> elements. Use 'browse select <parent-select> <value>' instead of 'click' for dropdown options.`
|
||||
);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
// Wait briefly for any navigation/DOM update
|
||||
await page.waitForLoadState('domcontentloaded').catch(() => {});
|
||||
|
||||
@@ -198,6 +198,27 @@ describe('Inspection', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('js handles multi-line with await', async () => {
|
||||
const code = 'const x = await Promise.resolve(42);\nreturn x;';
|
||||
const result = await handleReadCommand('js', [code], bm);
|
||||
expect(result).toBe('42');
|
||||
});
|
||||
|
||||
test('js handles await with semicolons', async () => {
|
||||
const result = await handleReadCommand('js', ['const x = await Promise.resolve(5); return x + 1;'], bm);
|
||||
expect(result).toBe('6');
|
||||
});
|
||||
|
||||
test('js handles await with statement keywords', async () => {
|
||||
const result = await handleReadCommand('js', ['const res = await Promise.resolve("ok"); return res;'], bm);
|
||||
expect(result).toBe('ok');
|
||||
});
|
||||
|
||||
test('js still works for simple expressions', async () => {
|
||||
const result = await handleReadCommand('js', ['1 + 2'], bm);
|
||||
expect(result).toBe('3');
|
||||
});
|
||||
|
||||
test('css returns computed property', async () => {
|
||||
const result = await handleReadCommand('css', ['h1', 'color'], bm);
|
||||
// Navy color
|
||||
@@ -247,6 +268,36 @@ describe('Interaction', () => {
|
||||
expect(val).toBe('admin');
|
||||
});
|
||||
|
||||
test('click on option ref auto-routes to selectOption', async () => {
|
||||
await handleWriteCommand('goto', [baseUrl + '/forms.html'], bm);
|
||||
// Reset select to default
|
||||
await handleReadCommand('js', ['document.querySelector("#role").value = ""'], bm);
|
||||
const snap = await handleMetaCommand('snapshot', [], bm, async () => {});
|
||||
// Find an option ref (e.g., "Admin" option)
|
||||
const optionLine = snap.split('\n').find((l: string) => l.includes('[option]') && l.includes('"Admin"'));
|
||||
expect(optionLine).toBeDefined();
|
||||
const refMatch = optionLine!.match(/@(e\d+)/);
|
||||
expect(refMatch).toBeDefined();
|
||||
const ref = `@${refMatch![1]}`;
|
||||
const result = await handleWriteCommand('click', [ref], bm);
|
||||
expect(result).toContain('auto-routed');
|
||||
expect(result).toContain('Selected');
|
||||
// Verify the select value actually changed
|
||||
const val = await handleReadCommand('js', ['document.querySelector("#role").value'], bm);
|
||||
expect(val).toBe('admin');
|
||||
});
|
||||
|
||||
test('click CSS selector on option gives helpful error', async () => {
|
||||
await handleWriteCommand('goto', [baseUrl + '/forms.html'], bm);
|
||||
try {
|
||||
await handleWriteCommand('click', ['option[value="admin"]'], bm);
|
||||
expect(true).toBe(false); // Should not reach here
|
||||
} catch (err: any) {
|
||||
expect(err.message).toContain('select');
|
||||
expect(err.message).toContain('option');
|
||||
}
|
||||
}, 15000);
|
||||
|
||||
test('hover works', async () => {
|
||||
const result = await handleWriteCommand('hover', ['h1'], bm);
|
||||
expect(result).toContain('Hovered');
|
||||
|
||||
Reference in New Issue
Block a user