diff --git a/nodes/src/nodes/tool_google_workspace/IInstance.py b/nodes/src/nodes/tool_google_workspace/IInstance.py index 5a1ebbfe7..070f720f0 100644 --- a/nodes/src/nodes/tool_google_workspace/IInstance.py +++ b/nodes/src/nodes/tool_google_workspace/IInstance.py @@ -91,14 +91,27 @@ def _check_connection_impl(self, probe: 'Callable[[Any], None] | None' = None) - 'connection_ok': self._svc() is not None, 'access': getattr(access, 'tier', None), 'requiredScopes': list(getattr(access, 'scopes', []) or []), + # What was actually exercised, so a caller can tell a verified + # answer from an assumed one. Without this, `connection_ok: true` + # meant "we hold a plausible credential" while everyone read it as + # "this node works" — see #1694, where it sat next to a 403 saying + # the API was disabled and sent the reporter off re-authenticating. + 'checked': ['client', 'scopes'], } service = self._svc() if probe is not None and service is not None: try: probe(service) + out['checked'].append('api') except Exception as exc: out['connection_ok'] = False out['error'] = str(exc)[:200] + # Google distinguishes accessNotConfigured (API disabled) from + # forbidden (permission) from rateLimitExceeded, and that word + # alone names the fix. Losing it costs the reader the diagnosis. + reason = getattr(exc, 'reason', None) or getattr(exc, '_get_reason', lambda: None)() + if reason: + out['errorReason'] = str(reason).strip()[:120] try: cfg = Config.getNodeConfig(self.IGlobal.glb.logicalType, self.IGlobal.glb.connConfig) auth_type = (cfg.get('authType') or 'service').strip() @@ -116,4 +129,15 @@ def _check_connection_impl(self, probe: 'Callable[[Any], None] | None' = None) - out['error'] = f'invalid user token data: {str(exc)[:160]}' except Exception: pass # config lookup diagnostics must never raise + + # No live call was made, so `connection_ok` here only means the + # credential looks usable. Say so rather than letting it read as proof: + # a disabled API, an exhausted quota, disabled billing and an org-policy + # block are all invisible to a client-and-scopes check. + if 'api' not in out['checked'] and out['connection_ok']: + out['connection_ok'] = 'unknown' + out['note'] = ( + 'Credential and scopes look correct; no API call was made, so this ' + 'does not confirm the service is reachable or enabled.' + ) return out diff --git a/nodes/src/nodes/tool_google_workspace/drive/IInstance.py b/nodes/src/nodes/tool_google_workspace/drive/IInstance.py index 5267e4933..112d7a178 100644 --- a/nodes/src/nodes/tool_google_workspace/drive/IInstance.py +++ b/nodes/src/nodes/tool_google_workspace/drive/IInstance.py @@ -162,7 +162,7 @@ def _files(self): ) def check_connection(self, args: dict) -> dict: """Check Drive connection status and whether granted OAuth scopes cover the access tier. Read-only.""" - return self._check_connection_impl() + return self._check_connection_impl(probe=lambda s: execute(s.about().get(fields='user'))) # ======================================================================= # FILES — read diff --git a/nodes/src/nodes/tool_google_workspace/gmail/IInstance.py b/nodes/src/nodes/tool_google_workspace/gmail/IInstance.py index 2b7e27534..2b3bbbb25 100644 --- a/nodes/src/nodes/tool_google_workspace/gmail/IInstance.py +++ b/nodes/src/nodes/tool_google_workspace/gmail/IInstance.py @@ -146,7 +146,7 @@ def _id_list(args: dict, key: str, op: str) -> list[str]: ) def check_connection(self, args: dict) -> dict: """Check Gmail connection status and whether granted OAuth scopes cover the configured access tier. Read-only.""" - return self._check_connection_impl() + return self._check_connection_impl(probe=lambda s: execute(s.users().getProfile(userId='me'))) # ======================================================================= # MESSAGES — read diff --git a/nodes/test/tool_google_workspace/docs/test_docs.py b/nodes/test/tool_google_workspace/docs/test_docs.py index c05860e76..f9e4433d1 100644 --- a/nodes/test/tool_google_workspace/docs/test_docs.py +++ b/nodes/test/tool_google_workspace/docs/test_docs.py @@ -260,11 +260,16 @@ def test_default_tier_is_write(): assert access.can_write is True -def test_check_connection_reports_ok(): +def test_check_connection_reports_unknown_without_api_probe(): inst = _make() out = inst.check_connection({}) assert isinstance(out, dict) - assert out['connection_ok'] is True + # 'unknown', not True: docs/sheets make no API probe, and a client-and-scopes + # check cannot see a disabled API, an exhausted quota, disabled billing or an + # org-policy block. Reporting True there is the false green this change removes. + assert out['connection_ok'] == 'unknown' + assert 'api' not in out['checked'] + assert 'note' in out assert out['access'] == 'write' assert any('documents' in s for s in out['requiredScopes']) diff --git a/nodes/test/tool_google_workspace/drive/test_drive.py b/nodes/test/tool_google_workspace/drive/test_drive.py index 9a5dc5f64..d79398608 100644 --- a/nodes/test/tool_google_workspace/drive/test_drive.py +++ b/nodes/test/tool_google_workspace/drive/test_drive.py @@ -186,6 +186,13 @@ def drives(self): def changes(self): return _Node(self, 'changes') + def about(self): + # check_connection's liveness probe calls about().get(fields='user'). + # Without this the probe raises AttributeError and the node reports + # connection_ok=False — which is correct behaviour for an unreachable + # API, and exactly why the diagnostic must be exercised, not assumed. + return _Node(self, 'about') + def call_for(self, op): """Return the kwargs of the FIRST recorded call to terminal method ``op``.""" return next((kw for n, kw in self.calls if n == op), None) @@ -727,6 +734,10 @@ def test_check_connection_reports_ok(): inst = _make() out = inst.check_connection({}) assert out['connection_ok'] is True + # True is only honest when an API call actually happened; assert the probe + # ran rather than trusting the boolean on its own. + assert 'api' in out['checked'] + assert inst.IGlobal.service.call_for('get') is not None assert out['access'] == 'write' assert any('drive' in s for s in out['requiredScopes']) diff --git a/nodes/test/tool_google_workspace/sheets/test_sheets.py b/nodes/test/tool_google_workspace/sheets/test_sheets.py index d42ecf531..d69ac7f38 100644 --- a/nodes/test/tool_google_workspace/sheets/test_sheets.py +++ b/nodes/test/tool_google_workspace/sheets/test_sheets.py @@ -310,10 +310,14 @@ def test_default_tier_is_write(): # --------------------------------------------------------------------------- -def test_check_connection_reports_ok(): +def test_check_connection_reports_unknown_without_api_probe(): inst = _make() out = inst.check_connection({}) - assert out['connection_ok'] is True + # 'unknown', not True: sheets makes no API probe, so the credential looking + # correct is not evidence the service is reachable. See IInstance.check_connection. + assert out['connection_ok'] == 'unknown' + assert 'api' not in out['checked'] + assert 'note' in out assert out['access'] == 'write' assert any('spreadsheets' in s for s in out['requiredScopes'])