Skip to content

feat(gaia): add developer-only coding-agent MCP handoffs - #3911

Merged
itomek merged 15 commits into
amd:mainfrom
kovtcharov:codex/harness-engineering-mcp
Sep 29, 2026
Merged

itomek merged 15 commits into
amd:mainfrom
kovtcharov:codex/harness-engineering-mcp

Conversation

@kovtcharov

Copy link
Copy Markdown
Contributor

Developers can now report friction while using GAIA and hand an explicitly approved snapshot to their existing Codex or Claude Code app. Developer mode loads the skill and prepares a cached source repository; the coding app diagnoses the problem, requests code scope, and iterates in a dedicated worktree with separately launched previews. Ordinary sessions do not expose this capability.

The usage guide and architecture scope are included. Autonomous fixes are a later phase; stable auto-update is tracked in #3898. @kovtcharov-amd: ready for architecture and manual pilot review.

Test plan

  • 184 focused Python tests, including real stdio MCP integration, developer-mode isolation, approval/revocation and worktree recovery.
  • Go CLI, confirmation UI and client tests; 22 WebUI consent tests; TypeScript check and production build.
  • Live macOS TUI → approved synthetic snapshot → native Codex MCP read/report roundtrip; revoke afterward.
  • Live WebUI synthetic request displayed recipient/context and required fresh consent without an “always allow” option.
  • Python formatting/lint, wheel package contents, skill validation and 992 documentation links.
  • Maintainer manual TUI/candidate-worktree pilot and human review.
  • Agent eval scorecard: attempted against the live backend, but the Claude-based runner hit its weekly quota. Codex end-to-end validation passed; live Claude inference remains unverified.
🔍 Technical details

The bridge provides recipient-bound, expiring snapshots through an explicitly enabled local stdio server. Code approval is tied to the displayed diagnosis revision. Preview metadata and results are reported by the coding app, not supervised or independently verified by GAIA. Packaged build provenance, live grants, automatic trace capture and automatic contribution orchestration remain future work.

Run gaia-tui --developer-mode --dev, then follow the guide.

Synthetic live WebUI consent evidence:

Developer-only sharing consent

@github-actions github-actions Bot added documentation Documentation changes dependencies Dependency updates mcp MCP integration changes cli CLI changes tests Test changes agents tui Go terminal UI (gaia-tui) labels Sep 15, 2026
@kovtcharov
kovtcharov marked this pull request as ready for review September 17, 2026 20:04
@github-actions

Copy link
Copy Markdown
Contributor

Request changes

This adds an opt-in developer mode that lets a GAIA user hand a reported problem to their own Claude Code or Codex app over a local, consent-gated MCP bridge, with managed git worktrees for the fix. The consent design is the strongest part of it: sharing can't be remembered, can't be auto-approved in bypass mode, and revocation really does cut off an already-connected client. I found no runtime bugs — the two things to fix are cheap.

A new doc file describes a capability this PR doesn't build. A second copy of the engineering skill was added under the docs tree. It isn't the one the agent loads, nothing links to it, and its text tells the model it can offer "bounded live sharing" with a grant that keeps covering later events. Snapshots that expire are the only thing implemented — the spec page in this very PR lists live grants as a future extension. Delete the stray copy, or point the docs at the real one.

The new CLI test runs the installed gaia binary instead of this checkout. Every other subprocess test in the suite launches the CLI through the interpreter for exactly this reason: on a machine with more than one worktree, the installed script points at whichever one was pip-installed last, so the test can pass or fail against code that isn't in this PR. It also errors rather than skips where the script isn't installed.

Five smaller suggestions are in the technical details — a worktree branch named after Codex even on the Claude path, a credential-redaction gap for JSON-shaped secrets, and a few convention/doc-duplication items.

Real-world evidence

No evidence-bundle.md was produced for this run, and I had no network or gh access in this environment, so I could not read the PR description to see what evidence it carries. What I could verify directly in the tree:

  • Agent UI — a real screenshot of the permission modal is committed at docs/images/harness-engineering/sharing-consent.png. It shows the share_engineering_context request with its arguments and disclosure text, Allow/Deny, and no "Always allow this tool" checkbox — which is exactly the behaviour the frontend change is supposed to produce.
  • MCP — tests/mcp/test_engineering_mcp.py drives a real stdio client against the server: it lists tools, reads context, gets refused on prepare_worktree before approval, and gets refused again after revocation. That is a genuine protocol-level exercise, not a mock.
  • CLI — no captured gaia engineering output was available to me; coverage there is the subprocess test flagged above.
  • TUI — static review only.

