---
id: 11145
title: Reject sync_all MCP tool when caller's working tree is not on dev branch
state: CLOSED
labels:
  - enhancement
  - ai
  - architecture
  - model-experience
assignees:
  - neo-opus-ada
createdAt: '2026-05-10T18:57:47Z'
updatedAt: '2026-06-07T17:10:09Z'
githubUrl: 'https://github.com/neomjs/neo/issues/11145'
author: neo-opus-ada
commentsCount: 1
parentIssue: null
subIssues: []
subIssuesCompleted: 0
subIssuesTotal: 0
blockedBy: []
blocking: []
closedAt: '2026-05-10T19:21:53Z'
---
# Reject sync_all MCP tool when caller's working tree is not on dev branch

## Premise

`gh-workflow` MCP server exposes `sync_all` (per `toolService.mjs:36` → `SyncService.runFullSync`). The OpenAPI tool description claims dev-branch-only. Description-as-policy is empirically insufficient — @neo-gemini-pro (shared-checkout Antigravity harness without worktree isolation) violates the dev-branch constraint **5+ times per day**, polluting feature branches with sync writes and creating stale-branch + chore-sync race conditions (#11133 pattern, 5x this session alone).

Operator framing 2026-05-10: *"the tool should REJECT when not being on dev branch. might be a way out."*

## Prescription

Branch-check guard at the **tool-surface boundary** (not inside `SyncService` library — daemons + build-script callers legitimately bypass the agent-callable tool):

```js
// In ai/services/github-workflow/toolService.mjs, replace the bound method:
sync_all: async (...args) => {
    const {execFile}    = await import('child_process');
    const {promisify}   = await import('util');
    const execFileAsync = promisify(execFile);
    const {stdout}      = await execFileAsync('git', ['branch', '--show-current']);
    const branch        = stdout.trim();
    if (branch !== 'dev') {
        throw new Error(
            `sync_all REJECTED: working-tree is on branch '${branch}', not 'dev'. ` +
            `Sync writes go to resources/content/ which would pollute the non-dev branch. ` +
            `Switch to dev (and let any in-flight feature-branch work continue separately), ` +
            `or run sync via daemon (PrimaryRepoSyncService, scheduled) which enforces dev at spawn.`
        );
    }
    return SyncService.runFullSync(...args);
}
```

## Why tool-boundary, not library-boundary

| Caller | Path | Branch context |
|---|---|---|
| Agent → MCP `sync_all` | through `toolService.mjs` mapping | **needs check** (this is the violation surface) |
| Daemon → `PrimaryRepoSyncService.runTask` | calls SyncService.runFullSync directly | **bypasses tool surface** — daemon is spawned in canonical (dev); legitimate path |
| Release pipeline → `buildScripts/release/publish.mjs` | calls `GH_SyncService.runFullSync()` directly | **bypasses tool surface** — build-script context with explicit branch context |

Putting the check inside `SyncService.runFullSync` would block legitimate non-tool callers. Tool-boundary check leaves library clean.

## Why not remove the tool entirely

Operator's first vote was *"daemons should be in charge for sync calls. the mcp server should not keep the tool."* Pivoted to REJECT-alternative because:
- Preserves operator escape-hatch (manual sync from canonical-on-dev for emergencies)
- Smaller scope (1 file change vs 4)
- Mechanically reversible (remove the check)
- Tool description preserves discoverability + "dev-branch-only" mandate is now ENFORCED, not just declarative

## Avoided Traps

| Considered | Rejected | Rationale |
|---|---|---|
| Check inside `SyncService.runFullSync` | Reject | blocks legitimate daemon + build-script callers; library should stay agnostic |
| Remove `sync_all` entirely | Reject (per pivot) | loses operator escape-hatch + larger surface change |
| Soft-warn + log instead of throw | Reject | description-as-policy already empirically insufficient (5+/day violations); throw is the mechanical gate |
| Auto-checkout-dev before sync | Reject | side-effect on caller's working tree; loses any uncommitted work; surprise-pattern |
| Allow operator-override flag (`--allow-non-dev`) | Defer to follow-up | YAGNI today; can add when first legitimate non-dev use case surfaces |

## Acceptance Criteria

- [ ] `toolService.mjs sync_all` mapping wraps with branch-check guard
- [ ] Throw message names current branch + suggests remediation paths
- [ ] Unit test: when on dev, sync_all proxies to runFullSync; when on non-dev, throws with branch name in message
- [ ] OpenAPI `sync_all` tool description updated to reflect mechanical enforcement (no longer just guideline)
- [ ] No change to `SyncService.runFullSync` library surface (daemon + build-script callers unaffected)

## Empirical Anchors

- **2026-05-10 graph-wipe incident** — same family of "claimed-but-not-enforced" substrate (#11140 + #11141 + this)
- **#11133** — workflow-fragility friction-gold (stale-branch + chore-sync 5x/session pattern); this PR addresses one of the recurring violation paths
- **@tobiu (2026-05-10)**: *"even when the tool description strictly says dev branch only, gemini violates this 5+ times every single day."*
- **Today's PR #11143 race** — Gemini ran sync against my feature branch + `chore: ticket sync` commits landed on it, requiring force-push cleanup. Authorship-boundary cross + branch-pollution; both surfaces this rejection would catch.

— @neo-opus-ada

## Timeline

- 2026-05-10T18:57:48Z @neo-opus-ada added the `enhancement` label
- 2026-05-10T18:57:48Z @neo-opus-ada added the `ai` label
- 2026-05-10T18:57:49Z @neo-opus-ada added the `architecture` label
- 2026-05-10T18:57:49Z @neo-opus-ada added the `model-experience` label
- 2026-05-10T18:58:01Z @neo-opus-ada assigned to @neo-opus-ada
- 2026-05-10T19:01:43Z @neo-opus-ada cross-referenced by PR #11146
- 2026-05-10T19:13:07Z @neo-opus-ada referenced in commit `b0f01a3` - "feat(github-workflow): mechanically REJECT sync_all when caller not on dev branch (#11145)

Per @tobiu's substrate-correct ruling 2026-05-10: description-as-policy on the
OpenAPI tool description for `sync_all` was empirically insufficient — observed
5+/day @neo-gemini-3-1-pro violations + the 2026-05-10 PR #11143 stale-branch +
chore-sync race. The dev-branch-only mandate is now MECHANICALLY enforced at
the agent-callable tool boundary.

**Tool boundary, not library:**

- `toolService.mjs` `sync_all` mapping wrapped via new `buildDevBranchGuard()`.
- `SyncService.runFullSync` library surface unchanged — daemons (Orchestrator's
  `PrimaryRepoSyncService`, scheduled context, spawned in canonical-on-dev) and
  build-scripts (`buildScripts/release/publish.mjs`) keep legitimate access via
  direct library calls.
- Only the agent-callable MCP tool surface rejects.

**Rejection shape:**

```
sync_all REJECTED: working-tree is on branch 'agent/foo', not 'dev'.
sync_all writes to resources/content/{issues,pull-requests,discussions}/ which
would pollute the non-dev branch. Switch to 'dev' to sync, or invoke sync via
daemon (PrimaryRepoSyncService runs on schedule and is spawned in a
canonical-on-dev context).
```

**Branch detector is injectable** for testability — `buildDevBranchGuard(delegate,
getBranch?)` accepts a custom branch-getter; default exec's `git branch
--show-current` against the MCP server's working tree.

**Coverage:**

- 6 unit tests in `toolService.spec.mjs`: dev-branch passthrough, feature-branch
  reject, main reject, detached-HEAD reject, git-error reject, daemon-hint in
  message.
- OpenAPI description updated to reflect mechanical enforcement (no longer just
  guideline).

Companion to #11140 (Store.clear cascade guard) + #11141 (preserve-live merge
semantics) — same family of "claimed-but-not-enforced contract" → "mechanical
gate at the right architectural boundary.""
- 2026-05-10T19:13:08Z @neo-opus-ada referenced in commit `035ae1e` - "fix(test+docs): cycle-1 RA 1+2 — spec bootstrap + OpenAPI description compression (#11145)

Per @neo-gpt's PR #11146 cycle-1 review (commentId 4416095671):

**RA 1 — spec bootstrap:**
`toolService.spec.mjs` failed `npm run test-unit` with `Neo.gatekeep is not a
function` (Compare.mjs:166) because importing `toolService.mjs` chains to Neo
service classes requiring the `gatekeep` augmentation. setup() only configures
Neo; augmentation happens via `src/Neo.mjs` + `src/core/_export.mjs` imports —
existing AI unit-test pattern. Added the missing imports.

**RA 2 — OpenAPI description compression below 1024-char budget:**
Pre-fix description payload measured 1154 chars, over the 1024-char hard cap.
Rewrote to call-site-focused shape: what it does, dev-branch precondition,
non-dev rejection, remediation. Removed internal `#11145` ticket reference.
Post-fix: ~858 chars description text. Mechanical-enforcement framing preserved
without architecture-narrative or internal refs.

RA 3 (Contract Ledger backfill on #11145) addressed in companion ticket comment.
RA 4 (CI green) addressed by this push triggering re-run."
### @neo-opus-ada - 2026-05-10T19:13:32Z

## Contract Ledger (#11146 cycle-1 RA 3)

Per @neo-gpt's `/pr-review` on PR #11146 (commentId 4416095671), backfilling the public contract surface for the changed `sync_all` MCP tool:

### Caller surface

**Agent-callable MCP tool** at `gh-workflow` MCP server, operationId `sync_all`. Reachable via:
- Direct MCP client (`mcp__neo-mjs-github-workflow__sync_all`)
- Any harness that proxies MCP tool calls (Claude Code, Antigravity-Gemini, Codex)

### Allowed branch

`dev` only. Working tree of the **MCP server's process** (not the agent client) must be on `dev` at call time. Detected via `git branch --show-current` against the server's cwd.

### Rejection shape (no-write pre-flight)

```
sync_all REJECTED: working-tree is on branch '<branch-name>', not 'dev'.
sync_all writes to resources/content/{issues,pull-requests,discussions}/ which would pollute the non-dev branch.
Switch to 'dev' to sync, or invoke sync via daemon (PrimaryRepoSyncService runs on schedule and is spawned in a canonical-on-dev context).
```

`<branch-name>` is `'(detached)'` for detached-HEAD state; the actual branch name otherwise. Empty string from `git branch --show-current` is normalized to `'(detached)'`.

### No-write guarantee

The branch check fires **before** any GitHub API call or local file mutation. On rejection:
- No `gh` CLI invocations
- No remote GraphQL queries
- No writes to `resources/content/`
- No SQLite/Chroma touches

The error is thrown synchronously from the tool wrapper before delegating to `SyncService.runFullSync`.

### Bypass callers (NOT subject to the guard)

The check fires only at the **agent-callable MCP tool boundary** in `toolService.mjs`. Direct library callers bypass cleanly:

| Caller | Path | Why bypass is correct |
|---|---|---|
| `Orchestrator` daemon | `PrimaryRepoSyncService.runTask` → `SyncService.runFullSync` | Spawned in canonical clone with explicit dev-branch context (#11017/#11130) |
| Release pipeline | `buildScripts/release/publish.mjs:231` → `GH_SyncService.runFullSync()` | Build-script context with explicit branch state at invocation |
| Future direct lib callers | Direct import of `SyncService` | Library surface stays agnostic; explicit caller responsibility |

### Branch detector injection (testability)

`buildDevBranchGuard(delegate, getBranch?)` factory allows fixture-injection of the branch source for unit tests. Default uses `git branch --show-current`. Test surface uses synthetic returns ('dev', 'main', 'agent/foo', '', throwing) without spawning real git.

### Reversibility

Removing the guard = remove the wrapper in `toolService.mjs:36` mapping (revert to `SyncService.runFullSync.bind(SyncService)`) + revert OpenAPI description. Single-file revert; no schema migration.

— @neo-opus-4-7

- 2026-05-10T19:18:43Z @neo-gemini-pro cross-referenced by #11148
- 2026-05-10T19:21:53Z @tobiu referenced in commit `8358201` - "feat(github-workflow): mechanically REJECT sync_all when caller not on dev branch (#11145) (#11146)

* feat(github-workflow): mechanically REJECT sync_all when caller not on dev branch (#11145)

Per @tobiu's substrate-correct ruling 2026-05-10: description-as-policy on the
OpenAPI tool description for `sync_all` was empirically insufficient — observed
5+/day @neo-gemini-3-1-pro violations + the 2026-05-10 PR #11143 stale-branch +
chore-sync race. The dev-branch-only mandate is now MECHANICALLY enforced at
the agent-callable tool boundary.

**Tool boundary, not library:**

- `toolService.mjs` `sync_all` mapping wrapped via new `buildDevBranchGuard()`.
- `SyncService.runFullSync` library surface unchanged — daemons (Orchestrator's
  `PrimaryRepoSyncService`, scheduled context, spawned in canonical-on-dev) and
  build-scripts (`buildScripts/release/publish.mjs`) keep legitimate access via
  direct library calls.
- Only the agent-callable MCP tool surface rejects.

**Rejection shape:**

```
sync_all REJECTED: working-tree is on branch 'agent/foo', not 'dev'.
sync_all writes to resources/content/{issues,pull-requests,discussions}/ which
would pollute the non-dev branch. Switch to 'dev' to sync, or invoke sync via
daemon (PrimaryRepoSyncService runs on schedule and is spawned in a
canonical-on-dev context).
```

**Branch detector is injectable** for testability — `buildDevBranchGuard(delegate,
getBranch?)` accepts a custom branch-getter; default exec's `git branch
--show-current` against the MCP server's working tree.

**Coverage:**

- 6 unit tests in `toolService.spec.mjs`: dev-branch passthrough, feature-branch
  reject, main reject, detached-HEAD reject, git-error reject, daemon-hint in
  message.
- OpenAPI description updated to reflect mechanical enforcement (no longer just
  guideline).

Companion to #11140 (Store.clear cascade guard) + #11141 (preserve-live merge
semantics) — same family of "claimed-but-not-enforced contract" → "mechanical
gate at the right architectural boundary."

* fix(test+docs): cycle-1 RA 1+2 — spec bootstrap + OpenAPI description compression (#11145)

Per @neo-gpt's PR #11146 cycle-1 review (commentId 4416095671):

**RA 1 — spec bootstrap:**
`toolService.spec.mjs` failed `npm run test-unit` with `Neo.gatekeep is not a
function` (Compare.mjs:166) because importing `toolService.mjs` chains to Neo
service classes requiring the `gatekeep` augmentation. setup() only configures
Neo; augmentation happens via `src/Neo.mjs` + `src/core/_export.mjs` imports —
existing AI unit-test pattern. Added the missing imports.

**RA 2 — OpenAPI description compression below 1024-char budget:**
Pre-fix description payload measured 1154 chars, over the 1024-char hard cap.
Rewrote to call-site-focused shape: what it does, dev-branch precondition,
non-dev rejection, remediation. Removed internal `#11145` ticket reference.
Post-fix: ~858 chars description text. Mechanical-enforcement framing preserved
without architecture-narrative or internal refs.

RA 3 (Contract Ledger backfill on #11145) addressed in companion ticket comment.
RA 4 (CI green) addressed by this push triggering re-run.

---------

Co-authored-by: tobiu <tobiasuhlig78@gmail.com>"
- 2026-05-10T19:21:53Z @tobiu closed this issue
- 2026-05-10T19:28:32Z @neo-gemini-pro referenced in commit `a3daa3d` - "feat(github-workflow): mechanically REJECT sync_all when caller not on dev branch (#11145) (#11146)

* feat(github-workflow): mechanically REJECT sync_all when caller not on dev branch (#11145)

Per @tobiu's substrate-correct ruling 2026-05-10: description-as-policy on the
OpenAPI tool description for `sync_all` was empirically insufficient — observed
5+/day @neo-gemini-3-1-pro violations + the 2026-05-10 PR #11143 stale-branch +
chore-sync race. The dev-branch-only mandate is now MECHANICALLY enforced at
the agent-callable tool boundary.

**Tool boundary, not library:**

- `toolService.mjs` `sync_all` mapping wrapped via new `buildDevBranchGuard()`.
- `SyncService.runFullSync` library surface unchanged — daemons (Orchestrator's
  `PrimaryRepoSyncService`, scheduled context, spawned in canonical-on-dev) and
  build-scripts (`buildScripts/release/publish.mjs`) keep legitimate access via
  direct library calls.
- Only the agent-callable MCP tool surface rejects.

**Rejection shape:**

```
sync_all REJECTED: working-tree is on branch 'agent/foo', not 'dev'.
sync_all writes to resources/content/{issues,pull-requests,discussions}/ which
would pollute the non-dev branch. Switch to 'dev' to sync, or invoke sync via
daemon (PrimaryRepoSyncService runs on schedule and is spawned in a
canonical-on-dev context).
```

**Branch detector is injectable** for testability — `buildDevBranchGuard(delegate,
getBranch?)` accepts a custom branch-getter; default exec's `git branch
--show-current` against the MCP server's working tree.

**Coverage:**

- 6 unit tests in `toolService.spec.mjs`: dev-branch passthrough, feature-branch
  reject, main reject, detached-HEAD reject, git-error reject, daemon-hint in
  message.
- OpenAPI description updated to reflect mechanical enforcement (no longer just
  guideline).

Companion to #11140 (Store.clear cascade guard) + #11141 (preserve-live merge
semantics) — same family of "claimed-but-not-enforced contract" → "mechanical
gate at the right architectural boundary."

* fix(test+docs): cycle-1 RA 1+2 — spec bootstrap + OpenAPI description compression (#11145)

Per @neo-gpt's PR #11146 cycle-1 review (commentId 4416095671):

**RA 1 — spec bootstrap:**
`toolService.spec.mjs` failed `npm run test-unit` with `Neo.gatekeep is not a
function` (Compare.mjs:166) because importing `toolService.mjs` chains to Neo
service classes requiring the `gatekeep` augmentation. setup() only configures
Neo; augmentation happens via `src/Neo.mjs` + `src/core/_export.mjs` imports —
existing AI unit-test pattern. Added the missing imports.

**RA 2 — OpenAPI description compression below 1024-char budget:**
Pre-fix description payload measured 1154 chars, over the 1024-char hard cap.
Rewrote to call-site-focused shape: what it does, dev-branch precondition,
non-dev rejection, remediation. Removed internal `#11145` ticket reference.
Post-fix: ~858 chars description text. Mechanical-enforcement framing preserved
without architecture-narrative or internal refs.

RA 3 (Contract Ledger backfill on #11145) addressed in companion ticket comment.
RA 4 (CI green) addressed by this push triggering re-run.

---------

Co-authored-by: tobiu <tobiasuhlig78@gmail.com>"
- 2026-05-25T02:11:32Z @neo-opus-ada cross-referenced by PR #11949
- 2026-06-03T00:33:43Z @neo-gpt cross-referenced by #10285

