ADE CLI / PR workflow fixes and integration lane ownership

[arul28]

ADE CLI RPC/bootstrap improvements, shared adeCliGuidance for agents, orchestrator/chat/CTO wiring, and safer integration lane ownership for cleanup (scratch lanes vs merge-into adoption).

Made with Cursor

Summary by CodeRabbit

Release Notes

• New Features

  • Expanded ADE actions with new domains and enhanced method allowlists for automation workflows.
  • Automated PR description drafting when title/body are omitted.
  • Role-based access controls for restricted actions.

• Improvements

  • Enhanced help system with detailed command and argument documentation.
  • Improved project and workspace root discovery with environment overrides.
  • Better PR detection and duplicate handling.
  • Refined agent guidance with inline CLI documentation.

Greptile Summary

This PR consolidates ADE CLI guidance into a single shared constant (ADE_CLI_AGENT_GUIDANCE), fixes several PR duplicate-recovery bugs (cascade-delete scoping, findExistingPrForBranch, #123 locator parsing), expands the action allowlist with ~15 new domains and role-based access enforcement, and improves worktree-aware project root discovery in the CLI.

All previously flagged issues appear resolved: startupCommandPreview now logs the augmented prompt, duplicate guidance in promptInspector.ts is removed, the || Boolean(candidate?.draft) closed-draft matching is gone, --text is absent from VALUE_CARRIER_FLAGS, and graph_state is correctly scoped to runtime.projectId.

Confidence Score: 5/5

Safe to merge; all P0/P1 concerns from prior reviews are addressed and the two new findings are minor style inconsistencies.

Every previously flagged P0/P1 issue has a clear fix in this diff. The two new findings are P2: a null-vs-empty-object inconsistency in layout.set and a hardcoded provider string gate. Neither causes data loss or incorrect behavior in the current provider set.

apps/desktop/src/main/services/adeActions/registry.ts — layout.set(null) inconsistency; apps/desktop/src/main/services/chat/agentChatService.ts — hardcoded "claude" provider check.

Important Files Changed

Filename	Overview
apps/ade-cli/src/cli.ts	Major expansion of help text, hasHelpFlag/VALUE_CARRIER_FLAGS routing, worktree-aware root discovery, and command aliases. --text no longer in VALUE_CARRIER_FLAGS, fixing the prior help-swallowing bug.
apps/desktop/src/main/services/adeActions/registry.ts	New domains (graph_state, layout, tiling_tree, keybindings, github, budget, etc.), role-based access control helpers (isCtoOnlyAdeAction/callerHasRoleAtLeast), and inline domain-service builders. layout.set(null) stores {} instead of clearing, inconsistent with tiling_tree.set and graph_state.set.
apps/desktop/src/main/services/prs/prService.ts	Adds getRowByLocator, repoPrKey, findExistingPrForBranch for safer duplicate-PR recovery; upsertRow returns the upserted ID; cascade deletes gated behind allowRepoPrAdoption; #123 PR locator parsing; getIntegrationLaneOrigin null-safe guard; listAll now accepts laneId filter.
apps/desktop/src/main/services/orchestrator/promptInspector.ts	Replaces hardcoded guidance strings with ADE_CLI_AGENT_GUIDANCE constant; removes the duplicate "ADE TOOLING" push that previously resulted in two copies of the guidance in every worker prompt.
apps/desktop/src/main/services/orchestrator/baseOrchestratorAdapter.ts	Injects ADE_CLI_AGENT_GUIDANCE into startup-command override prompt; startupCommandPreview now logs the augmented prompt (not the bare override), fixing prior debug-visibility bug.
apps/desktop/src/main/services/orchestrator/orchestratorService.ts	Adds comment clarifying that ADE_CLI_AGENT_GUIDANCE must NOT be prepended to the worker prompt file because buildFullPrompt already injects it via the system prompt for Claude workers.
apps/desktop/src/shared/adeCliGuidance.ts	New shared constant file exporting ADE_CLI_AGENT_GUIDANCE (multi-line) and ADE_CLI_INLINE_GUIDANCE (single-line) to consolidate ADE CLI guidance strings across agents, inspector, and chat service.
apps/desktop/src/main/services/chat/agentChatService.ts	Switches ADE tooling guidance to the shared constant; adds capability-based guidance injection guard (providerHasPersistentGuidance) using a hardcoded "claude" provider name comparison that may miss future providers.
apps/desktop/src/main/main.ts	Moves autoUpdateService creation to before initContextForProjectRoot so all RPC runtimes capture a live reference; exposes many additional services on rpcRuntime to match the expanded ADE_ACTION_ALLOWLIST.
apps/ade-cli/src/adeRpcServer.ts	Uses ADE_ACTION_DOMAIN_NAMES const instead of hardcoded enum arrays; wires isCtoOnlyAdeAction into both list_ade_actions (filter) and run_ade_action (enforcement) handlers; create_pr_from_lane title no longer required.
apps/desktop/src/shared/types/prs.ts	Minor type addition to the PR types shared module.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ade CLI invocation] --> B{hasHelpFlag?}
    B -- yes --> C[Return HELP_BY_COMMAND text]
    B -- no --> D{Primary command}
    D --> E[prs create / link]
    D --> F[actions run]
    D --> G[git / lanes / files / ...]

    E --> H[prService.createFromLane]
    H --> I{GitHub API create PR}
    I -- success --> J[upsertRow - returns prId]
    I -- duplicate error --> K[findExistingPrForBranch]
    K --> L[Adopt existing PR row
allowRepoPrAdoption:true]
    L --> J
    J --> M[markHotRefresh + refreshOne]

    F --> N{isCtoOnlyAdeAction?}
    N -- yes, caller not CTO --> O[Throw permission error]
    N -- no / caller is CTO --> P[Invoke domain service method]

    subgraph ADE_CLI_GUIDANCE [Shared guidance injection]
        Q[ADE_CLI_AGENT_GUIDANCE constant]
        Q --> R[baseOrchestratorAdapter - startup override prompt]
        Q --> S[promptInspector - worker base guidance]
        Q --> T[agentChatService - non-Claude providers only]
        Q --> U[ctoStateService - CTO doctrine]
    end

Loading

Comments Outside Diff (5)

1. apps/desktop/src/main/services/orchestrator/promptInspector.ts, line 178-225 (link)

   Duplicate ADE_CLI_AGENT_GUIDANCE block in every worker prompt

   buildWorkerBaseGuidance unconditionally calls sections.push(...) with ADE_CLI_AGENT_GUIDANCE at two separate points (lines 178 and 213). Both execute for every worker, so the resulting prompt contains the ## ADE CLI heading and its two-line body verbatim twice.

   Before this PR the two slots held distinct strings:

   • slot 1 → "ADE CLI: In terminal-capable sessions, use the bundled \ade` command…"`
   • slot 2 → "ADE TOOLING: Use ADE's action surface or the \ade` CLI for team collaboration commands when available."`

   After the consolidation both slots emit the same constant. The worker identity / key-actions block in the second sections.push is still unique and correct, but it is now prefixed with a redundant ## ADE CLI section that will appear a second time in the prompt.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/desktop/src/main/services/orchestrator/promptInspector.ts
   Line: 178-225
   
   Comment:
   **Duplicate `ADE_CLI_AGENT_GUIDANCE` block in every worker prompt**
   
   `buildWorkerBaseGuidance` unconditionally calls `sections.push(...)` with `ADE_CLI_AGENT_GUIDANCE` at two separate points (lines 178 and 213). Both execute for every worker, so the resulting prompt contains the `## ADE CLI` heading and its two-line body verbatim twice.
   
   Before this PR the two slots held *distinct* strings:
   - slot 1 → `"ADE CLI: In terminal-capable sessions, use the bundled \`ade\` command…"`
   - slot 2 → `"ADE TOOLING: Use ADE's action surface or the \`ade\` CLI for team collaboration commands when available."`
   
   After the consolidation both slots emit the same constant. The worker identity / key-actions block in the second `sections.push` is still unique and correct, but it is now prefixed with a redundant `## ADE CLI` section that will appear a second time in the prompt.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

2. apps/desktop/src/main/services/prs/prService.ts, line 2541-2559 (link)

   Regex heuristic for duplicate-PR detection may over-match

   The condition used to trigger findExistingPrForBranch is:

   /pull request already exists|already exists|validation failed/i.test(msg)

   "validation failed" is a broad phrase that GitHub also returns for unrelated schema violations (e.g. invalid base branch, missing required field). If such an error is misidentified as a duplicate-PR error, the code will silently look up and adopt an existing PR for the branch — potentially mapping the wrong PR to the lane. A narrower pattern such as /pull request already exists/i (or checking the GitHub 422 status code) would reduce the false-positive surface.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/desktop/src/main/services/prs/prService.ts
   Line: 2541-2559
   
   Comment:
   **Regex heuristic for duplicate-PR detection may over-match**
   
   The condition used to trigger `findExistingPrForBranch` is:
   
   ```typescript
   /pull request already exists|already exists|validation failed/i.test(msg)
   ```
   
   `"validation failed"` is a broad phrase that GitHub also returns for unrelated schema violations (e.g. invalid base branch, missing required field). If such an error is misidentified as a duplicate-PR error, the code will silently look up and adopt an existing PR for the branch — potentially mapping the wrong PR to the lane. A narrower pattern such as `/pull request already exists/i` (or checking the GitHub `422` status code) would reduce the false-positive surface.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

3. apps/desktop/src/main/services/prs/prService.ts, line 1308-1330 (link)

   findExistingPrForBranch typed as Promise<any | null>

   The function and its fetchAllPages<any> call use any, which drops type safety on the returned GitHub PR object. The rest of prService.ts accesses .state, .draft, .number, etc. from the result with runtime asString/Number guards — that pattern works — but the signature makes it impossible for callers to know what shape they're consuming without reading the implementation.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/desktop/src/main/services/prs/prService.ts
   Line: 1308-1330
   
   Comment:
   **`findExistingPrForBranch` typed as `Promise<any | null>`**
   
   The function and its `fetchAllPages<any>` call use `any`, which drops type safety on the returned GitHub PR object. The rest of `prService.ts` accesses `.state`, `.draft`, `.number`, etc. from the result with runtime `asString`/`Number` guards — that pattern works — but the signature makes it impossible for callers to know what shape they're consuming without reading the implementation.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

4. apps/desktop/src/main/services/orchestrator/promptInspector.ts, line 178-225 (link)

   Duplicate ADE_CLI_AGENT_GUIDANCE in buildWorkerBaseGuidance

   ADE_CLI_AGENT_GUIDANCE is pushed into sections at two separate points in this function (lines 178 and 213). Both calls execute unconditionally for every worker, so the inspector output contains the ## ADE CLI heading and body verbatim twice. The inspector therefore shows a longer prompt than what buildFullPrompt in baseOrchestratorAdapter.ts actually sends to workers (where the guidance only appears once), making the inspector inaccurate for debugging.

   The second push (lines 213–225) is what contains the unique "worker identity" / key-actions text; ADE_CLI_AGENT_GUIDANCE should not prefix that block as well.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/desktop/src/main/services/orchestrator/promptInspector.ts
   Line: 178-225
   
   Comment:
   **Duplicate `ADE_CLI_AGENT_GUIDANCE` in `buildWorkerBaseGuidance`**
   
   `ADE_CLI_AGENT_GUIDANCE` is pushed into `sections` at two separate points in this function (lines 178 and 213). Both calls execute unconditionally for every worker, so the inspector output contains the `## ADE CLI` heading and body verbatim twice. The inspector therefore shows a longer prompt than what `buildFullPrompt` in `baseOrchestratorAdapter.ts` actually sends to workers (where the guidance only appears once), making the inspector inaccurate for debugging.
   
   The second push (lines 213–225) is what contains the unique "worker identity" / key-actions text; `ADE_CLI_AGENT_GUIDANCE` should not prefix that block as well.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

5. apps/desktop/src/main/services/prs/prService.ts, line 1154-1215 (link)

   Cascade deletes silently strip group memberships on cross-lane reassignment

   upsertRow now falls back to getRowForRepoPr when getRowForLane finds nothing, and when it does match a PR that belongs to a different lane it immediately deletes that PR's pr_group_members rows and nullifies integration_proposals references. This path fires from both createFromLane (duplicate-recovery) and linkToLane.

   The linkToLane case is the risky one: if PR #X was already in a merge queue under lane A and a caller links it to lane B, the queue membership is silently destroyed with no notification or rollback. The caller has no signal that anything was cleared.

   Consider guarding the cascade deletes with a check that verifies neither pr_group_members nor active integration_proposals exist before proceeding, or at minimum logging a warning when rows are removed.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/desktop/src/main/services/prs/prService.ts
   Line: 1154-1215
   
   Comment:
   **Cascade deletes silently strip group memberships on cross-lane reassignment**
   
   `upsertRow` now falls back to `getRowForRepoPr` when `getRowForLane` finds nothing, and when it does match a PR that belongs to a *different* lane it immediately deletes that PR's `pr_group_members` rows and nullifies `integration_proposals` references. This path fires from both `createFromLane` (duplicate-recovery) and `linkToLane`.
   
   The `linkToLane` case is the risky one: if PR #X was already in a merge queue under lane A and a caller links it to lane B, the queue membership is silently destroyed with no notification or rollback. The caller has no signal that anything was cleared.
   
   Consider guarding the cascade deletes with a check that verifies neither `pr_group_members` nor active `integration_proposals` exist before proceeding, or at minimum logging a warning when rows are removed.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

This is a comment left during a code review.
Path: apps/desktop/src/main/services/adeActions/registry.ts
Line: 473-482

Comment:
**`layout.set(null)` stores `{}` instead of clearing — inconsistent with sibling services**

The error message on line 466 says "Pass an explicit null to clear", but when `rawLayout === null`, the code stores `{}` (empty object) rather than `null`. The two sibling services (`tiling_tree.set` and `graph_state.set`) both store `null` directly via `runtime.db.setJson(key, null)`, making the behavior of "clearing" consistent. For `layout`, calling `.set({ layoutId, layout: null })` leaves a `{}` record behind rather than clearing the key, which contradicts the documented intent.

```typescript
// Suggested fix: pass null through to the db directly (like tiling_tree/graph_state do)
if (rawLayout === null) {
  runtime.db.setJson(`dock_layout:${layoutId}`, null);
  return { layoutId, layout: {} };
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/desktop/src/main/services/chat/agentChatService.ts
Line: 10912-10915

Comment:
**Hardcoded provider string for guidance-injection gate**

`providerHasPersistentGuidance` is determined by `managed.session.provider === "claude"`. If a new provider with a persistent system prompt is added (e.g., Gemini with a native system-prompt slot), this gate will silently inject `ADE_CLI_AGENT_GUIDANCE` into its first user turn as well, creating the same redundancy that this code was written to prevent. Consider abstracting this into a provider-capability flag rather than a string comparison.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (9): Last reviewed commit: "ship: iteration 4 (post-rebate) — post-m..." | Re-trigger Greptile}