Commit Graph

231 Commits

Author SHA1 Message Date
Priyanshu Choudhary 117b10aae9 fix(windows): QA fixes — dashboard spawn, shellEscape, tmux hint, PID fallback
T04: resolveNextBin() skips POSIX .bin/next shim on Windows; invokes
next/dist/bin/next via process.execPath instead (ENOENT fix).

T18: shellEscape() branches on isWindows() — PowerShell uses '' doubling,
Unix uses POSIX '\'' escaping. All 4 agent plugins benefit automatically.

T06 secondary: tmux attach hint in `ao spawn` output gated behind
runtimeName === "tmux" (process runtime has no tmux session).

T06/T11: runtime-process destroy() and isAlive() fall back to
handle.data.pid when the in-memory processes Map is empty — fixes
cross-process CLI calls (ao session ls / ao session kill).
2026-04-10 01:09:30 +05:30
Priyanshu Choudhary 3fe711b76f fix(core): use F_OK and skip bare extension in Windows gh/git wrappers
On Windows, fs.constants.X_OK is equivalent to F_OK (execute bit does
not exist), so any existing file passes the check. The empty extension
was also tried first, meaning a bare file named "gh" would be selected
over gh.exe. Switch to F_OK and only check .exe/.cmd extensions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:04:13 +05:30
Priyanshu Choudhary 5ce89025af fix(core): always force-kill on Windows; fix stale .cjs JSDoc
killProcessTree: drop the SIGTERM-without-/F branch on Windows.
taskkill without /F sends WM_CLOSE which is unreliable for headless
Node.js console processes — they may silently survive, leaving orphaned
processes. Always pass /F so termination is guaranteed. Callers that
do SIGTERM→wait→SIGKILL escalation are unaffected: SIGKILL simply
finds the process already dead.

agent-workspace-hooks: correct JSDoc that still said <name>.js after
the extension was changed to .cjs (forced CJS mode).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:04:11 +05:30
Priyanshu Choudhary 7150661729 fix: resolve two open bugbot issues on windows-platform-adapter PR
- fix(platform): always use /bin/sh on Unix instead of \$SHELL so
  postCreate commands and runtime launches work correctly when the
  user's login shell is non-POSIX (fish, nushell, etc.)

- fix(script-runner): detect cmd.exe fallback on Windows and throw a
  clear, actionable error pointing to AO_BASH_PATH rather than passing
  the PowerShell-specific -File flag to cmd.exe (which produces a
  cryptic error and still can't run bash scripts)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:04:08 +05:30
Priyanshu Choudhary eccf19ac4c fix(cli,core): remove unused getEnvDefaults export, add SIGKILL fallback in signal forward handler
- Remove getEnvDefaults from @composio/ao-core public API — no production callers;
  the function remains in platform.ts for future use (B10/B11)
- Add 5 s SIGKILL fallback in dashboard.ts and start.ts forward() handlers so the
  parent process cannot hang indefinitely if the child ignores SIGTERM

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:04:06 +05:30
Priyanshu Choudhary 2c12b1eef6 test(core): add IPv6 netstat test for findPidByPort + mark _resetShellCache as @internal
- Add test case for IPv6 LISTENING entries ([::]:3000) in Windows netstat output
- Add @internal JSDoc to _resetShellCache to clarify it is test-only

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:04:04 +05:30
Priyanshu Choudhary ae1a6e4fc5 fix(core): deduplicate Node.js wrapper updateAoMetadata and add AO_DATA_DIR path validation
- Extract shared NODE_UPDATE_AO_METADATA constant used by both gh/git wrappers
- Add AO_DATA_DIR validation matching bash ao-metadata-helper.sh (must be under ~/.ao/, ~/.agent-orchestrator/, or tmpdir)
- Bump WRAPPER_VERSION to 0.5.0 to force reinstall with new security check
- Update version in agent-codex and core tests to match 0.5.0
- Fix CI test failures from WRAPPER_VERSION bump (0.2.0 → 0.4.0 → 0.5.0)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:04:03 +05:30
Priyanshu Choudhary 37a0d82d92 feat(windows): Node.js metadata wrappers + Claude Code hook + pwsh shell (squash merge PR5)
Squash merges feat/windows-hooks-and-launch (PR #1033) into PR1.

Blockers addressed:
- B16: ~/.ao/bin/gh and git wrappers are now Node.js scripts + .cmd shims on
  Windows (bash on Unix unchanged). WRAPPER_VERSION bumped to 0.3.0 to force
  reinstall.
- B17: Claude Code PostToolUse hook uses JSON.parse/fs built-ins on Windows
  instead of bash+jq+grep+sed. Atomic writes via temp file + renameSync.
  chmod skipped on Windows.
- B18: runtime-process uses getShell().cmd + shellInfo.args() instead of
  shell:true (which resolves to cmd.exe on Windows). getShell() returns
  pwsh > powershell.exe > cmd.exe on Windows, bash on Unix.

Conflict resolution:
- runtime-process spawn: kept PR5's getShell() approach (B18 fix), removed
  shell:true which PR1 had as a placeholder.
- runtime-process destroy: kept PR1's cleaner killProcessTree delegation
  (PR5 had inline platform-branching written before killProcessTree was
  integrated into this worktree).
- Tests: merged PR5's new Windows compatibility tests, adjusted assertions to
  match PR1's killProcessTree(pid, signal) API. Fixed Windows compat tests
  that expected old spawn(launchCommand, opts) signature.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:04:03 +05:30
Priyanshu Choudhary f9b0a53d5e fix(core): guard killProcessTree against pid <= 0
On Unix, -0 === 0 in JS, so killProcessTree(0) would call process.kill(0)
which sends the signal to every process in the calling process's group,
killing AO itself. findPidByPort can return "0" since it passes the
truthiness check and the /^\d+$/ regex. Guard pid <= 0 at the top of
killProcessTree and return early.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:04:02 +05:30
Priyanshu Choudhary 14a844829e fix(core): use lazy factory for Zod runtime default
z.string().default(getDefaultRuntime()) evaluates getDefaultRuntime() once
at module load time, creating hidden coupling between import order and
platform detection. Using a factory function ensures the default is resolved
lazily each time it is needed, making the intent explicit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:04:00 +05:30
Priyanshu Choudhary 2f93f89ccb test(core,cli): add missing coverage for platform.ts and lifecycle-service
- platform.mock.test.ts: cover Windows getEnvDefaults PATH fallback (line 148)
- lifecycle-service.test.ts: cover stopLifecycleWorker stale-PID path,
  normal SIGTERM kill, and SIGKILL escalation after timeout

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:03:55 +05:30
Priyanshu Choudhary 60d81337b8 test(core): update killProcessTree Windows tests for signal-aware taskkill
Split the single taskkill test into two: SIGTERM uses /T /PID (graceful)
and SIGKILL uses /T /F /PID (force), matching the updated implementation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:03:55 +05:30
Priyanshu Choudhary 138f20f503 fix(core): respect signal parameter in killProcessTree on Windows
SIGTERM now uses taskkill /T /PID (WM_CLOSE, graceful) instead of /F,
preserving the SIGTERM→wait→SIGKILL escalation used by lifecycle-service,
start.ts, and runtime-process. SIGKILL keeps /T /F /PID (force).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:03:54 +05:30
Priyanshu Choudhary 7ff6c7af9c fix(core): fix TypeScript errors in platform.mock.test.ts
Return ChildProcess from execFile mock implementations and use "" instead
of undefined for execFileSync mock return value to satisfy strict types.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:03:52 +05:30
Priyanshu Choudhary 54f573e723 test(core): add mocked tests for platform.ts to fix diff coverage
Adds platform.mock.test.ts with 25 tests covering Windows-specific
branches (resolveWindowsShell fallbacks, killProcessTree, findPidByPort)
and Unix error-handling/env-var fallback chains that require mocking
node:child_process. Pushes platform.ts diff coverage from 45.9% to ≥80%.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:03:52 +05:30
Cursor Agent 6b87dceae6 fix(core): tighten Windows PID port matching 2026-04-10 01:03:51 +05:30
Priyanshu Choudhary 60099b0b5d fix(cli): use platform-aware runtime fallback in start command
B01/B02: ensureTmux() is only called when runtime resolves to 'tmux'.
On Windows, runtime defaults to 'process', skipping tmux entirely.
2026-04-10 01:03:51 +05:30
Priyanshu Choudhary b180802c7f feat(core): platform-aware runtime default (tmux on Unix, process on Windows)
B04: Config and docs now reflect platform-specific defaults.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:03:50 +05:30
Priyanshu Choudhary 3b7248da6d feat(core): add cross-platform adapter (platform.ts)
Centralizes all platform-branching logic: shell resolution (pwsh > powershell > cmd),
process tree kill (taskkill on Windows), port-based PID discovery (netstat on Windows),
runtime defaults, and env defaults.

Addresses blockers B05, B06, B07, B08 from Windows compatibility proposal.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-10 01:03:49 +05:30
Gaurav Bhola 0613ac2970 Merge remote-tracking branch 'upstream/main' into feat/single-socket
# Conflicts:
#	packages/cli/__tests__/commands/start.test.ts
#	packages/cli/src/commands/start.ts
#	packages/core/src/__tests__/lifecycle-manager.test.ts
#	packages/core/src/agent-selection.ts
#	packages/core/src/lifecycle-manager.ts
#	packages/core/src/plugin-registry.ts
#	packages/core/src/recovery/validator.ts
#	packages/core/src/session-manager.ts
#	packages/web/src/__tests__/api-routes.test.ts
#	packages/web/src/app/sessions/[id]/page.tsx
#	packages/web/src/components/Dashboard.tsx
#	packages/web/src/components/PullRequestsPage.tsx
#	packages/web/src/components/__tests__/Dashboard.doneBar.test.tsx
#	packages/web/src/hooks/useSessionEvents.ts
2026-04-07 03:37:36 -07:00
Gaurav Bhola b4c108338f fix(core): allow external notifier with manifest name matching built-in
When an external notifier is loaded via path and its manifest.name
collides with a built-in (e.g. 'slack'), prepareConfig previously threw
because isBuiltin was true and the path field was present. Skip the
built-in guard for external loads — the path field in that case is the
loading path, not a stray user config value.
2026-04-06 23:25:42 -07:00
Gaurav Bhola 1531a914c0 fix(core): forward allSessionPrefixes in resolveSessionRole
Prevents cross-project misclassification where a worker session like
'app-orchestrator-1' of prefix 'app-orchestrator' could be matched as
an orchestrator for a shorter prefix 'app'.
2026-04-06 23:11:59 -07:00
Harsh Batheja f65e7d95bf fix(lifecycle): reduce GitHub API rate limiting from batch enrichment bypass (#906)
* fix(lifecycle): reduce GitHub API rate limiting from batch enrichment bypass

Three optimizations to prevent API storms in the lifecycle manager poll cycle:

1. **CRITICAL - maybeDispatchMergeConflicts**: Gate the getMergeability()
   fallback to only run when batch enrichment didn't run at all. Previously
   it called getMergeability() (3 REST calls) whenever hasConflicts was
   undefined, even when the batch had already fetched PR data. Now uses
   cachedData.hasConflicts ?? false when the batch ran.

2. **HIGH - maybeDispatchCIFailureDetails**: Use batch enrichment ciChecks
   when available instead of calling getCIChecks() (separate REST call)
   on every poll. The GraphQL batch query now fetches statusCheckRollup
   contexts (individual check names, statuses, URLs) alongside the rollup
   state. Falls back to getCIChecks() only when batch didn't run.

3. **MEDIUM - maybeDispatchReviewBacklog**: Throttle getPendingComments +
   getAutomatedComments API calls to at most once per 2 minutes per session.
   These were called every 30s even when nothing had changed.

Impact: ~8-10 API calls/PR/poll reduced to ~2-4, enabling 3-4x more
concurrent sessions before hitting GitHub's 5,000/hr REST limit.

Also extends PREnrichmentData with ciChecks?: CICheck[] and adds
parseCheckContexts() helper to graphql-batch.ts for parsing CheckRun
and StatusContext nodes from the GraphQL statusCheckRollup.contexts field.

* fix(scm-github): fall back to getCIChecks() when contexts list is truncated

When a PR has >20 CI checks, contexts(first: 20) silently truncates the
list. Setting ciChecks to undefined when pageInfo.hasNextPage is true
ensures maybeDispatchCIFailureDetails falls back to the getCIChecks()
REST call, which returns all checks without truncation.

Also adds pageInfo { hasNextPage } to the contexts GraphQL query so
truncation can be detected.

* fix(lifecycle): prune lastReviewBacklogCheckAt in pollAll cleanup loop

Add the new throttle map to the existing pruning loop that removes stale
entries for sessions no longer in the session list. Previously the map
was only cleared on terminal status transitions, leaving orphaned entries
for sessions removed externally (killed + cleaned up without transition).

* fix(lifecycle): bypass throttle on review transition; fix StatusContext conclusion

Two fixes for automated review findings:

1. Bypass review backlog throttle when a transition reaction just fired for
   humanReactionKey or automatedReactionKey. The transitionReaction branch
   needs to read the current fingerprint via the API to record
   lastPendingReviewDispatchHash. Without bypassing, the throttle prevents
   this write and the next unthrottled poll sees a stale (empty) hash,
   clears the reaction tracker, and fires a duplicate dispatch.

2. Set conclusion on StatusContext nodes in parseCheckContexts() to match
   the REST getCIChecksFromStatusRollup() format (rawState.toUpperCase()).
   The CI failure fingerprint includes c.conclusion ?? '', so inconsistent
   conclusion values between GraphQL and REST paths caused phantom fingerprint
   changes when switching sources, triggering duplicate dispatches.

* fix(scm-github): normalize CheckRun conclusion and map NEUTRAL to skipped

Two consistency fixes in parseCheckContexts() vs the REST path:

1. NEUTRAL conclusion: was mapped to 'passed' (with SUCCESS), but
   mapRawCheckStateToStatus() in the REST path maps NEUTRAL to 'skipped'.
   Changed to treat NEUTRAL the same as SKIPPED.

2. CheckRun conclusion: was stored as the raw GraphQL string (may be
   lowercase). REST getCIChecks/getCIChecksFromStatusRollup always store
   conclusion as rawState.toUpperCase(). Now stores rawConclusion which
   is already uppercased during the status branching logic.

Both fixes prevent phantom fingerprint changes when maybeDispatchCIFailureDetails
switches between GraphQL batch and REST fallback across poll cycles.

* fix(scm-github): map STALE/NOT_REQUIRED/NONE conclusions to skipped

parseCheckContexts() was mapping these conclusions to 'failed' via the
else fallback, while mapRawCheckStateToStatus() in the REST path
explicitly maps all of them to 'skipped'. Added them to the skipped
branch alongside SKIPPED and NEUTRAL to fully mirror the REST mapping.

* fix(scm-github): map QUEUED/WAITING to pending not running

parseCheckContexts() mapped QUEUED and WAITING CheckRun statuses to
'running', but mapRawCheckStateToStatus() in the REST path maps both
to 'pending'. Only IN_PROGRESS maps to 'running' in the REST path.

Fixes fingerprint inconsistency when switching between GraphQL batch
and REST fallback across poll cycles.

* fix(scm-github): map STARTUP_FAILURE to skipped; guard null pageInfo

- STARTUP_FAILURE conclusion now falls through to the "skipped" branch
  (matching mapRawCheckStateToStatus() REST default) instead of the
  explicit failure enumeration catch-all
- Null pageInfo guard prevents TypeError from typeof null === "object"
  JavaScript quirk when accessing hasNextPage on a null pageInfo field
- Tests added for both cases

* fix(scm-github): map COMPLETED+null conclusion to skipped not passed

When a CheckRun has status COMPLETED and conclusion null, the REST path's
mapRawCheckStateToStatus() converts it to "" which maps to "skipped".
The GraphQL path was incorrectly mapping it to "passed" via !rawConclusion.
Fix: only map rawConclusion === "SUCCESS" to "passed"; null falls through
to the else branch → "skipped", matching the REST path exactly.
2026-04-06 22:52:47 -07:00
Harsh Batheja 649fc67f6a revert: remove model rate-limit pause functionality (PR #367) (#908)
* Revert "fix: pause workers on model limits and stabilize session visibility (#367)"

This reverts commit 003eb78adb.

* fix: resolve post-revert type errors and broken test references

- Fix dangling import { in session-manager.ts (leftover from removing globalPause imports)
- Fix duplicate registerStop body in start.ts (leftover parent-version code after catch block)
- Remove --keep-session flag from stop command (was added by #367)
- Fix PullRequestsPage.tsx useSessionEvents call removing dropped initialGlobalPause arg
- Remove globalPause test cases from spawn.test.ts, communication.test.ts, api-routes.test.ts
- Remove unused updateMetadata imports from test files

* fix: remove unused allSessions variable after globalPause removal
2026-04-06 22:52:47 -07:00
harshitsinghbhandari 49426ccddb fix comments 2026-04-06 22:48:01 -07:00
harshitsinghbhandari 10163f0ec2 fix tests 2026-04-06 22:48:01 -07:00
Harsh Batheja 7e53542f9d feat: support multiple concurrent orchestrators with isolated worktrees (#870)
* feat: support multiple concurrent orchestrators with isolated worktrees

- Each orchestrator session gets a numbered ID ({prefix}-orchestrator-N)
  and an isolated git worktree, replacing the single shared orchestrator
- reserveNextOrchestratorIdentity atomically reserves the next available
  number and detects conflicting project prefix configurations early
- isOrchestratorSession / isOrchestratorSessionName accept optional
  allSessionPrefixes for cross-project false-positive prevention
- getProjectPause and resolveGlobalPause track the longest active pause
  across all concurrent orchestrators instead of returning the first found
- cleanupWorktreeAndMetadata helper used consistently on all failure paths
  including post-launch; system prompt file included in cleanup
- pollBacklog, status.ts, ProjectSidebar, sessions page, global-pause,
  project-utils, serialize, and sessions API route all pass
  allSessionPrefixes to isOrchestratorSession
- Web sessions/[id]/page.tsx fetches project prefix map and passes it
  through prefixByProjectRef to avoid stale closure in fetchProjectSessions

* fix: seed sseAttentionLevels from fresh sessions on full refresh

When scheduleRefresh dispatches a reset action, derive sseAttentionLevels
from the freshly fetched sessions using getAttentionLevel. This clears
phantom entries for removed sessions and seeds correct levels for new
sessions, preventing stale favicon color and attention counts until the
next SSE snapshot.

* fix: merge duplicate @/lib/types imports into single import statement

Combining the separate import type and value import from @/lib/types
into one import to satisfy the no-duplicate-imports lint rule.
2026-04-06 22:48:01 -07:00
harshitsinghbhandari c92c5ebaff fix: address illegalcall review comments
- Fix bad temp name for non-standard packages: use full packageName
  instead of last hyphen-segment to avoid collisions between packages
  like "custom-tracker-plugin" and "my-custom-plugin"

- Fix disabled top-level plugin blocking inline loading: when an
  existing plugin entry has `enabled: false`, it's now set to `true`
  when there's an inline tracker/scm/notifier reference for the same
  package/path

- Create validatePluginConfigFields helper to deduplicate the
  TrackerConfigSchema, SCMConfigSchema, and NotifierConfigSchema
  validation logic (was duplicated thrice)

- Fix multi-project external plugin order-dependency: tracker and SCM
  plugins now receive no project-level config at create() time, making
  them consistent with built-in plugins. Project-specific config is
  passed per-call via ProjectConfig argument, avoiding first-project
  wins behavior

- Document post-validation invariant for plugin field: added
  documentation clarifying that `plugin` is always populated after
  validateConfig() to help downstream consumers understand they can
  safely assume non-null after validation

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-06 22:46:29 -07:00
harshitsinghbhandari 8ffd6a6298 comments fixed 2026-04-06 22:46:29 -07:00
harshitsinghbhandari ee7f08ada7 performed review and fixed 2026-04-06 22:46:29 -07:00
harshitsinghbhandari 23cb1ee7a8 fix load builtin error 2026-04-06 22:46:29 -07:00
harshitsinghbhandari 3e6d2f97be feat(core): unify plugin config cleaning and enforce reserved fields for tracker and scm 2026-04-06 22:46:29 -07:00
harshitsinghbhandari 4645f6574b feat(core): enforce reserved fields in notifier configs and improve loader error handling 2026-04-06 22:46:29 -07:00
harshitsinghbhandari ff47d363c1 add error handling for path in config 2026-04-06 22:46:29 -07:00
harshitsinghbhandari e799c01086 fix(core): ensure consistent temp plugin name generation 2026-04-06 22:46:29 -07:00
harshitsinghbhandari 986c499acb fix: ensure notifier config is passed when manifest name differs
extractPluginConfig was being called with mod.manifest.name before the
config was updated from the temp name. When manifest name differs from
temp name (e.g., temp "teams" vs manifest "ms-teams"), extractPluginConfig
found no match and returned undefined, causing the plugin to be registered
without its configuration (webhook URLs, tokens, etc.).

Fix by reordering operations:
1. Validate and update configs with manifest.name FIRST
2. Extract plugin config AFTER (now finds the config by manifest.name)
3. Register plugin with its correct configuration

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-06 22:46:29 -07:00
harshitsinghbhandari 1396d07900 fix: prevent one project's bad config from breaking shared plugin
When multiple projects share the same external plugin, a misconfiguration
in one project (wrong expectedPluginName) would prevent the plugin from
being registered at all, silently breaking it for all projects.

Fix by:
1. Register the plugin FIRST, before validating individual project configs
2. Validate each project's config in its own try-catch
3. Log validation errors but continue processing other projects

This ensures:
- Plugin is always registered if it loads successfully
- Projects with correct config get updated
- Projects with bad config get a warning but don't break others
- Misconfigured projects fail at point of use with clearer errors

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-06 22:46:29 -07:00
harshitsinghbhandari b936114e52 fix: preserve multi-word plugin names from package specifiers
generateTempPluginName was splitting package names on both hyphens and
slashes, which truncated multi-word names like "jira-cloud" to just
"cloud".

Fix by first extracting the package name without scope, then using a
regex to match the common ao-plugin-{slot}-{name} pattern which
preserves the full plugin name including hyphens.

Examples:
- @acme/ao-plugin-tracker-jira-cloud -> jira-cloud (was: cloud)
- ao-plugin-scm-azure-devops -> azure-devops (was: devops)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-06 22:46:29 -07:00
harshitsinghbhandari 40f4efaca4 fix: address cursor bugbot review comments
- Strip package and path loading metadata from notifier config passed
  to plugin create(). These fields are used for plugin resolution, not
  plugin-specific configuration. Prevents filesystem paths from leaking
  into plugin config where they could conflict with plugin-specific
  fields of the same name.

- Remove collectExternalPluginConfigs from public API exports since it
  is only used internally within config.ts during validateConfig.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-06 22:46:29 -07:00
harshitsinghbhandari 8e4f4f6380 fix: address remaining PR review comments
- Fix multiple projects sharing same external plugin: change
  findExternalPluginEntry to findAllExternalPluginEntries which returns
  all matching entries, allowing all projects to be updated with the
  actual manifest.name

- Fix path without slashes misidentified as npm package: use separate
  pkg and path parameters in generateTempPluginName instead of combining
  them and checking for slashes. Local paths like "my-tracker" (no
  slashes) are now correctly returned as-is

- Replace console.warn with process.stderr.write for structured logging
  in plugin-registry.ts

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-06 22:46:29 -07:00
harshitsinghbhandari dfe879d7c2 fix: remove unused import and add missing location field
- Remove unused collectExternalPluginConfigs import from tests
- Add missing location field to _externalPluginEntries in test

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-06 22:46:29 -07:00
harshitsinghbhandari 470e53780d fix: address Copilot review feedback on external plugin support
1. Only set expectedPluginName when user explicitly specifies `plugin`
   - Auto-generated temp names no longer block manifest name inference
   - External plugins like @acme/ao-plugin-tracker-jira-cloud with
     manifest.name "jira-cloud" will now load correctly

2. Add structured location data to ExternalPluginEntryRef
   - New ExternalPluginLocation type with kind/projectId/configType
   - Avoids fragile string parsing of source (project/notifier keys
     can legally contain dots)

3. Expand home paths (~/) in inline plugin configs
   - Apply expandHome() to tracker/scm/notifier path fields
   - Consistent behavior with config.plugins path expansion

4. Improve error messages
   - Use "package" vs "path" in manifest mismatch errors instead
     of always saying "package"

5. Fix tests to use config._externalPluginEntries
   - Tests now check entries stored by validateConfig() instead
     of calling collectExternalPluginConfigs() again on modified config

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-06 22:46:29 -07:00
harshitsinghbhandari ebe6408586 feat(core): support external tracker, scm, and notifier plugins from config
Add support for specifying external plugins inline in tracker, scm, and
notifier configs using optional `package` or `path` fields.

**Config schema changes:**
- TrackerConfig, SCMConfig, NotifierConfig now accept optional `package`
  (npm) or `path` (local) fields alongside the existing `plugin` field
- When only `package`/`path` is specified, plugin name is auto-inferred
- When both `plugin` and `package`/`path` are specified, manifest.name
  is validated against the declared plugin name

**Example usage:**
```yaml
projects:
  my-app:
    tracker:
      plugin: jira  # optional - validates against manifest.name
      package: "@acme/ao-plugin-tracker-jira"
      teamId: "TEAM-123"
    scm:
      path: ./plugins/my-scm  # local plugin

notifiers:
  teams:
    package: "@acme/ao-plugin-notifier-teams"
```

**Implementation details:**
- collectExternalPluginConfigs() extracts external plugin entries from
  tracker/scm/notifier configs during config validation
- Auto-generates InstalledPluginConfig entries merged into config.plugins
- Plugin registry validates manifest.name matches expectedPluginName and
  updates config with actual manifest.name after loading
- Warns when plugin slot doesn't match configured slot

Closes #736

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-06 22:46:29 -07:00
Harsh Batheja 41c967a396 fix: use TERMINAL_STATUSES in lifecycle poll accounting
Replace hardcoded "merged"/"killed" checks in pollAll() with the
canonical TERMINAL_STATUSES set from types.ts. This ensures sessions
in done, terminated, cleanup, or errored states are correctly treated
as inactive for polling, active-count, and all-complete detection.

Closes #752
2026-04-06 22:46:29 -07:00
Your Name f07aedfa58 fix: resolve prompt delivery persistence and false warnings 2026-04-06 22:46:29 -07:00
Your Name 3d1913e690 fix: robust prompt delivery with retries and CLI warnings 2026-04-06 22:46:29 -07:00
Ashish Huddar 1bb80ef67c
feat: stable session titles via pinnedSummary metadata (#946)
* feat(core): pin first quality summary for stable session titles

When a session's first non-fallback summary is obtained, persist it to
metadata as pinnedSummary. This prevents the dashboard title from drifting
as the agent generates new summaries each turn.

Changes:
- Add pinnedSummary to SessionMetadata type
- Serialize/deserialize in writeMetadata/readMetadata
- Pin in lifecycle manager's checkSession (correct write path)
- Propagate through restore path

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(web): stable session titles via unified getSessionTitle

Reorder title fallback: PR > issue > branch > summary (was: summary > issue > branch).
Replace inline getSessionHeadline with shared getSessionTitle in both
SessionDetail and page.tsx for consistent behavior across all components.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: coverage for pinnedSummary and title fallback chain

9 new tests:
- 3 serialize tests: pinnedSummary priority over agent/metadata summary
- 5 lifecycle-manager tests: pinning logic guards and error handling
- 1 metadata test: pinnedSummary serialization in writeMetadata

Updated existing tests to match new title fallback order.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: fix mobile title expectations

* fix(web): use pinnedSummary only for title selection, not dashboard.summary

Previously, pinnedSummary was assigned to dashboard.summary in
sessionToDashboard(), causing two problems:
1. enrichSessionAgentSummary() never fetched newer summaries because
   dashboard.summary was already set
2. Every UI surface rendering session.summary showed the stale pinned
   value instead of live agent output

Fix: pinnedSummary is now read only by getSessionTitle() via
session.metadata["pinnedSummary"], keeping title stable without
freezing the live summary field. dashboard.summary always reflects the
current agentInfo or metadata summary.

Addresses review comment on PR #946.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 09:52:48 +05:30
Ashish Huddar 30ee327daf
fix(notifier): remove desktop notifications from default configs (#962)
* fix(notifier): remove desktop notifications from all default configs (#960)

Desktop notifications via osascript open blank Finder windows on macOS
without providing useful information. Remove 'desktop' from all default
notifier lists so new users are not affected. Existing configs that
explicitly list 'desktop' continue to work unchanged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(cli): add coverage for autoCreateConfig default notifiers

Line 585 in start.ts (`notifiers: []`) was not covered by the diff
coverage check. Added a test that exercises `autoCreateConfig` via
`createConfigOnly()`, verifying that the generated config does not
include 'desktop' in `defaults.notifiers`.

Required infrastructure additions:
- Mock `node:process` so `import { cwd } from "node:process"` in
  start.ts can be overridden per-test via `mockProcessCwd`
- Set `detectProjectType` mock to return `{ languages: [], frameworks: [] }`
  instead of null to avoid crash in autoCreateConfig

Closes #960

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(config): disable all notifications by default — fully opt-in

Change DefaultPluginsSchema.notifiers default from ["composio"] to []
and notificationRouting defaults to all empty arrays, so no notifications
are sent unless the user explicitly configures them. Addresses reviewer
feedback that composio notifier should not be active out of the box.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(config): use empty object as notificationRouting default

Empty array entries like { urgent: [] } block the ?? fallback in
notifyHuman (lifecycle-manager.ts:1169) because [] is not null/undefined.
Using {} lets unlisted priorities fall back to defaults.notifiers, so
explicit per-user notifier config is respected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-07 08:59:51 +05:30
Harsh Batheja e25e9b4967
fix(lifecycle): reduce GitHub API rate limiting from batch enrichment bypass (#906)
* fix(lifecycle): reduce GitHub API rate limiting from batch enrichment bypass

Three optimizations to prevent API storms in the lifecycle manager poll cycle:

1. **CRITICAL - maybeDispatchMergeConflicts**: Gate the getMergeability()
   fallback to only run when batch enrichment didn't run at all. Previously
   it called getMergeability() (3 REST calls) whenever hasConflicts was
   undefined, even when the batch had already fetched PR data. Now uses
   cachedData.hasConflicts ?? false when the batch ran.

2. **HIGH - maybeDispatchCIFailureDetails**: Use batch enrichment ciChecks
   when available instead of calling getCIChecks() (separate REST call)
   on every poll. The GraphQL batch query now fetches statusCheckRollup
   contexts (individual check names, statuses, URLs) alongside the rollup
   state. Falls back to getCIChecks() only when batch didn't run.

3. **MEDIUM - maybeDispatchReviewBacklog**: Throttle getPendingComments +
   getAutomatedComments API calls to at most once per 2 minutes per session.
   These were called every 30s even when nothing had changed.

Impact: ~8-10 API calls/PR/poll reduced to ~2-4, enabling 3-4x more
concurrent sessions before hitting GitHub's 5,000/hr REST limit.

Also extends PREnrichmentData with ciChecks?: CICheck[] and adds
parseCheckContexts() helper to graphql-batch.ts for parsing CheckRun
and StatusContext nodes from the GraphQL statusCheckRollup.contexts field.

* fix(scm-github): fall back to getCIChecks() when contexts list is truncated

When a PR has >20 CI checks, contexts(first: 20) silently truncates the
list. Setting ciChecks to undefined when pageInfo.hasNextPage is true
ensures maybeDispatchCIFailureDetails falls back to the getCIChecks()
REST call, which returns all checks without truncation.

Also adds pageInfo { hasNextPage } to the contexts GraphQL query so
truncation can be detected.

* fix(lifecycle): prune lastReviewBacklogCheckAt in pollAll cleanup loop

Add the new throttle map to the existing pruning loop that removes stale
entries for sessions no longer in the session list. Previously the map
was only cleared on terminal status transitions, leaving orphaned entries
for sessions removed externally (killed + cleaned up without transition).

* fix(lifecycle): bypass throttle on review transition; fix StatusContext conclusion

Two fixes for automated review findings:

1. Bypass review backlog throttle when a transition reaction just fired for
   humanReactionKey or automatedReactionKey. The transitionReaction branch
   needs to read the current fingerprint via the API to record
   lastPendingReviewDispatchHash. Without bypassing, the throttle prevents
   this write and the next unthrottled poll sees a stale (empty) hash,
   clears the reaction tracker, and fires a duplicate dispatch.

2. Set conclusion on StatusContext nodes in parseCheckContexts() to match
   the REST getCIChecksFromStatusRollup() format (rawState.toUpperCase()).
   The CI failure fingerprint includes c.conclusion ?? '', so inconsistent
   conclusion values between GraphQL and REST paths caused phantom fingerprint
   changes when switching sources, triggering duplicate dispatches.

* fix(scm-github): normalize CheckRun conclusion and map NEUTRAL to skipped

Two consistency fixes in parseCheckContexts() vs the REST path:

1. NEUTRAL conclusion: was mapped to 'passed' (with SUCCESS), but
   mapRawCheckStateToStatus() in the REST path maps NEUTRAL to 'skipped'.
   Changed to treat NEUTRAL the same as SKIPPED.

2. CheckRun conclusion: was stored as the raw GraphQL string (may be
   lowercase). REST getCIChecks/getCIChecksFromStatusRollup always store
   conclusion as rawState.toUpperCase(). Now stores rawConclusion which
   is already uppercased during the status branching logic.

Both fixes prevent phantom fingerprint changes when maybeDispatchCIFailureDetails
switches between GraphQL batch and REST fallback across poll cycles.

* fix(scm-github): map STALE/NOT_REQUIRED/NONE conclusions to skipped

parseCheckContexts() was mapping these conclusions to 'failed' via the
else fallback, while mapRawCheckStateToStatus() in the REST path
explicitly maps all of them to 'skipped'. Added them to the skipped
branch alongside SKIPPED and NEUTRAL to fully mirror the REST mapping.

* fix(scm-github): map QUEUED/WAITING to pending not running

parseCheckContexts() mapped QUEUED and WAITING CheckRun statuses to
'running', but mapRawCheckStateToStatus() in the REST path maps both
to 'pending'. Only IN_PROGRESS maps to 'running' in the REST path.

Fixes fingerprint inconsistency when switching between GraphQL batch
and REST fallback across poll cycles.

* fix(scm-github): map STARTUP_FAILURE to skipped; guard null pageInfo

- STARTUP_FAILURE conclusion now falls through to the "skipped" branch
  (matching mapRawCheckStateToStatus() REST default) instead of the
  explicit failure enumeration catch-all
- Null pageInfo guard prevents TypeError from typeof null === "object"
  JavaScript quirk when accessing hasNextPage on a null pageInfo field
- Tests added for both cases

* fix(scm-github): map COMPLETED+null conclusion to skipped not passed

When a CheckRun has status COMPLETED and conclusion null, the REST path's
mapRawCheckStateToStatus() converts it to "" which maps to "skipped".
The GraphQL path was incorrectly mapping it to "passed" via !rawConclusion.
Fix: only map rawConclusion === "SUCCESS" to "passed"; null falls through
to the else branch → "skipped", matching the REST path exactly.
2026-04-06 14:59:00 +05:30
Harsh Batheja 9ac659066a
revert: remove model rate-limit pause functionality (PR #367) (#908)
* Revert "fix: pause workers on model limits and stabilize session visibility (#367)"

This reverts commit 003eb78adb.

* fix: resolve post-revert type errors and broken test references

- Fix dangling import { in session-manager.ts (leftover from removing globalPause imports)
- Fix duplicate registerStop body in start.ts (leftover parent-version code after catch block)
- Remove --keep-session flag from stop command (was added by #367)
- Fix PullRequestsPage.tsx useSessionEvents call removing dropped initialGlobalPause arg
- Remove globalPause test cases from spawn.test.ts, communication.test.ts, api-routes.test.ts
- Remove unused updateMetadata imports from test files

* fix: remove unused allSessions variable after globalPause removal
2026-04-06 14:58:41 +05:30