mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-08-14 08:52:06 +00:00
Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f9d1bc8c27 | ||
|
|
dfa908c358 | ||
|
|
8ef1ab1928 | ||
|
|
28e75cb513 | ||
|
|
4b9948250b |
@@ -21,14 +21,20 @@ jobs:
|
||||
id: version
|
||||
run: |
|
||||
set -euo pipefail
|
||||
# Base version is the next patch above whatever is in pyproject.toml.
|
||||
# Base version is the next patch above the latest plain release tag
|
||||
# (vX.Y.Z) reachable from HEAD. pyproject.toml no longer carries a
|
||||
# static version (#526 switched it to hatch-vcs), so the release tag
|
||||
# is the source of truth. `.devN`/`.rcN`/`desktop-*` tags are excluded
|
||||
# so they can't be mistaken for the release base.
|
||||
# Any future manual `X.Y.Z` release will outrank every `X.Y.Z.devN`
|
||||
# autotag — PEP 440 sorts dev releases strictly below the final.
|
||||
BASE=$(grep -E '^version = "' pyproject.toml | head -1 | sed -E 's/^version = "([^"]+)"/\1/')
|
||||
if [[ -z "$BASE" ]]; then
|
||||
echo "::error::Could not parse version from pyproject.toml"
|
||||
LATEST_RELEASE=$(git tag --list 'v[0-9]*' --merged HEAD \
|
||||
| grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -1)
|
||||
if [[ -z "$LATEST_RELEASE" ]]; then
|
||||
echo "::error::No release tag (vX.Y.Z) reachable from HEAD"
|
||||
exit 1
|
||||
fi
|
||||
BASE="${LATEST_RELEASE#v}"
|
||||
MAJOR=$(echo "$BASE" | cut -d. -f1)
|
||||
MINOR=$(echo "$BASE" | cut -d. -f2)
|
||||
PATCH=$(echo "$BASE" | cut -d. -f3 | sed -E 's/[^0-9].*$//')
|
||||
|
||||
@@ -11,22 +11,33 @@ concurrency:
|
||||
group: claude-issues-${{ github.event.issue.number || github.run_id }}
|
||||
cancel-in-progress: true
|
||||
|
||||
# Least-privilege: only what the issue-fixer job actually needs.
|
||||
# id-token (OIDC) is intentionally omitted — claude-code-action@v1 is passed
|
||||
# github_token directly, so OIDC is unused here.
|
||||
permissions:
|
||||
contents: write
|
||||
pull-requests: write
|
||||
issues: write
|
||||
id-token: write
|
||||
|
||||
jobs:
|
||||
fix:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 60
|
||||
timeout-minutes: 15
|
||||
# Security gate: this job reaches secrets.ANTHROPIC_API_KEY and holds a
|
||||
# write-scoped GITHUB_TOKEN. `issues` / `issue_comment` are public,
|
||||
# attacker-controllable events that run in the base-repo context with full
|
||||
# secret access, so the human-triggered paths are restricted to actors with
|
||||
# write-level association (OWNER / MEMBER / COLLABORATOR). This blocks
|
||||
# external / first-time contributors from draining the API budget or
|
||||
# creating branches/PRs, while leaving maintainer use unaffected.
|
||||
if: |
|
||||
github.event_name == 'workflow_dispatch' ||
|
||||
(github.event_name == 'issues' &&
|
||||
contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.issue.author_association) &&
|
||||
(contains(github.event.issue.labels.*.name, 'bug') ||
|
||||
contains(github.event.issue.labels.*.name, 'autofix'))) ||
|
||||
(github.event_name == 'issue_comment' &&
|
||||
contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association) &&
|
||||
!github.event.issue.pull_request &&
|
||||
contains(github.event.comment.body, '@claude') &&
|
||||
github.actor != 'claude[bot]')
|
||||
|
||||
@@ -11,23 +11,33 @@ concurrency:
|
||||
group: claude-review-${{ github.event.pull_request.number || github.event.issue.number || github.run_id }}
|
||||
cancel-in-progress: true
|
||||
|
||||
# Least-privilege: PR review only needs to post comments on the PR.
|
||||
# id-token (OIDC) is omitted — claude-code-action@v1 is passed github_token
|
||||
# directly, so OIDC is unused here.
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: write
|
||||
issues: write
|
||||
id-token: write
|
||||
|
||||
jobs:
|
||||
review:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 30
|
||||
# Security gate: this job reaches secrets.ANTHROPIC_API_KEY. Both
|
||||
# issue_comment and pull_request_review_comment are public,
|
||||
# attacker-controllable events that run in the base-repo context with full
|
||||
# secret access, so the @claude paths are restricted to actors with
|
||||
# write-level association (OWNER / MEMBER / COLLABORATOR). External /
|
||||
# first-time contributors cannot trigger the key; maintainers are unaffected.
|
||||
if: |
|
||||
github.event_name == 'workflow_dispatch' ||
|
||||
(github.event_name == 'issue_comment' &&
|
||||
contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association) &&
|
||||
github.event.issue.pull_request &&
|
||||
contains(github.event.comment.body, '@claude') &&
|
||||
github.actor != 'claude[bot]') ||
|
||||
(github.event_name == 'pull_request_review_comment' &&
|
||||
contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association) &&
|
||||
contains(github.event.comment.body, '@claude') &&
|
||||
github.actor != 'claude[bot]')
|
||||
steps:
|
||||
|
||||
@@ -114,6 +114,11 @@ jobs:
|
||||
|
||||
steps:
|
||||
- uses: actions/checkout@v6
|
||||
with:
|
||||
# Full history + tags so the workflow_dispatch fallback in
|
||||
# "Determine release info" can derive the dev version from the
|
||||
# latest release tag (#526).
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Install system dependencies (Linux)
|
||||
if: matrix.platform == 'ubuntu-22.04'
|
||||
@@ -183,7 +188,16 @@ jobs:
|
||||
# workflow_dispatch fallback (manual UI dispatch without --ref).
|
||||
# Derive a PEP 440 dev version aligned with autotag.yml so we
|
||||
# don't burn the X.Y.Z release-version namespace.
|
||||
BASE=$(grep -E '^version = "' pyproject.toml | head -1 | sed -E 's/^version = "([^"]+)"/\1/')
|
||||
# pyproject.toml no longer carries a static version (#526), so the
|
||||
# base comes from the latest plain release tag (vX.Y.Z), matching
|
||||
# autotag.yml. .dev/.rc/desktop-* tags are excluded.
|
||||
LATEST_RELEASE=$(git tag --list 'v[0-9]*' --merged HEAD \
|
||||
| grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -1)
|
||||
if [[ -z "$LATEST_RELEASE" ]]; then
|
||||
echo "::error::No release tag (vX.Y.Z) reachable from HEAD"
|
||||
exit 1
|
||||
fi
|
||||
BASE="${LATEST_RELEASE#v}"
|
||||
MAJOR=$(echo "$BASE" | cut -d. -f1)
|
||||
MINOR=$(echo "$BASE" | cut -d. -f2)
|
||||
PATCH=$(echo "$BASE" | cut -d. -f3 | sed -E 's/[^0-9].*$//')
|
||||
|
||||
@@ -12,6 +12,11 @@ on:
|
||||
description: 'Tag to publish (e.g. v1.0.2.dev500). Overrides github.ref.'
|
||||
required: false
|
||||
type: string
|
||||
dry_run:
|
||||
description: 'Dry run: build + validate, then publish to TestPyPI instead of PyPI (no production upload).'
|
||||
required: false
|
||||
default: false
|
||||
type: boolean
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
@@ -67,27 +72,41 @@ jobs:
|
||||
exit 1
|
||||
}
|
||||
|
||||
- name: Set version from tag
|
||||
- name: Resolve build version from tag
|
||||
env:
|
||||
REF: ${{ steps.ref.outputs.ref }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
# Strip leading "v" if present (e.g. v1.0.2.dev500 -> 1.0.2.dev500)
|
||||
# Strip leading "v" (e.g. v1.0.3.dev825 -> 1.0.3.dev825).
|
||||
VERSION="${REF#v}"
|
||||
if [[ -z "$VERSION" ]]; then
|
||||
echo "::error::Could not resolve version from ref '$REF'"
|
||||
if ! [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+ ]]; then
|
||||
echo "::error::ref '$REF' is not a version tag (expected vX.Y.Z[.devN]); pass -f tag=vX.Y.Z"
|
||||
exit 1
|
||||
fi
|
||||
sed -i "s/^version = \".*\"/version = \"${VERSION}\"/" pyproject.toml
|
||||
# Sanity check the substitution actually took
|
||||
grep -q "^version = \"${VERSION}\"" pyproject.toml || {
|
||||
echo "::error::sed failed to update pyproject.toml version"
|
||||
exit 1
|
||||
}
|
||||
echo "Building version $VERSION"
|
||||
# pyproject.toml is now dynamic = ["version"] via hatch-vcs (#526), so
|
||||
# there is no static line to sed. setuptools_scm cannot bump custom
|
||||
# `.devN` tags, so we pin the exact build version explicitly — the
|
||||
# published version always equals the pushed tag.
|
||||
echo "SETUPTOOLS_SCM_PRETEND_VERSION=${VERSION}" >> "$GITHUB_ENV"
|
||||
echo "Building version ${VERSION}"
|
||||
|
||||
- name: Build package
|
||||
run: uv build
|
||||
|
||||
- name: Publish to TestPyPI (dry run)
|
||||
if: ${{ inputs.dry_run }}
|
||||
env:
|
||||
UV_PUBLISH_TOKEN: ${{ secrets.TEST_PYPI_API_TOKEN }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
if [[ -z "${UV_PUBLISH_TOKEN:-}" ]]; then
|
||||
echo "::warning::TEST_PYPI_API_TOKEN is not set — skipping the TestPyPI upload."
|
||||
echo "Build + twine check passed, which validated version derivation and packaging end to end."
|
||||
echo "To exercise a real upload, add a TEST_PYPI_API_TOKEN secret (or a TestPyPI trusted publisher)."
|
||||
exit 0
|
||||
fi
|
||||
uv publish --publish-url https://test.pypi.org/legacy/
|
||||
|
||||
- name: Publish to PyPI
|
||||
if: ${{ !inputs.dry_run }}
|
||||
run: uv publish
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { getBase } from './api';
|
||||
import type { ConnectorInfo, SyncStatus, ConnectRequest } from '../types/connectors';
|
||||
import type { ConnectorInfo, SyncStatus, ConnectRequest, ConnectResponse } from '../types/connectors';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Connectors API
|
||||
@@ -18,16 +18,47 @@ export async function getConnector(id: string): Promise<ConnectorInfo> {
|
||||
return res.json();
|
||||
}
|
||||
|
||||
export async function connectSource(id: string, req: ConnectRequest): Promise<ConnectorInfo> {
|
||||
export async function connectSource(id: string, req: ConnectRequest): Promise<ConnectResponse> {
|
||||
const res = await fetch(`${getBase()}/v1/connectors/${encodeURIComponent(id)}/connect`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(req),
|
||||
});
|
||||
if (!res.ok) throw new Error(`Failed to connect ${id}: ${res.status}`);
|
||||
if (!res.ok) {
|
||||
// Surface the backend's actionable detail (e.g. malformed Client ID /
|
||||
// Secret) instead of a bare status code so the UI can render it.
|
||||
const err = await res.json().catch(() => ({ detail: res.statusText }));
|
||||
throw new Error(err.detail || `Failed to connect ${id}: ${res.status}`);
|
||||
}
|
||||
return res.json();
|
||||
}
|
||||
|
||||
/** Open the server-side OAuth consent flow in a popup and resolve once the
|
||||
* connector reports connected (or reject on timeout). Reused for any OAuth
|
||||
* connector whose /connect returned `oauth_required` (issue #512). */
|
||||
export function startServerOAuth(id: string, oauthStartPath?: string): Promise<void> {
|
||||
const path = oauthStartPath || `/v1/connectors/${encodeURIComponent(id)}/oauth/start`;
|
||||
window.open(`${getBase()}${path}`, '_blank', 'width=600,height=700');
|
||||
return new Promise((resolve, reject) => {
|
||||
const interval = setInterval(async () => {
|
||||
try {
|
||||
const info = await getConnector(id);
|
||||
if (info.connected) {
|
||||
clearInterval(interval);
|
||||
clearTimeout(timer);
|
||||
resolve();
|
||||
}
|
||||
} catch {
|
||||
// ignore transient polling errors
|
||||
}
|
||||
}, 2000);
|
||||
const timer = setTimeout(() => {
|
||||
clearInterval(interval);
|
||||
reject(new Error('Authorization timed out — please try again.'));
|
||||
}, 180000);
|
||||
});
|
||||
}
|
||||
|
||||
export async function disconnectSource(id: string): Promise<void> {
|
||||
const res = await fetch(`${getBase()}/v1/connectors/${encodeURIComponent(id)}/disconnect`, {
|
||||
method: 'POST',
|
||||
|
||||
@@ -24,7 +24,7 @@ import {
|
||||
import type { LucideIcon } from 'lucide-react';
|
||||
import { SOURCE_CATALOG } from '../types/connectors';
|
||||
import type { ConnectRequest } from '../types/connectors';
|
||||
import { listConnectors, connectSource, disconnectSource, getSyncStatus, triggerSync } from '../lib/connectors-api';
|
||||
import { listConnectors, connectSource, disconnectSource, getSyncStatus, triggerSync, startServerOAuth } from '../lib/connectors-api';
|
||||
import type { SyncStatus } from '../types/connectors';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -673,7 +673,19 @@ function DataSourcesSection() {
|
||||
setConnectStage('Connecting...');
|
||||
setConnectError('');
|
||||
try {
|
||||
await connectSource(id, req);
|
||||
const resp = await connectSource(id, req);
|
||||
|
||||
// OAuth connectors (Google Drive/Calendar/Contacts/Gmail/Tasks): pasting
|
||||
// a Client ID / Secret only registers the app credentials. The backend
|
||||
// returns `oauth_required` with the path to the in-process consent flow,
|
||||
// which is the only path that actually mints an access token. Open it now
|
||||
// and wait for the callback to flip the connector to connected. Without
|
||||
// this the connector would stay "pending" forever — the exact #512 bug.
|
||||
if (resp.status === 'oauth_required') {
|
||||
setConnectStage('Opening Google sign-in...');
|
||||
await startServerOAuth(id, resp.oauth_start);
|
||||
}
|
||||
|
||||
setConnectStage('Connected! Starting sync...');
|
||||
|
||||
// Wait for connector to show as connected
|
||||
|
||||
@@ -55,6 +55,19 @@ export interface ConnectRequest {
|
||||
password?: string;
|
||||
}
|
||||
|
||||
/** Response from POST /v1/connectors/{id}/connect.
|
||||
* For OAuth connectors, pasting a Client ID / Secret pair only registers the
|
||||
* app credentials; the backend returns `status: "oauth_required"` plus an
|
||||
* `oauth_start` path the UI must open to run the browser consent flow that
|
||||
* actually mints an access token (see issue #512). */
|
||||
export interface ConnectResponse {
|
||||
connector_id: string;
|
||||
connected: boolean;
|
||||
status: "connected" | "pending" | "oauth_required" | "disconnected";
|
||||
oauth_start?: string;
|
||||
sync_status?: string | null;
|
||||
}
|
||||
|
||||
export type WizardStep = "pick" | "connect" | "ingest" | "ready";
|
||||
|
||||
// Backward-compatible alias
|
||||
@@ -257,12 +270,12 @@ export const SOURCE_CATALOG: ConnectorMeta[] = [
|
||||
urlLabel: 'Enable Drive API',
|
||||
},
|
||||
{
|
||||
label: 'Create OAuth credentials: go to Credentials (link below) → click "+ Create Credentials" → choose "OAuth client ID" → Application type: "Desktop app" → click "Create"',
|
||||
label: 'Create OAuth credentials: go to Credentials (link below) → click "+ Create Credentials" → choose "OAuth client ID" → Application type: "Web application". Under "Authorized redirect URIs" add this server\'s callback (e.g. http://localhost:1313/v1/connectors/gdrive/oauth/callback — match the host/port your OpenJarvis server is bound to) → click "Create".',
|
||||
url: 'https://console.cloud.google.com/apis/credentials',
|
||||
urlLabel: 'Open Credentials',
|
||||
},
|
||||
{
|
||||
label: 'A dialog will show your Client ID and Client Secret. Copy both and paste them below. (If you miss it, click the download icon next to your OAuth client to see them again)',
|
||||
label: 'A dialog will show your Client ID and Client Secret. Copy both and paste them below, then click Connect — a Google sign-in window opens to finish authorization. (If you miss the dialog, click the download icon next to your OAuth client to see them again.)',
|
||||
},
|
||||
],
|
||||
inputFields: [
|
||||
|
||||
+26
-2
@@ -1,10 +1,10 @@
|
||||
[build-system]
|
||||
requires = ["hatchling"]
|
||||
requires = ["hatchling", "hatch-vcs"]
|
||||
build-backend = "hatchling.build"
|
||||
|
||||
[project]
|
||||
name = "OpenJarvis"
|
||||
version = "1.0.2"
|
||||
dynamic = ["version"]
|
||||
description = "OpenJarvis — modular AI assistant backend with composable intelligence primitives"
|
||||
readme = "README.md"
|
||||
# Upper bound: numpy 2.2.x (pinned transitively via datasets/pandas) ships no
|
||||
@@ -154,6 +154,30 @@ Issues = "https://github.com/open-jarvis/OpenJarvis/issues"
|
||||
jarvis = "openjarvis.cli:main"
|
||||
openjarvis-eval = "openjarvis.evals.cli:main"
|
||||
|
||||
# Version is derived from git tags by hatch-vcs (see #526). For source/editable
|
||||
# checkouts this yields the true `git describe` version (e.g. 1.0.3.dev109+g<sha>)
|
||||
# rather than a stale static string. CI release builds override this with
|
||||
# SETUPTOOLS_SCM_PRETEND_VERSION so the published version equals the pushed tag.
|
||||
#
|
||||
# setuptools_scm cannot bump custom `.devN` tags (only `.dev0`), so the autotag
|
||||
# `vX.Y.Z.devN` tags are deliberately EXCLUDED from version derivation here; the
|
||||
# base is taken from the latest plain release tag (vX.Y.Z) and the dev distance
|
||||
# is computed from commit count since that release.
|
||||
[tool.hatch.version]
|
||||
source = "vcs"
|
||||
|
||||
[tool.hatch.version.raw-options]
|
||||
tag_regex = '^v(?P<version>[0-9]+\.[0-9]+\.[0-9]+)$'
|
||||
git_describe_command = [
|
||||
"git", "describe", "--dirty", "--tags", "--long",
|
||||
"--match", "v[0-9]*", "--exclude", "*dev*", "--exclude", "*rc*", "--exclude", "desktop-*",
|
||||
]
|
||||
# Builds without a git checkout (e.g. the `COPY src/ src/` Docker stages, which
|
||||
# never include .git) can't run `git describe`. Without a fallback that would
|
||||
# hard-fail the build. Mirror the runtime sentinel in src/openjarvis/__init__.py.
|
||||
# Such builds can inject the real version via SETUPTOOLS_SCM_PRETEND_VERSION.
|
||||
fallback_version = "0.0.0+unknown"
|
||||
|
||||
[tool.hatch.build.targets.wheel]
|
||||
packages = ["src/openjarvis"]
|
||||
|
||||
|
||||
@@ -20,7 +20,6 @@ from openjarvis.connectors.oauth import (
|
||||
delete_tokens,
|
||||
load_tokens,
|
||||
resolve_google_credentials,
|
||||
run_oauth_flow,
|
||||
save_tokens,
|
||||
)
|
||||
from openjarvis.core.config import DEFAULT_CONFIG_DIR
|
||||
@@ -290,12 +289,18 @@ class GCalendarConnector(BaseConnector):
|
||||
"""Handle the OAuth callback.
|
||||
|
||||
If *code* looks like a ``client_id:client_secret`` pair (containing
|
||||
``.apps.googleusercontent.com``), store the credentials and trigger
|
||||
the full browser-based OAuth flow. Otherwise treat it as a raw
|
||||
token / auth code.
|
||||
``.apps.googleusercontent.com``), persist the client credentials only.
|
||||
The browser consent + code→token exchange is owned by the in-process
|
||||
server flow (``/v1/connectors/{id}/oauth/start`` → ``/oauth/callback``),
|
||||
which writes the real ``access_token`` to every Google credential file.
|
||||
|
||||
The previous daemon-thread browser flow (its own ``localhost:8789``
|
||||
callback server) failed silently in the bundled desktop context and is
|
||||
intentionally removed here (issue #512).
|
||||
|
||||
Any other *code* is treated as a raw token / auth code.
|
||||
"""
|
||||
code = code.strip()
|
||||
# If user pastes client_id:client_secret, store and run OAuth flow
|
||||
if ":" in code and ".apps.googleusercontent.com" in code:
|
||||
client_id, client_secret = code.split(":", 1)
|
||||
save_tokens(
|
||||
@@ -305,20 +310,6 @@ class GCalendarConnector(BaseConnector):
|
||||
"client_secret": client_secret.strip(),
|
||||
},
|
||||
)
|
||||
import threading
|
||||
|
||||
def _run() -> None:
|
||||
try:
|
||||
run_oauth_flow(
|
||||
client_id=client_id.strip(),
|
||||
client_secret=client_secret.strip(),
|
||||
scopes=GOOGLE_ALL_SCOPES,
|
||||
credentials_path=self._credentials_path,
|
||||
)
|
||||
except Exception: # noqa: BLE001
|
||||
pass
|
||||
|
||||
threading.Thread(target=_run, daemon=True).start()
|
||||
else:
|
||||
# Raw token or auth code
|
||||
save_tokens(self._credentials_path, {"token": code})
|
||||
|
||||
@@ -20,7 +20,6 @@ from openjarvis.connectors.oauth import (
|
||||
delete_tokens,
|
||||
load_tokens,
|
||||
resolve_google_credentials,
|
||||
run_oauth_flow,
|
||||
save_tokens,
|
||||
)
|
||||
from openjarvis.core.config import DEFAULT_CONFIG_DIR
|
||||
@@ -195,12 +194,18 @@ class GContactsConnector(BaseConnector):
|
||||
"""Handle the OAuth callback.
|
||||
|
||||
If *code* looks like a ``client_id:client_secret`` pair (containing
|
||||
``.apps.googleusercontent.com``), store the credentials and trigger
|
||||
the full browser-based OAuth flow. Otherwise treat it as a raw
|
||||
token / auth code.
|
||||
``.apps.googleusercontent.com``), persist the client credentials only.
|
||||
The browser consent + code→token exchange is owned by the in-process
|
||||
server flow (``/v1/connectors/{id}/oauth/start`` → ``/oauth/callback``),
|
||||
which writes the real ``access_token`` to every Google credential file.
|
||||
|
||||
The previous daemon-thread browser flow (its own ``localhost:8789``
|
||||
callback server) failed silently in the bundled desktop context and is
|
||||
intentionally removed here (issue #512).
|
||||
|
||||
Any other *code* is treated as a raw token / auth code.
|
||||
"""
|
||||
code = code.strip()
|
||||
# If user pastes client_id:client_secret, store and run OAuth flow
|
||||
if ":" in code and ".apps.googleusercontent.com" in code:
|
||||
client_id, client_secret = code.split(":", 1)
|
||||
save_tokens(
|
||||
@@ -210,20 +215,6 @@ class GContactsConnector(BaseConnector):
|
||||
"client_secret": client_secret.strip(),
|
||||
},
|
||||
)
|
||||
import threading
|
||||
|
||||
def _run() -> None:
|
||||
try:
|
||||
run_oauth_flow(
|
||||
client_id=client_id.strip(),
|
||||
client_secret=client_secret.strip(),
|
||||
scopes=GOOGLE_ALL_SCOPES,
|
||||
credentials_path=self._credentials_path,
|
||||
)
|
||||
except Exception: # noqa: BLE001
|
||||
pass
|
||||
|
||||
threading.Thread(target=_run, daemon=True).start()
|
||||
else:
|
||||
# Raw token or auth code
|
||||
save_tokens(self._credentials_path, {"token": code})
|
||||
|
||||
@@ -20,7 +20,6 @@ from openjarvis.connectors.oauth import (
|
||||
delete_tokens,
|
||||
load_tokens,
|
||||
resolve_google_credentials,
|
||||
run_oauth_flow,
|
||||
save_tokens,
|
||||
)
|
||||
from openjarvis.core.config import DEFAULT_CONFIG_DIR
|
||||
@@ -178,15 +177,25 @@ class GDriveConnector(BaseConnector):
|
||||
"""Handle the OAuth callback.
|
||||
|
||||
If *code* looks like a ``client_id:client_secret`` pair (containing
|
||||
``.apps.googleusercontent.com``), store the credentials and trigger
|
||||
the full browser-based OAuth flow. Otherwise treat it as a raw
|
||||
token / auth code.
|
||||
``.apps.googleusercontent.com``), persist the client credentials only.
|
||||
The actual browser consent + code→token exchange is owned by the
|
||||
in-process server flow (``/v1/connectors/{id}/oauth/start`` →
|
||||
``/oauth/callback``), which writes the real ``access_token`` to every
|
||||
Google credential file.
|
||||
|
||||
Previously this spawned a daemon thread that popped a browser and ran
|
||||
its own ``localhost:8789`` callback server; that thread failed silently
|
||||
in the bundled desktop context, so the connector never gained an access
|
||||
token and never appeared in Data Sources (issue #512). The background
|
||||
flow is intentionally removed here.
|
||||
|
||||
Any other *code* is treated as a raw token / auth code.
|
||||
"""
|
||||
code = code.strip()
|
||||
# If user pastes client_id:client_secret, store and run OAuth flow
|
||||
# A pasted client_id:client_secret pair is the app registration, not a
|
||||
# completed credential — persist it and let the server flow finish auth.
|
||||
if ":" in code and ".apps.googleusercontent.com" in code:
|
||||
client_id, client_secret = code.split(":", 1)
|
||||
# Save credentials immediately
|
||||
save_tokens(
|
||||
self._credentials_path,
|
||||
{
|
||||
@@ -194,21 +203,6 @@ class GDriveConnector(BaseConnector):
|
||||
"client_secret": client_secret.strip(),
|
||||
},
|
||||
)
|
||||
# Run OAuth flow in background thread to avoid blocking
|
||||
import threading
|
||||
|
||||
def _run() -> None:
|
||||
try:
|
||||
run_oauth_flow(
|
||||
client_id=client_id.strip(),
|
||||
client_secret=client_secret.strip(),
|
||||
scopes=GOOGLE_ALL_SCOPES,
|
||||
credentials_path=self._credentials_path,
|
||||
)
|
||||
except Exception: # noqa: BLE001
|
||||
pass
|
||||
|
||||
threading.Thread(target=_run, daemon=True).start()
|
||||
else:
|
||||
# Raw token or auth code
|
||||
save_tokens(self._credentials_path, {"token": code})
|
||||
|
||||
@@ -946,8 +946,10 @@ class AgentConfig:
|
||||
system_prompt_path: str = "" # path to system prompt file (.txt, .md)
|
||||
context_from_memory: bool = True # inject relevant memory context into prompts
|
||||
default_system_prompt: str = (
|
||||
"You are a helpful AI assistant running locally on the user's own "
|
||||
"hardware through OpenJarvis. You are not a cloud service. Respond "
|
||||
"You are OpenJarvis, a helpful AI assistant running locally on the "
|
||||
"user's own hardware. You are not a cloud service, and you are not "
|
||||
"Claude, ChatGPT, Gemini, or any other branded assistant. If asked "
|
||||
"who or what you are, identify yourself as OpenJarvis. Respond "
|
||||
"helpfully, concisely, and accurately."
|
||||
)
|
||||
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
"""Cloud inference engine — OpenAI, Anthropic, Google, and MiniMax API backends."""
|
||||
"""Cloud inference engine.
|
||||
|
||||
OpenAI, Anthropic, Google, MiniMax, and DeepSeek API backends.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -48,6 +51,8 @@ PRICING: Dict[str, tuple[float, float]] = {
|
||||
"MiniMax-M2.7-highspeed": (0.60, 2.40),
|
||||
"MiniMax-M2.5": (0.30, 1.20),
|
||||
"MiniMax-M2.5-highspeed": (0.60, 2.40),
|
||||
"deepseek-v4-flash": (0.27, 1.10),
|
||||
"deepseek-v4-pro": (0.55, 2.19),
|
||||
}
|
||||
|
||||
# Well-known model IDs per provider
|
||||
@@ -83,6 +88,10 @@ _MINIMAX_MODELS = [
|
||||
"MiniMax-M2.5",
|
||||
"MiniMax-M2.5-highspeed",
|
||||
]
|
||||
_DEEPSEEK_MODELS = [
|
||||
"deepseek-v4-flash",
|
||||
"deepseek-v4-pro",
|
||||
]
|
||||
|
||||
# OpenRouter models — prefixed with "openrouter/" so they can be identified
|
||||
_OPENROUTER_POPULAR = [
|
||||
@@ -111,6 +120,10 @@ def _is_minimax_model(model: str) -> bool:
|
||||
return model.lower().startswith("minimax")
|
||||
|
||||
|
||||
def _is_deepseek_model(model: str) -> bool:
|
||||
return model.lower().startswith("deepseek")
|
||||
|
||||
|
||||
def _is_openrouter_model(model: str) -> bool:
|
||||
return model.startswith("openrouter/")
|
||||
|
||||
@@ -127,6 +140,35 @@ def _is_google_model(model: str) -> bool:
|
||||
return "gemini" in model.lower() and not _is_openrouter_model(model)
|
||||
|
||||
|
||||
# Positive prefix predicate for genuine OpenAI models. Kept in sync with
|
||||
# ``server/cloud_router.py:_OPENAI_PREFIXES`` so local-vs-cloud classification
|
||||
# agrees across the codebase. Used by ``_client_for_model``/``can_serve`` so the
|
||||
# cloud engine never claims it can serve an unrecognized (e.g. local Ollama)
|
||||
# model name just because an OpenAI key happens to be present (see #335).
|
||||
_OPENAI_PREFIXES = ("gpt-", "chatgpt-", "o1", "o3", "o4")
|
||||
|
||||
|
||||
def _is_openai_model(model: str) -> bool:
|
||||
"""True only for genuine OpenAI models (gpt-*, chatgpt-*, o1/o3/o4 series).
|
||||
|
||||
Defined positively so that an unrecognized model name (a local Ollama model
|
||||
like ``qwen3.5:0.8b``, or a typo) is NOT treated as an OpenAI model. This is
|
||||
the routing surface ``can_serve`` relies on; ``generate``/``stream`` keep
|
||||
their OpenAI fall-through so an explicitly-requested unknown cloud model
|
||||
still errors loudly at call time.
|
||||
|
||||
Caveat: a user may repoint the OpenAI client at an OpenAI-compatible server
|
||||
(vLLM/LM Studio) via ``OPENAI_BASE_URL`` and legitimately serve non-gpt
|
||||
names. That path is undocumented/untested in this engine; if it is added,
|
||||
this predicate (or ``_client_for_model``) should treat a configured custom
|
||||
base_url as "serves anything".
|
||||
"""
|
||||
m = model.lower()
|
||||
if m in (name.lower() for name in _OPENAI_MODELS):
|
||||
return True
|
||||
return m.startswith(_OPENAI_PREFIXES)
|
||||
|
||||
|
||||
def _is_openai_reasoning_model(model: str) -> bool:
|
||||
"""Check if model is an OpenAI reasoning model that restricts temperature."""
|
||||
m = model.lower()
|
||||
@@ -269,7 +311,7 @@ def _convert_tools_to_google(
|
||||
|
||||
@EngineRegistry.register("cloud")
|
||||
class CloudEngine(InferenceEngine):
|
||||
"""Cloud inference via OpenAI, Anthropic, Google, and MiniMax SDKs."""
|
||||
"""Cloud inference via OpenAI, Anthropic, Google, MiniMax, and DeepSeek SDKs."""
|
||||
|
||||
engine_id = "cloud"
|
||||
is_cloud = True
|
||||
@@ -280,6 +322,7 @@ class CloudEngine(InferenceEngine):
|
||||
self._google_client: Any = None
|
||||
self._openrouter_client: Any = None
|
||||
self._minimax_client: Any = None
|
||||
self._deepseek_client: Any = None
|
||||
self._codex_client: Any = None
|
||||
# Gemini thought_signatures: tool_call_id -> signature bytes
|
||||
self._thought_sigs: Dict[str, bytes] = {}
|
||||
@@ -332,6 +375,17 @@ class CloudEngine(InferenceEngine):
|
||||
)
|
||||
except ImportError:
|
||||
pass
|
||||
deepseek_key = os.environ.get("DEEPSEEK_API_KEY")
|
||||
if deepseek_key:
|
||||
try:
|
||||
import openai
|
||||
|
||||
self._deepseek_client = openai.OpenAI(
|
||||
base_url="https://api.deepseek.com/v1",
|
||||
api_key=deepseek_key,
|
||||
)
|
||||
except ImportError:
|
||||
pass
|
||||
# Codex — uses the OpenAI Responses API.
|
||||
# Supports both standard API keys (api.openai.com) and ChatGPT
|
||||
# OAuth tokens (chatgpt.com) via OPENAI_CODEX_BASE_URL override.
|
||||
@@ -985,6 +1039,56 @@ class CloudEngine(InferenceEngine):
|
||||
]
|
||||
return result
|
||||
|
||||
def _generate_deepseek(
|
||||
self,
|
||||
messages: Sequence[Message],
|
||||
*,
|
||||
model: str,
|
||||
temperature: float,
|
||||
max_tokens: int,
|
||||
**kwargs: Any,
|
||||
) -> Dict[str, Any]:
|
||||
if self._deepseek_client is None:
|
||||
raise EngineConnectionError(
|
||||
"DeepSeek client not available — set DEEPSEEK_API_KEY"
|
||||
)
|
||||
kwargs.pop("response_format", None)
|
||||
create_kwargs: Dict[str, Any] = {
|
||||
"model": model,
|
||||
"messages": messages_to_dicts(messages),
|
||||
"max_tokens": max_tokens,
|
||||
"temperature": temperature,
|
||||
}
|
||||
t0 = time.monotonic()
|
||||
resp = self._deepseek_client.chat.completions.create(**create_kwargs)
|
||||
elapsed = time.monotonic() - t0
|
||||
choice = resp.choices[0]
|
||||
usage = resp.usage
|
||||
prompt_tokens = usage.prompt_tokens if usage else 0
|
||||
completion_tokens = usage.completion_tokens if usage else 0
|
||||
result: Dict[str, Any] = {
|
||||
"content": choice.message.content or "",
|
||||
"usage": {
|
||||
"prompt_tokens": prompt_tokens,
|
||||
"completion_tokens": completion_tokens,
|
||||
"total_tokens": (usage.total_tokens if usage else 0),
|
||||
},
|
||||
"model": resp.model,
|
||||
"finish_reason": choice.finish_reason or "stop",
|
||||
"cost_usd": estimate_cost(model, prompt_tokens, completion_tokens),
|
||||
"ttft": elapsed,
|
||||
}
|
||||
if hasattr(choice.message, "tool_calls") and choice.message.tool_calls:
|
||||
result["tool_calls"] = [
|
||||
{
|
||||
"id": tc.id,
|
||||
"name": tc.function.name,
|
||||
"arguments": tc.function.arguments,
|
||||
}
|
||||
for tc in choice.message.tool_calls
|
||||
]
|
||||
return result
|
||||
|
||||
def generate(
|
||||
self,
|
||||
messages: Sequence[Message],
|
||||
@@ -1006,6 +1110,8 @@ class CloudEngine(InferenceEngine):
|
||||
return self._generate_openrouter(messages, **kw)
|
||||
if _is_minimax_model(model):
|
||||
return self._generate_minimax(messages, **kw)
|
||||
if _is_deepseek_model(model):
|
||||
return self._generate_deepseek(messages, **kw)
|
||||
if _is_anthropic_model(model):
|
||||
return self._generate_anthropic(messages, **kw)
|
||||
if _is_google_model(model):
|
||||
@@ -1036,6 +1142,9 @@ class CloudEngine(InferenceEngine):
|
||||
elif _is_minimax_model(model):
|
||||
async for token in self._stream_minimax(messages, **kw):
|
||||
yield token
|
||||
elif _is_deepseek_model(model):
|
||||
async for token in self._stream_deepseek(messages, **kw):
|
||||
yield token
|
||||
elif _is_anthropic_model(model):
|
||||
async for token in self._stream_anthropic(messages, **kw):
|
||||
yield token
|
||||
@@ -1254,6 +1363,30 @@ class CloudEngine(InferenceEngine):
|
||||
if delta and delta.content:
|
||||
yield delta.content
|
||||
|
||||
async def _stream_deepseek(
|
||||
self,
|
||||
messages: Sequence[Message],
|
||||
*,
|
||||
model: str,
|
||||
temperature: float,
|
||||
max_tokens: int,
|
||||
**kwargs: Any,
|
||||
) -> AsyncIterator[str]:
|
||||
if self._deepseek_client is None:
|
||||
raise EngineConnectionError("DeepSeek client not available")
|
||||
create_kwargs: Dict[str, Any] = {
|
||||
"model": model,
|
||||
"messages": messages_to_dicts(messages),
|
||||
"max_tokens": max_tokens,
|
||||
"temperature": temperature,
|
||||
"stream": True,
|
||||
}
|
||||
resp = self._deepseek_client.chat.completions.create(**create_kwargs)
|
||||
for chunk in resp:
|
||||
delta = chunk.choices[0].delta if chunk.choices else None
|
||||
if delta and delta.content:
|
||||
yield delta.content
|
||||
|
||||
# -- stream_full: rich streaming with tool_calls support ----------------
|
||||
|
||||
async def _stream_full_openai(
|
||||
@@ -1307,6 +1440,18 @@ class CloudEngine(InferenceEngine):
|
||||
"stream": True,
|
||||
**kwargs,
|
||||
}
|
||||
elif _is_deepseek_model(model):
|
||||
client = self._deepseek_client
|
||||
if client is None:
|
||||
raise EngineConnectionError("DeepSeek client not available")
|
||||
create_kwargs = {
|
||||
"model": model,
|
||||
"messages": messages_to_dicts(messages),
|
||||
"max_tokens": max_tokens,
|
||||
"temperature": temperature,
|
||||
"stream": True,
|
||||
**kwargs,
|
||||
}
|
||||
else:
|
||||
client = self._openai_client
|
||||
if client is None:
|
||||
@@ -1473,24 +1618,40 @@ class CloudEngine(InferenceEngine):
|
||||
models.extend(_OPENROUTER_POPULAR)
|
||||
if self._minimax_client is not None:
|
||||
models.extend(_MINIMAX_MODELS)
|
||||
if self._deepseek_client is not None:
|
||||
models.extend(_DEEPSEEK_MODELS)
|
||||
if self._codex_client is not None:
|
||||
models.extend(_CODEX_MODELS)
|
||||
return models
|
||||
|
||||
def _client_for_model(self, model: str) -> Any:
|
||||
"""Return the provider client ``generate``/``stream`` will dispatch to
|
||||
for *model* (mirrors the routing in those methods)."""
|
||||
for *model*, or ``None`` for a model this engine cannot route.
|
||||
|
||||
Mirrors the routing in ``generate``/``stream``, but is intentionally
|
||||
*stricter* on the OpenAI fall-through: only genuine OpenAI models map to
|
||||
the OpenAI client. Unrecognized names (e.g. a local Ollama model like
|
||||
``qwen3.5:0.8b``) return ``None`` so ``can_serve`` declines them and the
|
||||
cloud engine is not mis-selected as a fallback when the local engine is
|
||||
transiently down and any (even dummy) ``OPENAI_API_KEY`` is set (#335).
|
||||
``generate``/``stream`` keep their OpenAI fall-through, so an
|
||||
explicitly-requested unknown cloud model still fails loudly at call time.
|
||||
"""
|
||||
if _is_codex_model(model):
|
||||
return self._codex_client
|
||||
if _is_openrouter_model(model):
|
||||
return self._openrouter_client
|
||||
if _is_minimax_model(model):
|
||||
return self._minimax_client
|
||||
if _is_deepseek_model(model):
|
||||
return self._deepseek_client
|
||||
if _is_anthropic_model(model):
|
||||
return self._anthropic_client
|
||||
if _is_google_model(model):
|
||||
return self._google_client
|
||||
return self._openai_client
|
||||
if _is_openai_model(model):
|
||||
return self._openai_client
|
||||
return None
|
||||
|
||||
def can_serve(self, model: str) -> bool:
|
||||
"""Return ``True`` only if the provider client for *model* exists.
|
||||
@@ -1512,6 +1673,7 @@ class CloudEngine(InferenceEngine):
|
||||
or self._google_client is not None
|
||||
or self._openrouter_client is not None
|
||||
or self._minimax_client is not None
|
||||
or self._deepseek_client is not None
|
||||
or self._codex_client is not None
|
||||
)
|
||||
|
||||
|
||||
@@ -3,7 +3,23 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from typing import Any, Dict, Optional
|
||||
from typing import TYPE_CHECKING, Any, Dict, Optional
|
||||
|
||||
# ``Request`` must be importable at *module* scope so that FastAPI can resolve
|
||||
# the stringized ``request: Request`` annotations on the OAuth endpoints below.
|
||||
# Because this module uses ``from __future__ import annotations``, every
|
||||
# annotation is a string that FastAPI evaluates against the module globals; a
|
||||
# ``Request`` imported only inside ``create_connectors_router()`` is invisible
|
||||
# there, which makes FastAPI mistake ``request`` for a required *query* param
|
||||
# (HTTP 422 on /oauth/start) or inject ``None`` (AttributeError on
|
||||
# /oauth/callback). Keep this import at top level. See issue #512.
|
||||
if TYPE_CHECKING:
|
||||
from starlette.requests import Request
|
||||
else:
|
||||
try:
|
||||
from starlette.requests import Request
|
||||
except ImportError: # starlette ships with fastapi; absent only without it
|
||||
Request = Any # type: ignore[assignment,misc]
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -78,7 +94,7 @@ def create_connectors_router():
|
||||
this package.
|
||||
"""
|
||||
try:
|
||||
from fastapi import APIRouter, HTTPException, Request
|
||||
from fastapi import APIRouter, HTTPException
|
||||
except ImportError as exc:
|
||||
raise ImportError(
|
||||
"fastapi and pydantic are required for the connectors router"
|
||||
@@ -125,6 +141,69 @@ def create_connectors_router():
|
||||
"chunks": chunks,
|
||||
}
|
||||
|
||||
def _maybe_oauth_client_pair(
|
||||
connector_id: str, req: ConnectRequest
|
||||
) -> Optional[Dict[str, Any]]:
|
||||
"""Handle a pasted ``client_id:client_secret`` for an OAuth connector.
|
||||
|
||||
Returns an ``oauth_required`` directive (and persists the client
|
||||
credentials to every credential file for the provider) when *req*
|
||||
carries a Google ``client_id:client_secret`` pair, so the caller can
|
||||
return early instead of triggering the silent background OAuth flow.
|
||||
Returns ``None`` when there is no such pair (the caller then falls
|
||||
through to the normal ``handle_callback`` / token path).
|
||||
|
||||
Raises ``HTTPException(400)`` when the pair is present but malformed or
|
||||
the connector has no OAuth provider — per the silent-failure discipline
|
||||
in REVIEW.md, a bad credential surfaces an actionable error rather than
|
||||
a perpetual ``pending`` state.
|
||||
"""
|
||||
from openjarvis.connectors.oauth import (
|
||||
get_provider_for_connector,
|
||||
save_client_credentials,
|
||||
)
|
||||
|
||||
raw = (req.code or req.token or "").strip()
|
||||
# Only the client-registration pair routes through the server flow.
|
||||
# A raw access token (no ".apps.googleusercontent.com") is handled by
|
||||
# the connector's handle_callback unchanged.
|
||||
if ".apps.googleusercontent.com" not in raw or ":" not in raw:
|
||||
return None
|
||||
|
||||
provider = get_provider_for_connector(connector_id)
|
||||
if provider is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"No OAuth provider configured for '{connector_id}'",
|
||||
)
|
||||
|
||||
client_id, client_secret = raw.split(":", 1)
|
||||
client_id = client_id.strip()
|
||||
client_secret = client_secret.strip()
|
||||
if not client_id or not client_secret:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=(
|
||||
"Malformed credentials — expected 'CLIENT_ID:CLIENT_SECRET'. "
|
||||
f"Create an OAuth client at: {provider.setup_url}"
|
||||
),
|
||||
)
|
||||
|
||||
save_client_credentials(provider, client_id, client_secret)
|
||||
# Cached instances may have resolved a stale credentials path before
|
||||
# these client creds existed; drop them so /oauth/callback rebuilds
|
||||
# them against the freshly written files.
|
||||
for cid in provider.connector_ids:
|
||||
_instances.pop(cid, None)
|
||||
|
||||
return {
|
||||
"connector_id": connector_id,
|
||||
"connected": False,
|
||||
"status": "oauth_required",
|
||||
"oauth_start": f"/v1/connectors/{connector_id}/oauth/start",
|
||||
"sync_status": None,
|
||||
}
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Background-sync state tracking. Defined here (before the endpoints)
|
||||
# so that POST /connect can fire-and-forget into the same machinery
|
||||
@@ -317,6 +396,19 @@ def create_connectors_router():
|
||||
instance._connected = Path(req.path).is_dir()
|
||||
|
||||
elif auth_type == "oauth":
|
||||
# A pasted ``client_id:client_secret`` pair is NOT a completed
|
||||
# OAuth credential — it is the app registration. Persist it and
|
||||
# hand the UI a directive to run the in-process browser consent
|
||||
# flow (/oauth/start → /oauth/callback), which is the only path
|
||||
# that actually exchanges a code for an access_token. Previously
|
||||
# this routed into the connector's handle_callback, which spawned
|
||||
# a daemon thread that popped a browser + ran its own
|
||||
# localhost:8789 callback server; that thread fails silently in
|
||||
# the bundled desktop context, so the connector never became
|
||||
# connected and never appeared in Data Sources (issue #512).
|
||||
directive = _maybe_oauth_client_pair(connector_id, req)
|
||||
if directive is not None:
|
||||
return directive
|
||||
if req.code:
|
||||
instance.handle_callback(req.code)
|
||||
elif req.token:
|
||||
@@ -433,9 +525,9 @@ def create_connectors_router():
|
||||
@router.get("/{connector_id}/oauth/callback")
|
||||
async def oauth_callback(
|
||||
connector_id: str,
|
||||
request: Request,
|
||||
code: str = "",
|
||||
error: str = "",
|
||||
request: Request = None,
|
||||
):
|
||||
"""Handle OAuth callback from the provider."""
|
||||
from fastapi.responses import HTMLResponse
|
||||
|
||||
@@ -43,6 +43,51 @@ def _to_messages(chat_messages) -> list[Message]:
|
||||
return messages
|
||||
|
||||
|
||||
def _ensure_identity_prompt(messages: list[Message], app_config) -> list[Message]:
|
||||
"""Prepend OpenJarvis's identity system prompt when the client omits one.
|
||||
|
||||
The desktop UI's chat backend posts only user/assistant turns to
|
||||
``/v1/chat/completions`` (see ``frontend/.../Chat/InputArea.tsx``), so
|
||||
nothing grounds the model's identity. Without a system prompt the model
|
||||
answers from its training identity (e.g. "I'm Claude", "I am Qwen"),
|
||||
which is what #540 reported. The CLI paths inject this via
|
||||
``SystemPromptBuilder`` / ``BaseAgent``; the engine-direct server paths
|
||||
did not. This mirrors the agent fallback in ``agents/_stubs.py``.
|
||||
|
||||
If any message already carries a system role, the caller has supplied
|
||||
their own grounding and we leave the list untouched (no double-prompting).
|
||||
|
||||
Resolution of the identity text: ``app_config.agent.default_system_prompt``
|
||||
when a config is wired onto ``app.state``; otherwise fall back to
|
||||
``load_config()``. Config resolution is wrapped so a broken/missing
|
||||
config degrades to "no injection" rather than crashing the endpoint, but
|
||||
the failure is logged (per REVIEW.md — never silently swallow).
|
||||
"""
|
||||
if any(m.role == Role.SYSTEM for m in messages):
|
||||
return messages
|
||||
|
||||
prompt = ""
|
||||
try:
|
||||
if app_config is not None:
|
||||
prompt = app_config.agent.default_system_prompt or ""
|
||||
else:
|
||||
from openjarvis.core.config import load_config
|
||||
|
||||
prompt = load_config().agent.default_system_prompt or ""
|
||||
except Exception:
|
||||
logging.getLogger("openjarvis.server").debug(
|
||||
"Identity system prompt resolution failed; "
|
||||
"serving request without identity grounding",
|
||||
exc_info=True,
|
||||
)
|
||||
return messages
|
||||
|
||||
if not prompt:
|
||||
return messages
|
||||
|
||||
return [Message(role=Role.SYSTEM, content=prompt), *messages]
|
||||
|
||||
|
||||
@router.post("/v1/chat/completions")
|
||||
async def chat_completions(request_body: ChatCompletionRequest, request: Request):
|
||||
"""Handle chat completion requests (streaming and non-streaming)."""
|
||||
@@ -149,7 +194,7 @@ async def chat_completions(request_body: ChatCompletionRequest, request: Request
|
||||
# from the engine for true real-time output.
|
||||
if request_body.tools:
|
||||
return await _handle_stream_tools(
|
||||
engine, model, request_body, complexity_info
|
||||
engine, model, request_body, complexity_info, app_config=config
|
||||
)
|
||||
return await _handle_stream(
|
||||
engine,
|
||||
@@ -157,6 +202,7 @@ async def chat_completions(request_body: ChatCompletionRequest, request: Request
|
||||
request_body,
|
||||
complexity_info,
|
||||
trace_store=getattr(request.app.state, "trace_store", None),
|
||||
app_config=config,
|
||||
)
|
||||
|
||||
# Non-streaming: use agent if available, otherwise direct engine call.
|
||||
@@ -192,6 +238,7 @@ async def chat_completions(request_body: ChatCompletionRequest, request: Request
|
||||
request_body,
|
||||
bus=bus,
|
||||
complexity_info=complexity_info,
|
||||
app_config=config,
|
||||
)
|
||||
|
||||
|
||||
@@ -201,9 +248,11 @@ def _handle_direct(
|
||||
req: ChatCompletionRequest,
|
||||
bus=None,
|
||||
complexity_info=None,
|
||||
app_config=None,
|
||||
) -> ChatCompletionResponse:
|
||||
"""Direct engine call without agent."""
|
||||
messages = _to_messages(req.messages)
|
||||
messages = _ensure_identity_prompt(messages, app_config)
|
||||
kwargs: dict[str, Any] = {}
|
||||
if req.tools:
|
||||
kwargs["tools"] = req.tools
|
||||
@@ -380,6 +429,8 @@ async def _handle_stream_tools(
|
||||
model: str,
|
||||
req: ChatCompletionRequest,
|
||||
complexity_info=None,
|
||||
*,
|
||||
app_config=None,
|
||||
):
|
||||
"""Stream a raw OpenAI-compat function-calling response via SSE.
|
||||
|
||||
@@ -397,6 +448,7 @@ async def _handle_stream_tools(
|
||||
from openjarvis.server.cloud_router import is_cloud_model
|
||||
|
||||
messages = _to_messages(req.messages)
|
||||
messages = _ensure_identity_prompt(messages, app_config)
|
||||
chunk_id = f"chatcmpl-{uuid.uuid4().hex[:12]}"
|
||||
use_cloud = is_cloud_model(model)
|
||||
|
||||
@@ -491,6 +543,7 @@ async def _handle_stream(
|
||||
complexity_info=None,
|
||||
*,
|
||||
trace_store=None,
|
||||
app_config=None,
|
||||
):
|
||||
"""Stream response using SSE format.
|
||||
|
||||
@@ -509,6 +562,7 @@ async def _handle_stream(
|
||||
)
|
||||
|
||||
messages = _to_messages(req.messages)
|
||||
messages = _ensure_identity_prompt(messages, app_config)
|
||||
chunk_id = f"chatcmpl-{uuid.uuid4().hex[:12]}"
|
||||
|
||||
# Last user message — recorded as the trace query.
|
||||
|
||||
@@ -0,0 +1,273 @@
|
||||
"""Regression tests for the connectors-router OAuth flow (issue #512).
|
||||
|
||||
These tests reproduce the three coupled defects that prevented Google Drive
|
||||
(and its Google siblings) from ever completing OAuth and appearing in Data
|
||||
Sources, and assert the fixed behaviour:
|
||||
|
||||
(A/B) ``POST /connect`` with a pasted ``client_id:client_secret`` pair must
|
||||
persist the client credentials and return an ``oauth_required`` directive
|
||||
pointing at ``/oauth/start`` — NOT silently spawn a background browser
|
||||
thread and report a perpetual ``pending`` state.
|
||||
(C-1) ``GET /oauth/start`` must return a redirect to the provider's consent
|
||||
page (regression: HTTP 422 because ``request: Request`` was mis-bound as
|
||||
a query param under ``from __future__ import annotations`` + a local
|
||||
``Request`` import).
|
||||
(C-2) ``GET /oauth/callback`` must read ``request.base_url`` and exchange the
|
||||
code for tokens without crashing (regression: ``request`` defaulted to
|
||||
``None`` → ``AttributeError``), persisting the access token to every
|
||||
Google credential file and flipping ``is_connected()`` to True.
|
||||
|
||||
All tests are hermetic: the connectors directory, the shared Google
|
||||
credentials path, and every Google connector's default credentials path are
|
||||
redirected to ``tmp_path`` so the suite neither depends on nor pollutes
|
||||
``~/.openjarvis/connectors`` (a real source of spurious failures — see the
|
||||
verifier note on ``resolve_google_credentials`` silently substituting the
|
||||
shared file when the caller-supplied path does not yet exist on disk).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from typing import Any, Iterator
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
fastapi = pytest.importorskip("fastapi", reason="requires the 'server' extra")
|
||||
from fastapi import FastAPI # noqa: E402
|
||||
from fastapi.testclient import TestClient # noqa: E402
|
||||
|
||||
_CLIENT_PAIR = "myid-123.apps.googleusercontent.com:GOCSPX-secret"
|
||||
_CLIENT_ID = "myid-123.apps.googleusercontent.com"
|
||||
|
||||
_ALL_GOOGLE_FILES = (
|
||||
"google.json",
|
||||
"gdrive.json",
|
||||
"gcalendar.json",
|
||||
"gcontacts.json",
|
||||
"gmail.json",
|
||||
"google_tasks.json",
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def hermetic_connectors(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
|
||||
"""Redirect all Google credential paths into *tmp_path*.
|
||||
|
||||
Ensures connector instances created by the router's ``_get_or_create``
|
||||
resolve to the same directory the OAuth callback writes to, and that the
|
||||
test leaves ``~/.openjarvis`` untouched.
|
||||
|
||||
Why this is more than a one-line monkeypatch: the autouse registry-clear
|
||||
fixture causes ``_ensure_connectors_registered()`` to ``importlib.reload``
|
||||
each connector module on the first router call, which re-executes the
|
||||
module body. To survive that reload we patch ``DEFAULT_CONFIG_DIR`` at its
|
||||
*source* (``openjarvis.core.config``) — every connector re-derives
|
||||
``_DEFAULT_CREDENTIALS_PATH`` from it on reload, so the tmp dir sticks.
|
||||
We also pre-register + pre-reload the connectors inside the fixture so the
|
||||
reload happens while the patch is live, then reset module state on
|
||||
teardown so a later test that imports these modules fresh is unaffected.
|
||||
"""
|
||||
import importlib
|
||||
import sys
|
||||
|
||||
import openjarvis.connectors.oauth as oauth_mod
|
||||
import openjarvis.core.config as config_mod
|
||||
import openjarvis.server.connectors_router as router_mod
|
||||
from openjarvis.core.registry import ConnectorRegistry
|
||||
|
||||
conn_dir = tmp_path / "connectors"
|
||||
conn_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
monkeypatch.setattr(config_mod, "DEFAULT_CONFIG_DIR", tmp_path)
|
||||
monkeypatch.setattr(oauth_mod, "_CONNECTORS_DIR", conn_dir)
|
||||
monkeypatch.setattr(
|
||||
oauth_mod, "_SHARED_GOOGLE_CREDENTIALS_PATH", str(conn_dir / "google.json")
|
||||
)
|
||||
|
||||
# Force the connector modules to re-derive their default paths from the
|
||||
# patched DEFAULT_CONFIG_DIR now, before any request, and register them so
|
||||
# the router's lazy reload-on-empty-registry path is a no-op.
|
||||
google_mods = [
|
||||
"openjarvis.connectors.gdrive",
|
||||
"openjarvis.connectors.gcalendar",
|
||||
"openjarvis.connectors.gcontacts",
|
||||
"openjarvis.connectors.gmail",
|
||||
"openjarvis.connectors.google_tasks",
|
||||
]
|
||||
for name in google_mods:
|
||||
if name in sys.modules:
|
||||
importlib.reload(sys.modules[name])
|
||||
|
||||
router_mod._instances.clear()
|
||||
yield conn_dir
|
||||
router_mod._instances.clear()
|
||||
ConnectorRegistry.clear()
|
||||
# Restore the connector modules to their real (unpatched) default paths so
|
||||
# subsequent tests in the same process see ~/.openjarvis again.
|
||||
for name in google_mods:
|
||||
if name in sys.modules:
|
||||
importlib.reload(sys.modules[name])
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def client(hermetic_connectors: Path) -> Iterator[TestClient]:
|
||||
from openjarvis.server.connectors_router import create_connectors_router
|
||||
|
||||
app = FastAPI()
|
||||
app.include_router(create_connectors_router())
|
||||
with TestClient(app) as c:
|
||||
yield c
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Defect A/B — POST /connect must not silently spawn a background OAuth thread
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"connector_id", ["gdrive", "gcalendar", "gcontacts", "gmail", "google_tasks"]
|
||||
)
|
||||
def test_connect_client_pair_returns_oauth_required_no_browser(
|
||||
client: TestClient, hermetic_connectors: Path, connector_id: str
|
||||
) -> None:
|
||||
"""Pasting client_id:secret persists creds + asks the UI to run the flow.
|
||||
|
||||
Covers every Google connector that shares the OAuth provider, proving the
|
||||
sibling connectors are fixed too (not just gdrive).
|
||||
"""
|
||||
with patch("openjarvis.core.open_browser") as mock_browser:
|
||||
resp = client.post(
|
||||
f"/v1/connectors/{connector_id}/connect", json={"code": _CLIENT_PAIR}
|
||||
)
|
||||
|
||||
assert resp.status_code == 200, resp.text
|
||||
body = resp.json()
|
||||
assert body["status"] == "oauth_required"
|
||||
assert body["oauth_start"] == f"/v1/connectors/{connector_id}/oauth/start"
|
||||
assert body["connected"] is False
|
||||
# No fire-and-forget browser thread (the root cause of "nothing happens").
|
||||
mock_browser.assert_not_called()
|
||||
|
||||
# Client credentials persisted to EVERY Google credential file so a single
|
||||
# consent covers all Google connectors.
|
||||
for filename in _ALL_GOOGLE_FILES:
|
||||
path = hermetic_connectors / filename
|
||||
assert path.exists(), f"{filename} not written"
|
||||
assert json.loads(path.read_text())["client_id"] == _CLIENT_ID
|
||||
|
||||
|
||||
def test_connect_malformed_client_pair_raises_400(
|
||||
client: TestClient,
|
||||
) -> None:
|
||||
"""A blank secret surfaces an actionable 400 — not a silent pending state."""
|
||||
resp = client.post(
|
||||
"/v1/connectors/gdrive/connect",
|
||||
json={"code": "myid-123.apps.googleusercontent.com:"},
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert "CLIENT_ID:CLIENT_SECRET" in resp.json()["detail"]
|
||||
|
||||
|
||||
def test_connect_raw_token_still_handled(
|
||||
client: TestClient, hermetic_connectors: Path
|
||||
) -> None:
|
||||
"""A raw token (not a client pair) still flows through handle_callback."""
|
||||
resp = client.post(
|
||||
"/v1/connectors/gdrive/connect", json={"token": "ya29.raw-access-token"}
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
saved = json.loads((hermetic_connectors / "gdrive.json").read_text())
|
||||
assert saved.get("token") == "ya29.raw-access-token"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Defect C-1 — GET /oauth/start must redirect (was HTTP 422)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_oauth_start_redirects_to_consent(
|
||||
client: TestClient,
|
||||
) -> None:
|
||||
# First save client creds via the connect call.
|
||||
client.post("/v1/connectors/gdrive/connect", json={"code": _CLIENT_PAIR})
|
||||
|
||||
resp = client.get("/v1/connectors/gdrive/oauth/start", follow_redirects=False)
|
||||
# FastAPI's RedirectResponse defaults to 307; any 3xx is a pass (was 422).
|
||||
assert resp.status_code in (302, 307), resp.text
|
||||
location = resp.headers["location"]
|
||||
assert location.startswith("https://accounts.google.com/o/oauth2/v2/auth")
|
||||
assert _CLIENT_ID in location
|
||||
# redirect_uri must point back at OUR in-process callback.
|
||||
assert "oauth%2Fcallback" in location or "oauth/callback" in location
|
||||
|
||||
|
||||
def test_oauth_start_without_creds_returns_400(client: TestClient) -> None:
|
||||
resp = client.get("/v1/connectors/gdrive/oauth/start", follow_redirects=False)
|
||||
assert resp.status_code == 400
|
||||
assert "client credentials" in resp.json()["detail"].lower()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Defect C-2 — GET /oauth/callback must exchange + persist (was 500 on None)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_oauth_callback_exchanges_and_connects(
|
||||
client: TestClient, hermetic_connectors: Path
|
||||
) -> None:
|
||||
import openjarvis.connectors.oauth as oauth_mod
|
||||
|
||||
client.post("/v1/connectors/gdrive/connect", json={"code": _CLIENT_PAIR})
|
||||
|
||||
fake_tokens = {
|
||||
"access_token": "ya29.REAL",
|
||||
"refresh_token": "1//REAL",
|
||||
"token_type": "Bearer",
|
||||
"expires_in": 3600,
|
||||
}
|
||||
with patch.object(oauth_mod, "_exchange_token", return_value=fake_tokens) as ex:
|
||||
resp = client.get("/v1/connectors/gdrive/oauth/callback?code=authcode123")
|
||||
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert "Connected!" in resp.text
|
||||
ex.assert_called_once()
|
||||
|
||||
# Access token written to ALL Google credential files.
|
||||
for filename in _ALL_GOOGLE_FILES:
|
||||
saved = json.loads((hermetic_connectors / filename).read_text())
|
||||
assert saved["access_token"] == "ya29.REAL"
|
||||
assert saved["refresh_token"] == "1//REAL"
|
||||
|
||||
# The connector now reports connected, and GET /connectors agrees.
|
||||
from openjarvis.connectors.gdrive import GDriveConnector
|
||||
|
||||
assert GDriveConnector().is_connected() is True
|
||||
|
||||
listing = client.get("/v1/connectors").json()["connectors"]
|
||||
gdrive = next(c for c in listing if c["connector_id"] == "gdrive")
|
||||
assert gdrive["connected"] is True
|
||||
|
||||
|
||||
def test_oauth_callback_error_param_renders_failure(client: TestClient) -> None:
|
||||
resp = client.get("/v1/connectors/gdrive/oauth/callback?error=access_denied")
|
||||
assert resp.status_code == 400
|
||||
assert "access_denied" in resp.text
|
||||
|
||||
|
||||
def test_oauth_callback_exchange_failure_renders_error(
|
||||
client: TestClient,
|
||||
) -> None:
|
||||
import openjarvis.connectors.oauth as oauth_mod
|
||||
|
||||
client.post("/v1/connectors/gdrive/connect", json={"code": _CLIENT_PAIR})
|
||||
|
||||
def _boom(*_a: Any, **_k: Any) -> dict[str, Any]:
|
||||
raise RuntimeError("token endpoint 400")
|
||||
|
||||
with patch.object(oauth_mod, "_exchange_token", side_effect=_boom):
|
||||
resp = client.get("/v1/connectors/gdrive/oauth/callback?code=bad")
|
||||
|
||||
assert resp.status_code == 500
|
||||
assert "Token Exchange Failed" in resp.text
|
||||
@@ -30,19 +30,35 @@ def test_exchange_google_token_calls_endpoint() -> None:
|
||||
mock_post.assert_called_once()
|
||||
|
||||
|
||||
def test_gdrive_handle_callback_triggers_oauth(tmp_path: Path) -> None:
|
||||
def test_gdrive_handle_callback_persists_creds_no_background_flow(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""A pasted client pair persists creds ONLY — no silent browser thread.
|
||||
|
||||
Regression for issue #512: the previous implementation spawned a daemon
|
||||
thread that popped a browser and ran its own localhost:8789 callback
|
||||
server. That thread failed silently in the bundled desktop context, so the
|
||||
connector never gained an access token. ``handle_callback`` must now only
|
||||
save the client_id/secret; the in-process server flow owns the consent
|
||||
round-trip. We assert ``open_browser`` is never invoked.
|
||||
"""
|
||||
from openjarvis.connectors.gdrive import GDriveConnector
|
||||
from openjarvis.connectors.oauth import load_tokens
|
||||
|
||||
creds = str(tmp_path / "gdrive.json")
|
||||
conn = GDriveConnector(credentials_path=creds)
|
||||
|
||||
with patch("openjarvis.connectors.gdrive.run_oauth_flow") as mock_flow:
|
||||
mock_flow.return_value = {"access_token": "ya29.test"}
|
||||
with patch("openjarvis.core.open_browser") as mock_browser:
|
||||
conn.handle_callback("test-id.apps.googleusercontent.com:test-secret")
|
||||
|
||||
mock_flow.assert_called_once()
|
||||
call_kwargs = mock_flow.call_args
|
||||
assert "test-id.apps.googleusercontent.com" in str(call_kwargs)
|
||||
mock_browser.assert_not_called()
|
||||
tokens = load_tokens(creds)
|
||||
assert tokens is not None
|
||||
assert tokens["client_id"] == "test-id.apps.googleusercontent.com"
|
||||
assert tokens["client_secret"] == "test-secret"
|
||||
# No access token yet — that arrives via /oauth/callback.
|
||||
assert not tokens.get("access_token")
|
||||
assert conn.is_connected() is False
|
||||
|
||||
|
||||
def test_gdrive_is_connected_requires_access_token(tmp_path: Path) -> None:
|
||||
@@ -61,48 +77,46 @@ def test_gdrive_is_connected_requires_access_token(tmp_path: Path) -> None:
|
||||
assert conn.is_connected() is True
|
||||
|
||||
|
||||
def test_gcalendar_handle_callback_triggers_oauth(tmp_path: Path) -> None:
|
||||
def test_gcalendar_handle_callback_persists_creds_no_background_flow(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Sibling connector shares the fix: creds saved, no browser thread (#512)."""
|
||||
from openjarvis.connectors.gcalendar import GCalendarConnector
|
||||
from openjarvis.connectors.oauth import load_tokens
|
||||
|
||||
creds = str(tmp_path / "gcalendar.json")
|
||||
conn = GCalendarConnector(credentials_path=creds)
|
||||
|
||||
with patch("openjarvis.connectors.gcalendar.run_oauth_flow") as mock_flow:
|
||||
mock_flow.return_value = {"access_token": "ya29.test"}
|
||||
with patch("openjarvis.core.open_browser") as mock_browser:
|
||||
conn.handle_callback("test-id.apps.googleusercontent.com:test-secret")
|
||||
|
||||
mock_flow.assert_called_once()
|
||||
|
||||
|
||||
def test_gcontacts_handle_callback_triggers_oauth(tmp_path: Path) -> None:
|
||||
from openjarvis.connectors.gcontacts import GContactsConnector
|
||||
|
||||
creds = str(tmp_path / "gcontacts.json")
|
||||
conn = GContactsConnector(credentials_path=creds)
|
||||
|
||||
with patch("openjarvis.connectors.gcontacts.run_oauth_flow") as mock_flow:
|
||||
mock_flow.return_value = {"access_token": "ya29.test"}
|
||||
conn.handle_callback("test-id.apps.googleusercontent.com:test-secret")
|
||||
|
||||
mock_flow.assert_called_once()
|
||||
|
||||
|
||||
def test_gdrive_handle_callback_fallback_on_failure(tmp_path: Path) -> None:
|
||||
from openjarvis.connectors.gdrive import GDriveConnector
|
||||
from openjarvis.connectors.oauth import load_tokens
|
||||
|
||||
creds = str(tmp_path / "gdrive.json")
|
||||
conn = GDriveConnector(credentials_path=creds)
|
||||
|
||||
with patch("openjarvis.connectors.gdrive.run_oauth_flow") as mock_flow:
|
||||
mock_flow.side_effect = RuntimeError("OAuth failed")
|
||||
conn.handle_callback("test-id.apps.googleusercontent.com:test-secret")
|
||||
|
||||
# Should have saved client_id and client_secret as fallback
|
||||
mock_browser.assert_not_called()
|
||||
tokens = load_tokens(creds)
|
||||
assert tokens is not None
|
||||
assert tokens["client_id"] == "test-id.apps.googleusercontent.com"
|
||||
assert tokens["client_secret"] == "test-secret"
|
||||
assert conn.is_connected() is False
|
||||
|
||||
|
||||
def test_gcontacts_handle_callback_persists_creds_no_background_flow(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Sibling connector shares the fix: creds saved, no browser thread (#512)."""
|
||||
from openjarvis.connectors.gcontacts import GContactsConnector
|
||||
from openjarvis.connectors.oauth import load_tokens
|
||||
|
||||
creds = str(tmp_path / "gcontacts.json")
|
||||
conn = GContactsConnector(credentials_path=creds)
|
||||
|
||||
with patch("openjarvis.core.open_browser") as mock_browser:
|
||||
conn.handle_callback("test-id.apps.googleusercontent.com:test-secret")
|
||||
|
||||
mock_browser.assert_not_called()
|
||||
tokens = load_tokens(creds)
|
||||
assert tokens is not None
|
||||
assert tokens["client_id"] == "test-id.apps.googleusercontent.com"
|
||||
assert tokens["client_secret"] == "test-secret"
|
||||
assert conn.is_connected() is False
|
||||
|
||||
|
||||
def test_gdrive_handle_callback_raw_token(tmp_path: Path) -> None:
|
||||
|
||||
@@ -237,6 +237,14 @@ class TestAgentConfigNew:
|
||||
or isinstance(getattr(ac.__class__, "temperature", None), property) is False
|
||||
)
|
||||
|
||||
def test_default_system_prompt_anchors_identity(self) -> None:
|
||||
"""#540: the hardened wording must name OpenJarvis and explicitly
|
||||
deny the model's training identity so distilled models stop
|
||||
claiming to be Claude/ChatGPT/etc."""
|
||||
prompt = AgentConfig().default_system_prompt
|
||||
assert "OpenJarvis" in prompt
|
||||
assert "not Claude" in prompt
|
||||
|
||||
|
||||
class TestNestedEngineConfig:
|
||||
def test_nested_access(self) -> None:
|
||||
@@ -561,6 +569,7 @@ class TestWhatsAppBaileysChannelConfig:
|
||||
|
||||
def test_mining_config_absent_means_none(tmp_path):
|
||||
from openjarvis.core.config import load_config
|
||||
|
||||
cfg_path = tmp_path / "config.toml"
|
||||
cfg_path.write_text("") # empty config
|
||||
cfg = load_config(cfg_path)
|
||||
|
||||
@@ -9,9 +9,13 @@ import pytest
|
||||
|
||||
from openjarvis.core.registry import EngineRegistry
|
||||
from openjarvis.core.types import Message, Role
|
||||
from openjarvis.engine._base import EngineConnectionError
|
||||
from openjarvis.engine.cloud import (
|
||||
CloudEngine,
|
||||
_is_codex_model,
|
||||
_is_deepseek_model,
|
||||
_is_openai_model,
|
||||
_is_openrouter_model,
|
||||
estimate_cost,
|
||||
)
|
||||
|
||||
@@ -457,6 +461,7 @@ class TestCloudEngineCanServe:
|
||||
"_google_client",
|
||||
"_openrouter_client",
|
||||
"_minimax_client",
|
||||
"_deepseek_client",
|
||||
"_codex_client",
|
||||
):
|
||||
setattr(eng, name, clients.get(name))
|
||||
@@ -469,7 +474,154 @@ class TestCloudEngineCanServe:
|
||||
assert eng.can_serve("gemini-2.5-pro") is False
|
||||
assert eng.can_serve("openrouter/openai/gpt-4o") is False
|
||||
|
||||
def test_openai_key_does_not_claim_local_models(self) -> None:
|
||||
"""#335: with only the OpenAI client set (e.g. a present-but-dummy
|
||||
OPENAI_API_KEY), the cloud engine must NOT claim it can serve a local
|
||||
Ollama model name — otherwise it gets mis-selected as a fallback when
|
||||
the local engine is transiently down and dies with "OpenAI client not
|
||||
available". Only genuine OpenAI models route to the OpenAI client.
|
||||
"""
|
||||
eng = self._engine(_openai_client=object())
|
||||
# Local Ollama / unrecognized names are NOT served by the cloud engine.
|
||||
assert eng.can_serve("qwen3.5:0.8b") is False
|
||||
assert eng.can_serve("llama3.2") is False
|
||||
assert eng.can_serve("mistral") is False
|
||||
assert eng.can_serve("phi3:mini") is False
|
||||
assert eng.can_serve("some-unknown-model") is False
|
||||
# Genuine OpenAI families still served.
|
||||
assert eng.can_serve("gpt-4o") is True
|
||||
assert eng.can_serve("gpt-5.4") is True
|
||||
assert eng.can_serve("o3-mini") is True
|
||||
|
||||
def test_unknown_model_not_served_even_with_all_clients(self) -> None:
|
||||
"""#335: an unrecognized model is declined regardless of how many
|
||||
provider clients are configured — it never falls through to OpenAI."""
|
||||
eng = self._engine(
|
||||
_openai_client=object(),
|
||||
_anthropic_client=object(),
|
||||
_google_client=object(),
|
||||
_minimax_client=object(),
|
||||
_deepseek_client=object(),
|
||||
)
|
||||
assert eng.can_serve("qwen3.5:0.8b") is False
|
||||
assert eng.can_serve("totally-made-up") is False
|
||||
|
||||
def test_anthropic_only_serves_anthropic_models(self) -> None:
|
||||
eng = self._engine(_anthropic_client=object())
|
||||
assert eng.can_serve("claude-sonnet-4") is True
|
||||
assert eng.can_serve("gpt-4o") is False
|
||||
|
||||
def test_deepseek_only_serves_deepseek_models(self) -> None:
|
||||
"""The DeepSeek client serves deepseek-* models (and only those)."""
|
||||
eng = self._engine(_deepseek_client=object())
|
||||
assert eng.can_serve("deepseek-v4-flash") is True
|
||||
assert eng.can_serve("deepseek-v4-pro") is True
|
||||
assert eng.can_serve("DeepSeek-V4-Pro") is True # case-insensitive
|
||||
assert eng.can_serve("gpt-4o") is False
|
||||
# OpenRouter-prefixed deepseek is NOT the direct DeepSeek provider.
|
||||
assert eng.can_serve("openrouter/deepseek/deepseek-r1") is False
|
||||
|
||||
|
||||
class TestCloudEngineDeepSeek:
|
||||
"""PR #504: DeepSeek as a first-class cloud provider (OpenAI-compatible)."""
|
||||
|
||||
def test_is_deepseek_model_predicate(self) -> None:
|
||||
assert _is_deepseek_model("deepseek-v4-flash") is True
|
||||
assert _is_deepseek_model("deepseek-v4-pro") is True
|
||||
assert _is_deepseek_model("DeepSeek-V4-Pro") is True # case-insensitive
|
||||
assert _is_deepseek_model("gpt-4o") is False
|
||||
# No predicate collision: openrouter/deepseek/* belongs to OpenRouter.
|
||||
assert _is_deepseek_model("openrouter/deepseek/deepseek-r1") is False
|
||||
assert _is_openrouter_model("openrouter/deepseek/deepseek-r1") is True
|
||||
# And a deepseek name is not mistaken for an OpenAI model.
|
||||
assert _is_openai_model("deepseek-v4-pro") is False
|
||||
|
||||
def test_pricing_entries_present(self) -> None:
|
||||
assert estimate_cost("deepseek-v4-flash", 1_000_000, 1_000_000) == (
|
||||
pytest.approx(1.37) # 0.27 + 1.10
|
||||
)
|
||||
assert estimate_cost("deepseek-v4-pro", 1_000_000, 1_000_000) == (
|
||||
pytest.approx(2.74) # 0.55 + 2.19
|
||||
)
|
||||
|
||||
def test_init_wires_deepseek_client(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""DEEPSEEK_API_KEY builds an openai client pointed at api.deepseek.com."""
|
||||
for var in ("OPENAI_API_KEY", "ANTHROPIC_API_KEY"):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
monkeypatch.setenv("DEEPSEEK_API_KEY", "sk-deepseek-test")
|
||||
|
||||
fake_openai = mock.MagicMock()
|
||||
with mock.patch.dict("sys.modules", {"openai": fake_openai}):
|
||||
EngineRegistry.register_value("cloud", CloudEngine)
|
||||
engine = CloudEngine()
|
||||
|
||||
fake_openai.OpenAI.assert_any_call(
|
||||
base_url="https://api.deepseek.com/v1",
|
||||
api_key="sk-deepseek-test",
|
||||
)
|
||||
assert engine._deepseek_client is not None
|
||||
|
||||
def test_health_and_list_models_gated_on_deepseek_key(
|
||||
self, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
for var in ("OPENAI_API_KEY", "ANTHROPIC_API_KEY"):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
monkeypatch.setenv("DEEPSEEK_API_KEY", "sk-deepseek-test")
|
||||
|
||||
fake_openai = mock.MagicMock()
|
||||
with mock.patch.dict("sys.modules", {"openai": fake_openai}):
|
||||
EngineRegistry.register_value("cloud", CloudEngine)
|
||||
engine = CloudEngine()
|
||||
|
||||
assert engine.health() is True
|
||||
models = engine.list_models()
|
||||
assert "deepseek-v4-flash" in models
|
||||
assert "deepseek-v4-pro" in models
|
||||
# can_serve must agree with list_models (regression for the missing
|
||||
# _client_for_model deepseek branch flagged by the #504 verifier).
|
||||
assert engine.can_serve("deepseek-v4-pro") is True
|
||||
assert engine.can_serve("deepseek-v4-flash") is True
|
||||
|
||||
def test_generate_routes_to_deepseek_client(
|
||||
self, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
for var in ("OPENAI_API_KEY", "ANTHROPIC_API_KEY", "DEEPSEEK_API_KEY"):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
|
||||
fake_usage = SimpleNamespace(
|
||||
prompt_tokens=7, completion_tokens=3, total_tokens=10
|
||||
)
|
||||
fake_choice = SimpleNamespace(
|
||||
message=SimpleNamespace(content="ds-hello"),
|
||||
finish_reason="stop",
|
||||
)
|
||||
fake_resp = SimpleNamespace(
|
||||
choices=[fake_choice], usage=fake_usage, model="deepseek-v4-pro"
|
||||
)
|
||||
fake_client = mock.MagicMock()
|
||||
fake_client.chat.completions.create.return_value = fake_resp
|
||||
|
||||
EngineRegistry.register_value("cloud", CloudEngine)
|
||||
engine = CloudEngine()
|
||||
engine._deepseek_client = fake_client
|
||||
|
||||
result = engine.generate(
|
||||
[Message(role=Role.USER, content="Hi")], model="deepseek-v4-pro"
|
||||
)
|
||||
assert result["content"] == "ds-hello"
|
||||
assert result["usage"]["prompt_tokens"] == 7
|
||||
# Routed to the DeepSeek client, not OpenAI.
|
||||
fake_client.chat.completions.create.assert_called_once()
|
||||
|
||||
def test_generate_without_client_raises(
|
||||
self, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
for var in ("OPENAI_API_KEY", "ANTHROPIC_API_KEY", "DEEPSEEK_API_KEY"):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
EngineRegistry.register_value("cloud", CloudEngine)
|
||||
engine = CloudEngine()
|
||||
assert engine._deepseek_client is None
|
||||
with pytest.raises(EngineConnectionError):
|
||||
engine.generate(
|
||||
[Message(role=Role.USER, content="Hi")], model="deepseek-v4-pro"
|
||||
)
|
||||
|
||||
@@ -204,6 +204,54 @@ class TestGetEngine:
|
||||
assert result is not None
|
||||
assert result[0] == "local"
|
||||
|
||||
def test_dummy_openai_key_does_not_misroute_local_model(
|
||||
self, monkeypatch: object
|
||||
) -> None:
|
||||
"""#335: a present-but-dummy OPENAI_API_KEY + a down local engine must
|
||||
NOT cause a local Ollama model to be routed to the cloud engine.
|
||||
|
||||
Before the fix, CloudEngine.can_serve('qwen3.5:0.8b') returned True
|
||||
whenever any OpenAI client existed (even a junk key), so get_engine
|
||||
picked 'cloud' and the request later died with "OpenAI client not
|
||||
available". With the strict _client_for_model fall-through it returns
|
||||
None for unrecognized names, so get_engine declines cloud and (with the
|
||||
local engine down) returns None — surfacing a "start your local engine"
|
||||
failure instead.
|
||||
"""
|
||||
from openjarvis.engine.cloud import CloudEngine
|
||||
|
||||
_reg("ollama", "ollama")
|
||||
EngineRegistry.register_value("cloud", CloudEngine)
|
||||
|
||||
cfg = JarvisConfig()
|
||||
cfg.engine.default = "ollama"
|
||||
|
||||
def _make(k, c): # noqa: ANN001
|
||||
if k == "ollama":
|
||||
# Local engine is down (post-restart Ollama not yet up).
|
||||
return _FakeEngine(healthy=False, models=["qwen3.5:0.8b"])
|
||||
# Real CloudEngine with only a (dummy) OpenAI client wired.
|
||||
eng = CloudEngine.__new__(CloudEngine)
|
||||
for name in (
|
||||
"_openai_client",
|
||||
"_anthropic_client",
|
||||
"_google_client",
|
||||
"_openrouter_client",
|
||||
"_minimax_client",
|
||||
"_deepseek_client",
|
||||
"_codex_client",
|
||||
):
|
||||
setattr(eng, name, object() if name == "_openai_client" else None)
|
||||
return eng
|
||||
|
||||
with mock.patch(
|
||||
"openjarvis.engine._discovery._make_engine",
|
||||
side_effect=_make,
|
||||
):
|
||||
result = get_engine(cfg, model="qwen3.5:0.8b")
|
||||
# Cloud must NOT be selected for a local model name.
|
||||
assert result is None
|
||||
|
||||
def test_model_none_preserves_model_agnostic_selection(self) -> None:
|
||||
"""model=None keeps the legacy behaviour: first healthy engine wins."""
|
||||
_reg("primary", "primary")
|
||||
|
||||
@@ -487,6 +487,167 @@ class TestChatCompletions:
|
||||
assert data["choices"][0]["finish_reason"] == "stop"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Identity system-prompt injection (#540)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_capturing_engine(captured: list):
|
||||
"""Like ``_make_engine`` but records the messages each path receives.
|
||||
|
||||
``engine.generate`` is a MagicMock so ``call_args`` works on the
|
||||
direct/non-stream path. ``engine.stream`` / ``engine.stream_full`` are
|
||||
plain async-generator FUNCTIONS, so they capture their ``messages``
|
||||
argument into the shared *captured* list from inside the generator body
|
||||
(``call_args`` does not apply to plain functions).
|
||||
"""
|
||||
engine = MagicMock()
|
||||
engine.engine_id = "mock"
|
||||
engine.health.return_value = True
|
||||
engine.list_models.return_value = ["test-model"]
|
||||
engine.generate.return_value = {
|
||||
"content": "ok",
|
||||
"usage": {"prompt_tokens": 5, "completion_tokens": 3, "total_tokens": 8},
|
||||
"model": "test-model",
|
||||
"finish_reason": "stop",
|
||||
}
|
||||
|
||||
async def mock_stream(messages, *, model, temperature=0.7, max_tokens=1024, **kw):
|
||||
captured.append(messages)
|
||||
for token in ["Hello", " ", "world"]:
|
||||
yield token
|
||||
|
||||
async def mock_stream_full(
|
||||
messages, *, model, temperature=0.7, max_tokens=1024, **kw
|
||||
):
|
||||
from openjarvis.engine._stubs import StreamChunk
|
||||
|
||||
captured.append(messages)
|
||||
yield StreamChunk(content="ok", finish_reason="stop")
|
||||
|
||||
engine.stream = mock_stream
|
||||
engine.stream_full = mock_stream_full
|
||||
return engine
|
||||
|
||||
|
||||
class TestIdentityPromptInjection:
|
||||
"""Regression for #540.
|
||||
|
||||
The desktop UI posts only user/assistant turns to the
|
||||
OpenAI-compatible ``/v1/chat/completions`` endpoint, so the engine never
|
||||
saw OpenJarvis's identity system prompt and the model answered from its
|
||||
training identity ("I'm Claude", "I am Qwen", ...). The engine-direct
|
||||
server handlers must now inject ``agent.default_system_prompt`` whenever
|
||||
the client omits a system message — and must NOT inject a second one when
|
||||
the client already supplies their own.
|
||||
"""
|
||||
|
||||
def test_stream_injects_identity_when_absent(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [{"role": "user", "content": "who are you?"}],
|
||||
"stream": True,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
# Drain the stream so the generator body runs and records messages.
|
||||
_ = resp.text
|
||||
assert captured, "engine.stream was never called"
|
||||
msgs = captured[-1]
|
||||
assert msgs[0].role.value == "system"
|
||||
assert "OpenJarvis" in msgs[0].content
|
||||
|
||||
def test_stream_no_double_injection_when_client_supplies_system(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [
|
||||
{"role": "system", "content": "Be terse."},
|
||||
{"role": "user", "content": "who are you?"},
|
||||
],
|
||||
"stream": True,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
_ = resp.text
|
||||
msgs = captured[-1]
|
||||
system_msgs = [m for m in msgs if m.role.value == "system"]
|
||||
assert len(system_msgs) == 1
|
||||
assert system_msgs[0].content == "Be terse."
|
||||
|
||||
def test_direct_injects_identity_when_absent(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
# No agent -> non-stream request goes through _handle_direct.
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [{"role": "user", "content": "who are you?"}],
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert engine.generate.called
|
||||
msgs = engine.generate.call_args.args[0]
|
||||
assert msgs[0].role.value == "system"
|
||||
assert "OpenJarvis" in msgs[0].content
|
||||
|
||||
def test_direct_no_double_injection_when_client_supplies_system(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [
|
||||
{"role": "system", "content": "Be terse."},
|
||||
{"role": "user", "content": "who are you?"},
|
||||
],
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
msgs = engine.generate.call_args.args[0]
|
||||
system_msgs = [m for m in msgs if m.role.value == "system"]
|
||||
assert len(system_msgs) == 1
|
||||
assert system_msgs[0].content == "Be terse."
|
||||
|
||||
def test_stream_tools_injects_identity_when_absent(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [{"role": "user", "content": "who are you?"}],
|
||||
"tools": [{"type": "function", "function": {"name": "calc"}}],
|
||||
"stream": True,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
_ = resp.text
|
||||
assert captured, "engine.stream_full was never called"
|
||||
msgs = captured[-1]
|
||||
assert msgs[0].role.value == "system"
|
||||
assert "OpenJarvis" in msgs[0].content
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Models endpoint tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user