Skip to content

planner task - #518

Open
harishmotamarri wants to merge 2 commits into
agentrhq:mainfrom
harishmotamarri:planner
Open

harishmotamarri wants to merge 2 commits into
agentrhq:mainfrom
harishmotamarri:planner

Conversation

@harishmotamarri

Copy link
Copy Markdown

Description

Related issue:

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 🌐 New site adapter
  • 📝 Documentation
  • ♻️ Refactor
  • 🔧 CI / build / tooling

Checklist

  • I ran the checks relevant to this PR
  • I updated tests or docs if needed
  • I included output or screenshots when useful
  • If I edited skill-src/, I ran make build and committed skills/

Adapter Notes

  • Updated generated or lean docs when command discoverability changed
  • Used positional args for the command's primary subject unless a named flag is clearly better
  • Normalized expected adapter failures to CliError subclasses instead of raw Error

Screenshots / Output

Copilot AI lite review requested due to automatic review settings September 12, 2026 09:19
@github-actions

Copy link
Copy Markdown
Contributor

🟠 Maintainer review suggested — low confidence

The automated review could not reach a fully supported conclusion.

This review is advisory and does not block merging.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical and moderate issues remain in session handling, context safety, workflow execution, and dependency locking.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds Gemini-backed natural-language browser workflows with validation, context passing, CLI execution, and documentation.

Changes:

  • Adds workflow planning, validation, execution, and browser-script support.
  • Adds Gemini integration and a workflow run CLI command.
  • Updates dependencies, tests, exports, and quickstart documentation.
File summaries
File Reviewed changes
src/planner/index.ts Workflow planning, validation, execution, and browser helpers
src/planner/index.test.ts Planner and workflow execution tests
src/planner/gemini.ts Gemini workflow generation
src/cli.ts workflow run command and session lifecycle
package.json Planner exports and runtime dependency
package-lock.json Dependency lockfile updates
docs/quickstart.mdx Workflow usage documentation
Review details

Suppressed comments (5)

package.json:101

  • @google/genai is now a production dependency in package.json, but the tracked bun.lock still lists it under the root devDependencies. Bun frozen or production installs can therefore remain inconsistent or omit the SDK used by the runtime import; regenerate and commit the Bun lockfile with this dependency move.
    "@google/genai": "^2.10.0",

src/cli.ts:667

  • The new top-level workflow command is absent from the maintained Top-Level Commands table in docs/cli-reference.mdx (lines 152-166), even though this change adds a user-facing command and the reference documents every other top-level command. Add the workflow entry so CLI discoverability matches the implementation.
    .command('workflow')

src/planner/index.ts:247

  • When args.text is supplied, this passes visible text such as "Save" to page.locator, which treats it as a CSS selector rather than a text query and normally finds no element. Keep selector/ref handling separate and use a text locator such as getByText for the text form.
  if (step.command === 'click') {
    const target = args.selector ?? args.ref ?? args.text;
    return typeof target === 'string'
      ? `await page.locator(${JSON.stringify(target)}).click(); return { clicked: true };`
      : undefined;

src/planner/index.ts:79

  • Only exact duplicate output keys are rejected here. Overlapping paths such as event and event.date pass validation, but writePath replaces the existing scalar/object while applying the second mapping, silently discarding context data. Reject ancestor/descendant output paths before execution or define explicit merge semantics.
    for (const key of Object.keys(step.outputs ?? {})) {
      if (!key || produced.has(key)) throw invalidWorkflow(`Workflow output "${key}" is produced more than once.`);
      produced.add(key);
    }

src/planner/index.ts:78

  • Only output keys are checked here, but readPath below assumes every mapping value is a string and calls .replace on it. The Gemini schema also permits arbitrary values in outputs, so a response such as outputs: { value: 1 } passes validation and later throws a raw TypeError instead of a workflow ConfigError. Validate the mapping values as strings before adding their keys.
    for (const key of Object.keys(step.outputs ?? {})) {
      if (!key || produced.has(key)) throw invalidWorkflow(`Workflow output "${key}" is produced more than once.`);
      produced.add(key);
  • Files reviewed: 6/7 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/cli.ts
created = true;
}
try {
const result = await runBrowserWorkflow(workflow, { session, profile: opts.profile, context: {} });
Comment thread src/planner/index.ts
Comment on lines +212 to +213
const parts = path.split('.').filter(Boolean);
let current: Record<string, unknown> = target;
Comment thread src/cli.ts
.description('Ask Gemini to plan a workflow and run it in a browser Session')
.requiredOption('--goal <text>', 'Natural-language task to perform')
.option('--session <id>', 'Existing browser Session ID; created automatically when omitted')
.option('--profile <name>', 'Browser profile to use', 'default')
Comment thread src/cli.ts
}
const profile = resolveProfileSelection(opts.profile);
const profileId = profile?.contextId ?? 'default';
let session = opts.session?.trim();
Comment thread src/planner/index.ts
Comment on lines +85 to +86
validateWorkflow(workflow);
const context: WorkflowContext = options.context ?? {};
Comment thread src/planner/index.ts
Comment on lines +155 to +163
executeScript: options.executeScript ?? ((script) => sendCommand('run', {
session: options.session,
surface: 'browser',
...profileRouteParams(resolveProfileSelection(options.profile)),
source: script,
timeoutMs: 120_000,
timeout: 125,
snapshotMode: 'act',
})),
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants