Skip to content

fix: preserve Google Workspace MCP credentials and add runner feedback route#1234

Open
ambient-code[bot] wants to merge 4 commits intomainfrom
fix/1222-mcp-auth-feedback-endpoint
Open

fix: preserve Google Workspace MCP credentials and add runner feedback route#1234
ambient-code[bot] wants to merge 4 commits intomainfrom
fix/1222-mcp-auth-feedback-endpoint

Conversation

@ambient-code
Copy link
Copy Markdown
Contributor

@ambient-code ambient-code bot commented Apr 7, 2026

Summary

Fixes #1222 — Google Workspace MCP authentication fails in ACP sessions and Amber feedback workflow can't submit feedback.

  • Google Workspace MCP: Stop deleting credentials.json in clear_runtime_credentials() between turns. The workspace-mcp process reads from this file continuously; removing it forces an inaccessible localhost:8000 OAuth flow. Fresh credentials are written at each run start anyway.
  • Skip redundant first-run credential clear: On first run, _setup_platform() already populates credentials and builds MCP servers with correctly expanded env vars. The subsequent clear_runtime_credentials() in _initialize_run() was needlessly clearing env vars like USER_GOOGLE_EMAIL before re-populating them.
  • Credential refresh diagnostics: refresh_credentials tool now verifies MCP auth status after refresh and includes diagnostic warnings, preventing false positives where it reports success but MCP servers still can't authenticate.
  • Feedback endpoint: Add runner-accessible route at .../runner/feedback outside ValidateProjectContext() middleware so in-session workflows (Amber) can submit feedback using BOT_TOKEN without user OAuth tokens. Handler retains its own SSAR auth check.

Test plan

  • Existing tests pass (34 tests in test_shared_session_credentials.py)
  • New test: test_preserves_google_credentials_file verifies credentials file is NOT deleted during cleanup
  • New test: test_includes_mcp_diagnostics_on_auth_warning verifies MCP diagnostics are included in refresh response
  • Go backend compiles successfully with new route
  • Manual: verify workspace-mcp retains credentials across multi-turn sessions
  • Manual: verify Amber feedback workflow can submit via /runner/feedback endpoint

🤖 Generated with Claude Code

…runner feedback route (#1222)

Three fixes for issues reported in #1222:

1. **Google Workspace MCP auth**: Stop deleting the credentials file
   (`credentials.json`) in `clear_runtime_credentials()`. The workspace-mcp
   process reads from this file continuously; removing it between turns
   forces it into an inaccessible localhost OAuth flow. The file is
   overwritten with fresh tokens at the start of each run anyway.

2. **Credential refresh diagnostics**: After refreshing credentials, the
   `refresh_credentials` tool now checks MCP server auth status and
   includes diagnostic warnings in its response. This prevents false
   positives where the tool reports success but MCP servers still can't
   authenticate.

3. **Feedback endpoint auth**: Add a runner-accessible feedback route at
   `/api/projects/:projectName/agentic-sessions/:sessionName/runner/feedback`
   that bypasses `ValidateProjectContext()` middleware. In-session workflows
   (e.g. Amber) can now submit feedback using BOT_TOKEN without requiring
   a user OAuth token. The handler still performs its own SSAR auth check.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ambient-code ambient-code bot added the ambient-code:managed PR managed by AI automation label Apr 7, 2026
adalton pushed a commit that referenced this pull request Apr 7, 2026
## Summary

When the agent creates a PR from an issue (via `ambient-code:auto-fix`
label or `@ambient-code` on an issue), it now sends a Slack notification
with links to the PR and the session.

## Message format

```
PR created for #1234
PR: https://github.com/ambient-code/platform/pull/5678
Session: https://ambient.ai/projects/my-project/sessions/session-abc123
```

## Test plan

- [ ] Add `ambient-code:auto-fix` to an issue — verify Slack
notification sent after PR created
- [ ] `@ambient-code` on an issue — verify same

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Chores**
* Updated workflow logic to require creation of a plan and to prompt for
clarification when ambiguous.
* Added conditional Slack notifications: one before requesting user
clarification and one after PR creation (sent only if configured).
* Adjusted workflow step ordering and related guidance to reflect the
new notification and clarification flow.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Ambient Code Bot <bot@ambient-code.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ver env vars

On first run, _setup_platform() already populates credentials and builds
MCP servers with correctly expanded env vars. The subsequent
clear_runtime_credentials() call in _initialize_run() would needlessly
remove env vars like USER_GOOGLE_EMAIL before re-populating them,
creating a brief window where workspace-mcp env state is inconsistent.

Skip the clear-repopulate cycle on first run since _setup_platform()
already handled it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Gkrumbach07 added a commit that referenced this pull request Apr 7, 2026
## Summary

Rewrite the review queue from an LLM-powered session to a deterministic
shell script. Now acts as a strict readiness gate — only PRs that are
actually mergeable get added to the milestone and notified in Slack.

## Changes

### Review Queue (`pr-merge-review.yml`)
- **Hourly** (was daily at 1pm)
- **Shell-driven** (was LLM session — cheaper, faster, deterministic)
- **Strict readiness check**: CI must pass (not pending), no changes
requested, not draft
- **Milestone management**: adds ready PRs, removes ones that fell out
of readiness
- **Slack notification only for NEW additions** — not every run, not
every PR
- Milestone description auto-updated with ready/not-ready lists

### Amber Handler
- **Removed "PR created" Slack notifications** — these were noisy
- The review queue now handles the notification when PRs are actually
ready

## Slack message format
```
📋 2 PRs ready for review

• #1234 — Fix session timeout handling
• #1235 — Add model selector to header

Total in Review Queue: 5
```

## Test plan
- [ ] Manual dispatch review queue — verify milestone updated
- [ ] Open a PR with CI passing — verify it gets added and Slack
notified
- [ ] PR with failing CI — verify NOT added to milestone
- [ ] Draft PR — verify skipped

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Chores**
  * Removed Slack prompts from PR-creation session flows
  * Review-queue schedule changed to run hourly on weekdays
  * Added concurrency controls to review-queue runs

* **New Features**
* Review queue now evaluates PR readiness (drafts, CI, reviews) and
updates a milestone
  * Slack notifications for PRs newly marked as ready
* Batch PR fixer now skips healthy PRs by checking CI, mergeability, and
review states
* PR comments and bodies now include a session footer/link when relevant
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Ambient Code Bot <bot@ambient-code.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Gkrumbach07
Copy link
Copy Markdown
Contributor

@ambient-code tell me a joke

@github-actions github-actions bot added this to the Review Queue milestone Apr 7, 2026
@Gkrumbach07
Copy link
Copy Markdown
Contributor

@ambient-code reply that joke as a comment

@Gkrumbach07
Copy link
Copy Markdown
Contributor

@ambient-code do a review

…dback-endpoint

# Conflicts:
#	components/runners/ambient-runner/tests/test_shared_session_credentials.py
@ambient-code ambient-code bot added ambient-code:needs-human AI circuit breaker fired, needs human attention and removed ambient-code:managed PR managed by AI automation labels Apr 8, 2026
@ambient-code
Copy link
Copy Markdown
Contributor Author

ambient-code bot commented Apr 8, 2026

🔴 Circuit breaker triggered (retry_count=3).

All code changes are complete and pushed. Merge conflict with origin/main resolved (test file conflict in test_shared_session_credentials.py). All 34 runner tests pass.

The PR addresses both sub-issues from #1222:

  1. Google Workspace MCP auth — credentials file preserved between turns
  2. Runner feedback endpoint — added route outside ValidateProjectContext() middleware

CI was green before the merge; waiting for new checks to complete. Needs human review and merge.

🤖 Session

@jeremyeder jeremyeder enabled auto-merge (squash) April 10, 2026 18:44
@github-actions
Copy link
Copy Markdown
Contributor

Pull Reviews

Positive — This PR fixes Google Workspace MCP credential preservation and adds a runner-accessible feedback endpoint. It prevents credential file deletion between turns and introduces diagnostics for MCP authentication issues.

https://pub-510dcf6312d143b68159aa24908df5dc.r2.dev/videos/ci-ambient-code-platform-1234-1775847546941.mp4

Callouts

  • warning (breaking-change): Removal of Google credential file deletion changes behavior of clear_runtime_credentials(). Any code depending on this function fully clearing all credentials could be affected, though the new behavior appears more correct for the MCP use case.
  • warning (complexity): The _first_run flag logic adds state tracking to bridge.py that must be carefully maintained. If _first_run is not properly set during initialization, credentials could be stale on the first run.
  • info (testing): The new runner feedback endpoint in routes.go has no unit test coverage visible in this diff. Integration tests should verify that the endpoint correctly validates service account tokens and performs SSAR checks.
File-by-file breakdown
  • components/runners/ambient-runner/ambient_runner/platform/auth.py (high): Preserve Google Workspace credentials file to prevent MCP server authentication failures between conversation turns
  • components/backends/routes.go (high): Add a runner-accessible feedback endpoint outside of user OAuth validation middleware
  • components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py (high): Optimize credential initialization to avoid unnecessary clearing and repopulation on first run
  • components/runners/ambient-runner/ambient_runner/bridges/claude/tools.py (medium): Add MCP authentication diagnostics to credential refresh feedback and helper function
  • components/runners/ambient-runner/tests/test_shared_session_credentials.py (medium): Add test coverage for Google credential preservation and MCP diagnostics

Generated by pull-reviews — automated video reviews for PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ambient-code:needs-human AI circuit breaker fired, needs human attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Google Workspace MCP authentication fails in ACP + Amber feedback workflow can't submit feedback

2 participants