The verdict rests on static review plus those two artifacts. Nothing I saw contradicts the change working; neither finding above is disproved or supported by the evidence.

🔍 Technical details

🟡 Important

1. Orphan skill copy contradicts both the shipped skill and this PR's own spec (docs/spec/skills/gaia-harness-engineering/SKILL.md:1)

Two files now carry name: gaia-harness-engineering with different bodies:

  • hub/agents/gaia/python/gaia_agent/skills/gaia-harness-engineering/SKILL.md (52 lines) — the one GaiaAgent.__init__ actually loads via _bundled_skill_dirs(). Correct: snapshot-only.
  • docs/spec/skills/gaia-harness-engineering/SKILL.md (90 lines) — new directory, referenced by nothing (not docs/docs.json, not code, not any .mdx).

The docs copy instructs: "Ask permission for a snapshot or bounded live sharing of this task. A live grant covers later events only within its approved categories, recipient and lifetime."

JobStore.create (src/gaia/engineering/store.py:164) only ever writes "mode": "snapshot", and docs/spec/harness-engineering-mode.mdx:297 states plainly that live grants "are design extensions below, not implemented capabilities." Per CLAUDE.md ("a second copy is a second thing to drift"), and since the two copies already disagree on day one, drop the duplicate or replace it with a link to the shipped path.

2. tests/unit/engineering/test_cli.py:15 escapes the checkout pin

[str(Path(sys.executable).parent / "gaia"), "engineering", *args],

The root conftest.py pins sys.path for the test process; a subprocess launching the installed console script does not inherit that. It resolves gaia.cli:main out of site-packages — whichever worktree ran pip install -e last. tests/unit/cli/test_cli_smoke.py:154 documents the house rule: "Uses [sys.executable, "-m", <module>] rather than the bare binary name to match the project subprocess convention … and avoid PATH dependency." Every other CLI subprocess test in the suite (test_skills_migrate.py:777, eval/test_iterations.py:114, cli/test_cli_telegram.py:24) follows it; this is the only file that doesn't. It also raises FileNotFoundError instead of skipping when the script isn't installed.

        [sys.executable, "-m", "gaia.cli", "engineering", *args],

with the repo's src on PYTHONPATH in the env= dict so the child imports this checkout.

🟢 Minor

3. Worktree branch says codex/ on the Claude path (src/gaia/engineering/repository.py:164)

branch = f"codex/engineering-{job_id}" is used for both backends, so a Claude-backed job gets a branch prefixed with the other vendor's name. It surfaces in create_worktree's return value and in git branch output inside the worktree the developer works in.

            branch = f"gaia/engineering-{job_id}"

4. Credential redaction misses JSON-shaped secrets (src/gaia/engineering/store.py:94)

The key: value pattern requires the delimiter to follow the key name directly, so api_key=sk-... is redacted but "api_key": "sk-abc" — the shape most logs and config dumps take — is not. The sk- / gh*_ literal patterns catch many real tokens, and the guide correctly tells users to inspect content themselves, so this is defence-in-depth rather than a hole. Allowing an optional closing quote costs one character:

        r"(?i)(\b(?:api[_-]?key|access[_-]?token|password|secret|authorization)\"?\s*[:=]\s*)([^\n]+)",

5. Cache-path branch compares a resolved path to an unresolved one (src/gaia/engineering/service.py:44)

self.root comes back from JobStore already .resolve()d, but it's compared against a raw Path.home() / ".gaia" / "engineering". If ~/.gaia is a symlink (a common "small system disk" setup), the equality fails and worktrees silently land in ~/.gaia/engineering/cache instead of the ~/.gaia/cache/engineering the guide documents. Resolving both sides makes the comparison mean what it reads as.

6. tests/unit/engineering/ has no __init__.py

Every other package under tests/unit/ (agents/, api/, chat/, cli/, connectors/, eval/, installer/, mcp/, rag/) ships one. Without it, test_cli.py and test_store.py are imported as bare top-level modules — and tests/unit/connectors/ already has files with both of those basenames. It happens to work today because the connectors package resolves to a dotted name, but it's one missing __init__.py away from an "import file mismatch" collection error.

7. The same 10-line paragraph was pasted into three npm docs (hub/agents/gaia/npm/{README,SKILL,SPEC}.md)

Byte-identical text in all three. CLAUDE.md gives each a distinct job — README is integrator-facing, SPEC is the technical reference, SKILL is the AI-assistant playbook. Worth differentiating: SPEC should carry the flag/env contract and the frozen-binary limitation, SKILL should say what an assistant may and may not claim about a handoff.

(Not counted as a nit — one pattern note: evidence appends are bounded in practice to roughly 15 snapshots by MAX_RECORD_BYTES against MAX_CONTEXT_BYTES. The error is actionable and nothing is corrupted, but the guide's iterate-until-fixed loop doesn't mention a ceiling.)

Strengths

  • The consent model holds up under adversarial reading. grant_scope returns None for all three gated tools, so no "always allow" can ever be recorded; _engineering_consent independently refuses when auto_approve_confirmations_enabled() or call_is_granted() is true; and the frontend ignores any pre-existing localStorage entry via requiresFreshConsent. Three layers that each fail closed, with a test for each.
  • Instance-local tool registration is the right fix for a real race. @tool(registry=...) keeps developer closures out of the process-global _TOOL_REGISTRY entirely, and test_overlapping_chat_construction_never_sees_developer_tools constructs an ordinary ChatAgent at the exact interleaving point to prove a concurrent WebUI agent can't snapshot them. The decorator change is minimal and backward-compatible.
  • The honesty plumbing is unusually disciplined. task_created: False, prompt_prefilled, connection_verified, verification: "reported", process_owner: "coding_app" — every place the system could let a model imply more happened than did, it returns an explicit negative instead, and the skill text tells the model not to say "posted to Codex."
  • Git handling is careful: staged bare clone with atomic os.replace, pinned per-job base ref so a later upstream fetch can't move a job's base, remote verified against the official URL before every worktree operation, symlink checks throughout, and GIT_TERMINAL_PROMPT=0 so a credential prompt can't hang the agent. test_clone_worktree_retry_preserves_base_and_work exercises the interrupted-retry path against a real repository.
  • All four hub doc surfaces (README, SPEC, SKILL, CHANGELOG) were updated together, as CLAUDE.md requires.

Kalin Ovtcharov added 4 commits September 18, 2026 23:54
…ring-mcp

# Conflicts:
#	hub/agents/gaia/npm/CHANGELOG.md
#	hub/agents/gaia/python/gaia_agent/stdio.py
#	src/gaia/apps/webui/src/components/ChatView.tsx
#	src/gaia/apps/webui/src/components/PermissionPrompt.tsx
#	src/gaia/apps/webui/src/stores/notificationStore.ts
…ering CLI

GaiaAgent listed EngineeringToolsMixin ahead of ProjectMapMixin, which broke
the test that pins ProjectMapMixin as the first base so Agent's no-op
task-start hook can't shadow it. The two mixins share no methods, so the
engineering mixin now sits second.

The engineering CLI test ran the installed `gaia` script, which on a machine
with several worktrees imports whichever one was pip-installed last. It now
runs `python -m gaia.cli` with this checkout's src on PYTHONPATH, like the
other CLI subprocess tests. tests/unit/engineering gains the __init__.py every
sibling test package has, so its test_cli/test_store modules can't collide
with the same basenames under tests/unit/connectors.

Removes docs/spec/skills/gaia-harness-engineering/SKILL.md: nothing loaded or
linked it, and it described live sharing grants that aren't implemented.
…s for GAIA

- Redaction missed `"api_key": "..."`, the shape most logs and config dumps
  take, because the key had to be followed directly by `:` or `=`. An optional
  closing quote now matches it too.
- Worktree branches were named `codex/engineering-<job>` even when Claude Code
  did the work; they are now `gaia/engineering-<job>`.
- The default-profile check compared a resolved root against an unresolved
  home path, so a symlinked ~/.gaia put worktrees under the wrong cache
  directory. Both sides are resolved now.
- The context cursor check uses isinstance (pylint C0123) and still rejects
  bools.
- The npm README, SPEC and SKILL carried one pasted paragraph. README now says
  what the mode is for, SPEC carries the flag, tool, consent and packaging
  contract, and SKILL says what an assistant may and may not claim about a
  handoff.
The spec linked the draft skill copy under docs/spec/skills, which was removed
as an orphan. It now names the shipped location, which the host loads only in
developer mode.
@kovtcharov

Copy link
Copy Markdown
Contributor Author

The branch now merges cleanly with current main, the three failing checks are fixed, and all seven review items are addressed. The one merge conflict with real impact: main changed "always allow" into a per-chat, in-memory grant. The rule that engineering sharing and code approval always ask again now lives in that new grant check, so it still holds.

  • Unit tests (py3.10/3.12/macOS): GaiaAgent lists ProjectMapMixin first again. The two mixins share no methods (e144d13).
  • Code quality (pylint C0123): the cursor check uses isinstance and still rejects bools (6078b5d).
  • Verify external URLs: that failure was a dead link in the v0.15.3 release notes that main already fixed. Merging picked it up.
  • 🟡 Orphan skill copy: deleted (e144d13). The architecture spec linked to it, so it now names the shipped skill instead (e713123).
  • 🟡 CLI test used the installed gaia: it now runs python -m gaia.cli against this checkout (e144d13).
  • 🟢 codex/ branch prefix: worktree branches are now gaia/engineering-<job> (6078b5d).
  • 🟢 JSON-shaped secrets: "api_key": "…" and 'password': '…' are now redacted (6078b5d).
  • 🟢 Symlinked ~/.gaia: both sides of the cache-path check are resolved (6078b5d).
  • 🟢 Missing __init__.py: added (e144d13).
  • 🟢 Same paragraph in three npm docs: README now covers what the mode is for, SPEC the flag, tool and consent contract, and SKILL what an assistant may claim about a handoff (6078b5d).

Still open: the maintainer pilot, the agent eval, and the note about the evidence-size ceiling, which the review didn't count as an item.

🔍 Technical details

Conflicts (merge 0c45842)

  • notificationStore.ts: took main's per-chat alwaysAllowGrants. respondToPermission keeps the PR's rememberChoice for both the grant and the Electron IPC flag. isAlwaysAllowed now returns false for requiresFreshConsent tools, so a grant for an engineering tool can't auto-approve even if one exists.
  • ChatView.tsx: identical to main. main removed the tool_confirm branch the PR had edited, and the guard now lives in isAlwaysAllowed.
  • PermissionPrompt.tsx: main's "Allow this tool for the rest of this chat" label, still hidden for fresh-consent tools.
  • Tests rewritten for the grant model: notificationStore.engineering.test.ts checks that no grant is recorded, remember is false over IPC, an existing grant is ignored, and unrelated tools still get grants. The ChatView.confirmation case seeds a chat grant and expects the prompt, not an auto-confirm. PermissionPrompt.engineering asserts the new label is absent.
  • stdio.py: kept both the engineering setup-status notice and main's clear-conversation sentinel. CHANGELOG.md: kept both Added entries.

Other

  • clean_context key pattern gains [\"']? before \s*[:=].
  • New tests: test_context_cursor_must_be_a_nonnegative_int (True, -1, "1", 1.0), JSON and dict redaction cases, and a branch-name assertion in the worktree test.

Results

  • pytest tests/unit/engineering tests/mcp/test_engineering_mcp.py tests/unit/test_project_map.py tests/unit/test_tool_decorator.py tests/unit/test_amd_gaia_urls.py: 127 passed.
  • hub/agents/gaia/python/tests/test_engineering_mode.py: 15 passed.
  • The full hub gaia suite before the fix commits: 166 passed. test_full_tool_bundles fails on search_documentation; that test needs npx, which this Mac doesn't have.
  • WebUI: vitest 401/401 and tsc --noEmit clean. TUI: go build ./..., and go vet / go test on internal/cli and internal/ui/components, all pass.
  • black, isort, flake8 and pylint (CI flags) show nothing in the touched files. check_doc_links.py --internal-only passes.

@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

Merged main in — the Test Gaia Agent failure here was not this branch's doing. It was a check that main itself fixed after this branch forked, so the branch was failing on stale code.

🔍 Technical details

The failing step was Run Skill Framework Tests, specifically tests/unit/test_starter_skills.py::test_starter_skill_tools_required_are_real_tools[coding] — AssertionError: coding declares tools_required that no mixin registers. The coding skill started requiring run_python, added by #3994; #4051 then taught the starter-skill guard to see it. This branch predated both.

The merge was clean. pytest tests/unit/test_starter_skills.py on the merged head → 188 passed, 15 skipped.

The same stale-base failure hit #3687, #3911, #3982 and #3616 identically.

# Conflicts:
#	hub/agents/gaia/python/gaia_agent/agent.py
@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

Not merging this unilaterally, for a reason the PR itself states: the body asks "@kovtcharov-amd: ready for architecture and manual pilot review", and the test plan's "Maintainer manual TUI/candidate-worktree pilot and human review" box is unchecked. That is an author-designated human gate, and an agent squash-merging past it would defeat the point of asking.

It is also the right gate to have. This feature hands an approved snapshot of a user's source tree to a third-party coding agent (Codex / Claude Code) over a local stdio MCP server. The boundary being crossed is data egress, and the controls — recipient-binding, snapshot expiry, code approval tied to the displayed diagnosis revision, no "always allow" — are exactly the kind of thing that needs a person to exercise once rather than read about.

All seven review findings are addressed — I checked each against the head commits, not just the author's summary:

  • orphan skill copy deleted, spec re-pointed at the shipped skill (e144d135, e7131236)
  • CLI test runs python -m gaia.cli against the checkout instead of the installed gaia (e144d135)
  • worktree branches renamed codex/ → gaia/engineering-<job> (6078b5da)
  • JSON-shaped secrets ("api_key": "…", 'password': '…') now redacted (6078b5da)
  • symlinked ~/.gaia resolved on both sides of the cache-path check (6078b5da)
  • missing __init__.py added (e144d135)
  • the duplicated paragraph split across npm README / SPEC / SKILL (6078b5da)

Nothing is outstanding from review.

What a human needs to do before merge: run the pilot, and decide whether the two acknowledged gaps ship as-is — preview metadata and results are reported by the coding app, not verified by GAIA, and there is no evidence-size ceiling.

🔍 Technical details

Layering reads clean. src/gaia/engineering/ is a new leaf subsystem (handoff, identity, repository, service, store, cli) with the MCP surface in src/gaia/mcp/servers/engineering_mcp.py and the agent-facing tools in hub/agents/gaia/python/gaia_agent/engineering_tools.py. Dependencies point downward; nothing in agents/base/ imports it. The only core-framework touch is src/gaia/agents/base/tools.py, which is the requiresFreshConsent plumbing the consent model needs.

The merge-conflict resolution that matters. main replaced "always allow" with a per-chat in-memory grant while this branch was open. The invariant — engineering sharing and code approval always re-ask — was re-established inside the new grant check rather than alongside it: isAlwaysAllowed returns false for requiresFreshConsent tools, so a pre-existing chat grant cannot auto-approve an engineering tool. Tests were rewritten to the grant model rather than patched (notificationStore.engineering.test.ts asserts no grant is recorded, remember is false over IPC, and an existing grant is ignored). That is the correct place for the rule.

Merge state: CONFLICTING / DIRTY, 5 conflicts — src/gaia/agents/base/tools.py, src/gaia/cli.py, tests/unit/test_tool_decorator.py, tui/internal/cli/root.go, hub/agents/gaia/npm/CHANGELOG.md. Small, and none look semantic on inspection, but tools.py is core framework so it wants care.

CI: one real failure, Test GAIA CLI on Windows (Full Integration) — not this PR's doing. It dies at 31s inside the install-lemonade composite action during server discovery, before any test in this PR's scope runs. Same class as the known stale-Lemonade-port issue (#4281). The earlier Test Gaia Agent failure was the test_starter_skill_tools_required_are_real_tools[coding] stale-base problem that main fixed in #4051, and it hit #3687, #3982 and #3616 identically.

Eval: attempted, but the Claude-based runner hit its weekly quota. Codex end-to-end passed; live Claude inference is unverified. Since this adds tools to the flagship's surface, the eval is in scope under CLAUDE.md and should run before merge.

# Conflicts:
#	docs/docs.json
#	hub/agents/gaia/npm/CHANGELOG.md
#	setup.py
#	src/gaia/agents/base/tools.py
#	src/gaia/cli.py
#	tests/unit/test_tool_decorator.py
#	tui/internal/cli/root.go

@kovtcharov-amd kovtcharov-amd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rebased the branch onto current main and pushed — conflicts had grown since the last review pass because main kept moving (7 files now, up from the 5 noted a couple of days ago, including a real clash where two different features added a keyword argument to the same function signature on the same line). Both features are preserved in the merge; local tests, lint, and the Go build all pass, and fresh CI is running against the merged branch.

The one new thing I found: the shared context this feature writes to disk is only protected on Linux/macOS. On Windows, the file-permission lockdown it uses doesn't actually restrict other local accounts from reading it — this repo already hit and fixed the identical bug for the daemon's launch secret, but this feature doesn't reuse that fix. Worth closing before this reaches general availability, since confidentiality of the shared snapshot is half of the feature's pitch.

Everything else already raised in this thread — the developer-mode gate, the consent model, the subsystem layering — checks out on an independent read. Not requesting changes; the maintainer's already-stated plan (pilot + eval before merge) stands.

🔍 Technical details

Merge: pushed c2b6297ea to kovtcharov/gaia:codex/harness-engineering-mcp (merge of origin/main). Conflicts resolved, all additive — no contradictory logic:

  • docs/docs.json, hub/agents/gaia/npm/CHANGELOG.md, setup.py: both sides' entries kept.
  • src/gaia/agents/base/tools.py: @tool() keeps both preflight (main) and registry (this PR) kwargs; _SUPPORTED_TOOL_KWARGS lists both.
  • src/gaia/cli.py: both the engineering dispatch branch and the use_chatgpt removed-provider guard kept.
  • tests/unit/test_tool_decorator.py: expected-error message updated to list both kwargs.
  • tui/internal/cli/root.go: developerMode and fullAccessFlag vars coexist; no leftover references to the pre-rename name.

Verified locally: pytest tests/unit/test_tool_decorator.py tests/unit/engineering tests/mcp/test_engineering_mcp.py (41 passed, 1 pre-existing platform gap — see below); pytest tests/unit/engineering/test_cli.py tests/unit/test_amd_gaia_urls.py tests/unit/test_starter_skills.py (208 passed, 15 skipped); black/isort/targeted pylint clean on touched files; python util/lint.py --tool-descriptions clean (88 flagship schemas within budget); go build ./... and go vet ./internal/cli/... clean in tui/.

Windows ACL gap: private_directory() (src/gaia/engineering/store.py:41-42) does path.mkdir(mode=0o700) / path.chmod(0o700) only. tests/unit/engineering/test_store.py::test_atomic_concurrent_feedback_preserves_all_updates asserts st_mode & 0o777 == 0o600 with no platform skip, and fails as-is on Windows in this environment — but tests/unit/ only runs on ubuntu-latest/macos-latest in CI (.github/workflows/test_unit.yml), so this has never been exercised on Windows there. The fix pattern already exists in-repo: _lock_down_windows_acl in src/gaia/daemon/sidecars/manager.py:115 builds a real NTFS DACL restricted to the current user, with a docstring citing issue #2250 for exactly this "POSIX mode bits are a no-op on Windows" failure mode. It's currently private to the sidecar manager; worth factoring out (or replicating) for gaia.engineering.store.

CI: the previous "Test GAIA CLI on Windows (Full Integration)" failure was HttpError: API rate limit exceeded for installation fetching the FedericoCarboni/setup-ffmpeg@v3 action, not a test failure — confirmed from the job log. That workflow (test_gaia_cli_windows.yml) doesn't run tests/unit/engineering/ at all (its own comment notes tests/unit/ is Ubuntu-only), so it wouldn't have caught the ACL gap above either way.

Gate verification: EngineeringService.__init__ (src/gaia/engineering/service.py:29-36) raises PermissionError unless developer_mode=True (passed by whoever launched gaia mcp engineering --developer-mode ...) or GAIA_DEVELOPER_MODE=1 was already set in that process's environment before any MCP client connected. The stdio protocol gives the connecting Claude Code / Codex client no channel to set this itself, so the gate can't be spoofed client-side.

Layering: src/gaia/engineering/ doesn't touch src/gaia/connectors/, which is correct — connectors govern GAIA-as-client grants to external services (Google/GitHub OAuth, MCP servers); here GAIA is the MCP server a local coding app connects to, an orthogonal direction. No inconsistency with the existing grant model.

…xclusion

test_cli_docs_drift.py (added on main since this branch forked) now walks
the parser to the leaf and requires each gaia engineering subcommand, and
the Global Options exclusion list, to name the literal command.
@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

Pushed one more fix after that review: main grew a new guard (tests/unit/cli/test_cli_docs_drift.py, not present when this branch forked) that walks the CLI parser and requires every leaf subcommand to appear verbatim in docs/reference/cli.mdx. It caught two real gaps in this PR's own docs — the "Developer engineering" section named the gaia engineering subcommands in prose but never spelled out gaia engineering <subcommand> literally, and the new top-level command wasn't listed among those that reject the shared --logging-level/--claude-model/--no-lemonade-check flags. Both fixed and pushed (32f39214d): a subcommand table with the literal invocations, and gaia engineering added to the Global Options exclusion list. pytest tests/unit/cli/test_cli_docs_drift.py and util/check_doc_links.py --internal-only are clean.

One correction to my last comment: I said tests/unit/ only runs on Linux/macOS in CI. There's also a Unit Tests (Windows smoke) job — I missed it lower in test_unit.yml. It's continue-on-error: true (an explicit rollout exception while the suite is verified clean on Windows) and stops at the first failure (-x), so an unrelated flake earlier in the alphabetical run can mask a real failure deeper in — which is what happened on the CI run just now (test_client_disconnect_cancels_the_agent in tests/unit/api/, timing-sensitive and reproduces the same way on plain main with no PR changes, so unrelated to this branch). That doesn't reach tests/unit/engineering/ in this run, but it means the Windows ACL gap from my last comment still isn't being verified there in practice, just via a non-blocking job that stopped before it got there.

private_directory() only did POSIX chmod(0o700), a no-op on NTFS, so a
shared engineering snapshot or grant was readable by any other local
account on a shared Windows box. Reuses the same DACL-lockdown pattern
already shipped for the daemon's launch secret (amd#2250).
Environment variables like DB_PASSWORD or AWS_SECRET_ACCESS_KEY carry a prefix, and the snapshot scrubber required a word boundary before the credential keyword, so those values were shipped in the clear to the third-party coding agent. Prefixed and suffixed names now match, and auth tokens and passwd are covered too.

Prose like "secretary: alice" and code like parser.add_argument("--api-key") are left untouched.
@itomek

itomek commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Ran the maintainer pilot that was still unchecked on the test plan — the full host flow plus a real stdio MCP round-trip from a simulated coding app. The handoff works and the consent model holds: developer mode cannot be switched on by an ambient environment variable, a bad pairing credential stops the server from starting at all, forged job and evidence ids are refused, prepare_worktree is refused until the host approves code scope, and a host revoke cuts off the coding app's next read immediately.

One thing needed fixing before this lands, now pushed as ccdcdf8: the snapshot scrubber missed credentials whose name carries a prefix. DB_PASSWORD=... and AWS_SECRET_ACCESS_KEY=... were handed to the coding agent in the clear while API_KEY=... on the very next line was redacted. Prefixed and suffixed names are now covered, and ordinary prose like secretary: alice is still left alone.

🔍 Technical details

Cause. clean_context in src/gaia/engineering/store.py anchored its credential-name alternation with \b. _ is a word character, so DB_PASSWORD has no boundary before PASSWORD and never matched. The fix allows a leading name run and an optional separator-led suffix; the separator requirement is what keeps secretary: from being treated as secret.

Measured on the pre-fix branch, against the shipped function:

ok    API_KEY=abc123xyz            -> API_KEY=[REDACTED]
LEAK  DB_PASSWORD=hunter2          -> DB_PASSWORD=hunter2
LEAK  AWS_SECRET_ACCESS_KEY=wJal.. -> AWS_SECRET_ACCESS_KEY=wJal..
LEAK  MY_API_KEY=zzz               -> MY_API_KEY=zzz
LEAK  ANTHROPIC_AUTH_TOKEN=xyzzy   -> ANTHROPIC_AUTH_TOKEN=xyzzy
LEAK  client_secret=shhh           -> client_secret=shhh

After the fix: 11/11 of those shapes redacted, and 0/6 false positives across
secretary: alice, the password reset flow, parser.add_argument("--api-key"),
secretariat = 1972, Secrets are managed by vault,
# password rotation policy: 90 days.

What the coding app actually received over MCP, before and after — same input file, real
JSON-RPC get_context against gaia.mcp.servers.engineering_mcp over stdio:

before: "DB_PASSWORD=hunter2\n  API_KEY=[REDACTED]\n  export ANTHROPIC_AUTH_TOKEN=[REDACTED]"
after:  "DB_PASSWORD=[REDACTED]\n  API_KEY=[REDACTED]\n  export ANTHROPIC_AUTH_TOKEN=[REDACTED]"

Pilot transcript. initialize succeeded and tools/list returned all seven tools
(connection_status, get_context, get_evidence, report_diagnosis, prepare_worktree,
register_preview, report_result). Then:

Check Result
engineering status with no --developer-mode refused
same with GAIA_DEVELOPER_MODE=1 set but flag absent refused
MCP server started with a wrong GAIA_ENGINEERING_TOKEN PermissionError: Invalid engineering client credential, server exits
get_context on an approved job returns snapshot + grant + build identity
get_context on a forged job id refused
get_evidence on a forged evidence id refused
prepare_worktree before host code approval refused
get_context after host revoke refused

Tests. tests/unit/engineering, hub/agents/gaia/python/tests/test_engineering_mode.py
and tests/mcp/test_engineering_mcp.py: 46 passed, 0 failed, including the new prefixed-name
regression case in tests/unit/engineering/test_store.py.

Noticed, not fixed here — --root "" resolves to the current working directory, so the
pairing credential and job store get written wherever the command was run. Worth a follow-up;
it does not block this PR.

@github-actions github-actions Bot added the devops DevOps/infrastructure changes label Sep 29, 2026
@itomek

itomek commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

CI triage for the three red lanes, now resolved.

Lint was ours and is fixed in 6026733: the PR added tests/mcp/test_engineering_mcp.py without wiring it into any CI lane, which trips the repo's own Test Lane Coverage gate. It now runs in the offline-integration lane.

Windows and macOS smoke are pre-existing on main and unrelated to this PR — the same single test fails identically on main's own tip, and this PR touches nothing in that code path.

🔍 Technical details

python util/lint.py --all, same command on both trees:

Tree Result
origin/main (86fb455) 15 checks, 0 failed — ALL QUALITY CHECKS PASSED
PR head before 6026733 15 checks, 1 critical — Test Lane Coverage
PR head after 6026733 15 checks, 0 failed

So the lint failure was genuinely introduced here, not inherited.

Smoke lanes — Unit Tests (Windows smoke) on main's own tip (run 36576942529, head 86fb455):

FAILED tests/unit/api/test_chat_completion_stream.py::test_client_disconnect_cancels_the_agent - assert False
1 failed, 1969 passed, 111 skipped in 217.47s

and on this PR (run 36579718567, head ccdcdf8):

FAILED tests/unit/api/test_chat_completion_stream.py::test_client_disconnect_cancels_the_agent - assert False
1 failed, 1969 passed, 111 skipped in 214.73s

Same test, same assertion, same counts. Note that main's test_unit.yml run still concludes success with those jobs red, so the lane is non-blocking there — which is why this has gone unnoticed. Worth a separate issue; it does not belong to this PR.

@itomek
itomek added this pull request to the merge queue Sep 29, 2026
Merged via the queue into amd:main with commit c54ebdf Sep 29, 2026
90 of 92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents cli CLI changes dependencies Dependency updates devops DevOps/infrastructure changes documentation Documentation changes mcp MCP integration changes tests Test changes tui Go terminal UI (gaia-tui)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants