Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions docs/modules/server/connections.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,48 @@ Two consequences worth knowing:
probe is a network call with no injection point, and the removed gate lived on
the far side of it, which is how it survived unasserted.

## The capabilities panel's Composio verdict is a tier, not a stored token (issue #886)

`GET …/capabilities` is the panel an operator checks first when a tool looks
missing, so a wrong answer there sends the whole debugging session the wrong
way. It used to compute its Composio verdict from `composio::token_configured`,
which reads exactly one secret slot — the BYO override `composio/token`.

The credential is resolved over **three** tiers, and the toolbelt gates on all
three (`composio::resolve_credential`, the seam issue #586 established): the BYO
override, then the company's own TinyHumans key, then this instance's platform
identity. On a hosted tenant nobody pastes a BYO token — the third tier answers,
the tools wire up, and the agents call them successfully. The one-tier probe
reported `false` throughout.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

So the route now sends **both**, answering two different questions:

| Field | Question | Shape |
| --- | --- | --- |
| `composioTokenConfigured` | did *this company* paste a BYO token? | boolean, unchanged meaning |
| `composioCredentialSource` | which tier does the credential resolve from? | the same `attested` / `company` / `static` / `none` tiers above, from the resolver itself |
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

Three properties are load-bearing:

- **The tier comes from the resolver, never from a second copy of its
precedence.** This is the same rule the `GET …/composio` status route already
follows; #886 is the copy that route's migration missed. The console must
never be able to name a tier the agents are not on.
- **An unreadable secret store omits the field**, rather than reporting `none`.
`none` is a verdict — "nothing resolves, no tools are wired" — and claiming it
on a transient hiccup sends an operator to paste a token they already have.
The console treats an absent field as unknown and must not render it in the
alarm colour.
- **It is a resolution verdict, not a liveness one.** `attested` says a bearer
can be obtained, not that Composio answered or that any account is connected.
`GET …/connections` above is the axis that answers those, and a company with a
valid bearer and zero connections is a working empty account, not a fault.

The evidence pack the planning station builds reads the same resolver, for the
same reason: it used to tell operators "this company has no Composio credential,
so no Composio account can be reached" on a card whose own evidence listed the
connectors as connected.

## Releasing a connection: two routes, not interchangeable (issue #404)

There are two disconnects and they act on different things:
Expand Down
29 changes: 28 additions & 1 deletion frontend/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -969,8 +969,35 @@ export interface CapabilityStatusDto {
composioGranted?: boolean;
/** Whether the `composio` feature is compiled into this build at all. */
composioInBuild?: boolean;
/** Whether a per-tenant Composio token is stored — never the token itself. */
/**
* Whether a per-tenant Composio **BYO override** token is stored under
* `composio/token` — never the token itself.
*
* Narrow on purpose, and **not** "can this company reach Composio" (issue
* #886). The BYO slot is the first of three credential tiers; on a hosted
* tenant nobody pastes one and the instance's platform identity answers, so
* this reads `false` for companies whose Composio tools are wired and
* working. Read `composioCredentialSource` for the resolution verdict.
*/
composioTokenConfigured?: boolean;
/**
* Which tier this company's Composio credential actually resolves from
* (issue #886) — the same three-tier resolution the toolbelt gates on, and
* the same `credentialSource` the Composio status route reports:
*
* * `attested` — the instance's platform identity (nothing stored here);
* * `company` — the company's own TinyHumans key;
* * `static` — a pasted BYO token, or a static instance key;
* * `none` — nothing resolves, so no tools are wired.
*
* A **resolution** verdict, not a liveness one: `attested` says a bearer can
* be obtained, not that Composio answered or that any account is connected.
*
* `undefined` is **unknown** — either an older host that does not send the
* field, or one whose secret store could not be read this request. It must
* never be rendered as `none`: that is the #886 lie in the other direction.
*/
composioCredentialSource?: "attested" | "company" | "static" | "none";
/**
* Metered web search (issue #238): whether the company **explicitly** grants
* the `search` namespace (a `*` wildcard does not count). Every call is a
Expand Down
39 changes: 30 additions & 9 deletions frontend/src/views/UsageView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ const NAMESPACE_LABELS: Record<string, string> = {
};

// Badge variant subset the media status row uses.
type BadgeVariant = "default" | "secondary" | "destructive" | "outline";
export type BadgeVariant = "default" | "secondary" | "destructive" | "outline";

/**
* The media-generation capability (issue #109) is opt-in per tool grant and
Expand Down Expand Up @@ -341,9 +341,8 @@ function mediaStatus(caps: CapabilityStatusDto): { label: string; variant: Badge

/**
* The Composio capability (issue #110) is opt-in per tool grant and gated on a
* per-tenant OAuth token, so it gets its own status row like media. Four states:
* not compiled into this build, not granted, granted-but-awaiting-token, and
* active. Set the token from Connections.
* resolved credential, so it gets its own status row like media. Five states —
* see {@link composioStatus}.
*/
function ComposioStatusRow({ caps }: { caps: CapabilityStatusDto }) {
const { label, variant } = composioStatus(caps);
Expand All @@ -352,8 +351,10 @@ function ComposioStatusRow({ caps }: { caps: CapabilityStatusDto }) {
<div className="space-y-0.5">
<span className="font-medium">Composio integrations</span>
<p className="text-xs text-muted-foreground">
Gmail, Slack &amp; GitHub via Composio — opt-in, runs on the company&apos;s own OAuth
token, and every send/authorize is approved before it runs. Set the token in Connections.
Gmail, Slack &amp; GitHub via Composio — opt-in, and every send/authorize is approved
before it runs. Runs on this company&apos;s own Composio token when one is set in
Connections; otherwise on the company&apos;s TinyHumans key, or on the platform identity
this instance already carries.
</p>
</div>
<Badge variant={variant} className="shrink-0">
Expand All @@ -363,11 +364,31 @@ function ComposioStatusRow({ caps }: { caps: CapabilityStatusDto }) {
);
}

function composioStatus(caps: CapabilityStatusDto): { label: string; variant: BadgeVariant } {
/**
* The Composio row's five states, in order (issue #886).
*
* The credential is resolved over three tiers — a BYO Composio token, the
* company's TinyHumans key, this instance's platform identity — so "is a token
* stored" is the wrong question to render. This reads `composioCredentialSource`,
* the tier the host says the toolbelt actually resolves.
*
* The `undefined` rung is load-bearing and must stay above the `"none"` rung.
* `undefined` means the host did not answer — an older build that does not send
* the field, or one whose secret store could not be read — and falling through
* it into the destructive branch is exactly the bug #886 was filed about: a red
* "no credential" badge over a Composio account that is working. Unknown is
* shown as unknown, and never in the alarm colour.
*/
export function composioStatus(caps: CapabilityStatusDto): {
label: string;
variant: BadgeVariant;
} {
if (caps.composioInBuild === false) return { label: "Not in this build", variant: "outline" };
if (!caps.composioGranted) return { label: "Not granted", variant: "secondary" };
if (!caps.composioTokenConfigured)
return { label: "Awaiting token", variant: "destructive" };
if (caps.composioCredentialSource === undefined)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

priority medium critique likely

Add tests for composioStatus credential source states

The repository requires focused tests with every behavior change. This function now branches on composioCredentialSource (undefined, "none", and other values) instead of composioTokenConfigured, but the index shows no test exercises composioStatus. Add unit tests covering each of the five states, especially the undefined rung that must not fall through to the destructive branch.

[RULE] missing-tests ·

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

A test for exactly this was added in this PR: frontend/test/unit/composio-capability-status.test.ts.

It covers the five-state matrix, and pins the specific rung this finding is right to care about — undefined must render non-destructively rather than falling through to the red branch, because falling through is what re-creates #886. Verified failing against the old composioStatus body (6 of 8 cases) before the fix.

return { label: "Couldn't check", variant: "outline" };
if (caps.composioCredentialSource === "none")
return { label: "Awaiting credential", variant: "destructive" };
return { label: "Active", variant: "default" };
}

Expand Down
91 changes: 91 additions & 0 deletions frontend/test/unit/composio-capability-status.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* Issue #886 — the Composio row on the Usage view must never paint a working
* connector red.
*
* The credential resolves over three tiers (a BYO Composio token, the company's
* TinyHumans key, this instance's platform identity). The row used to read
* `composioTokenConfigured`, which answers only the first, so a hosted tenant
* running on the platform identity got "Awaiting token" in the alarm colour
* while its agents were calling `GITHUB_*` tools successfully.
*
* The state that matters most here is the one with no obvious label: the host
* not answering. It is a separate rung above `"none"` precisely so it cannot
* fall through into the destructive branch and re-create the bug.
*/
import { describe, expect, it } from "vitest";

import type { CapabilityStatusDto } from "@/api/types";
import { composioStatus } from "@/views/UsageView";

/** A granted, in-build company — the only shape the credential rungs are reached from. */
function granted(over: Partial<CapabilityStatusDto> = {}): CapabilityStatusDto {
return {
configured: false,
composioInBuild: true,
composioGranted: true,
...over,
};
}

describe("composioStatus", () => {
it("reports a build without the feature before anything else", () => {
expect(
composioStatus(granted({ composioInBuild: false, composioCredentialSource: "attested" })),
).toEqual({ label: "Not in this build", variant: "outline" });
});

it("reports an ungranted company without consulting the credential", () => {
expect(
composioStatus(granted({ composioGranted: false, composioCredentialSource: "attested" })),
).toEqual({ label: "Not granted", variant: "secondary" });
});

/**
* The #886 regression guard. An unanswered host is unknown, not broken —
* and specifically not `destructive`, which is the colour that sent the
* original debugging in the wrong direction.
*/
it("reports an unanswered host as unknown, never as an alarm", () => {
const status = composioStatus(granted({ composioCredentialSource: undefined }));
expect(status.label).toBe("Couldn't check");
expect(status.variant).not.toBe("destructive");
});

it("reports a genuinely unresolvable credential as the destructive state", () => {
expect(composioStatus(granted({ composioCredentialSource: "none" }))).toEqual({
label: "Awaiting credential",
variant: "destructive",
});
});

/**
* All three resolving tiers are Active. `attested` is the hosted shape the
* issue was reported against, and it is the one the old code got wrong:
* nothing is stored on the instance, so `composioTokenConfigured` is `false`
* while the toolbelt is fully wired.
*/
it.each(["attested", "company", "static"] as const)(
"reports a resolved `%s` credential as active",
(source) => {
expect(
composioStatus(granted({ composioCredentialSource: source, composioTokenConfigured: false })),
).toEqual({ label: "Active", variant: "default" });
},
);

/**
* The narrow legacy field must not be able to steer the verdict in either
* direction: it answers "did somebody paste a BYO token", which is a
* different question from "does a credential resolve".
*/
it("ignores the BYO-token flag once the resolver has answered", () => {
expect(
composioStatus(granted({ composioTokenConfigured: true, composioCredentialSource: "none" }))
.variant,
).toBe("destructive");
expect(
composioStatus(granted({ composioTokenConfigured: false, composioCredentialSource: "attested" }))
.label,
).toBe("Active");
});
});
24 changes: 23 additions & 1 deletion src/company/composio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,29 @@ pub async fn resolve_credential(
})
}

/// Whether a non-empty per-tenant token is stored — never the token itself.
/// Whether a non-empty **BYO override** token is stored under [`TOKEN_KEY`] —
/// never the token itself.
///
/// ## This is not "can this company reach Composio" (issue #886)
///
/// It answers exactly one question about exactly one secret slot: did somebody
/// paste a token into the company's own [`TOKEN_KEY`]. That is the *first* tier
/// of three. [`resolve_credential`] falls through it to the company's own
/// TinyHumans key and then to this instance's platform identity, and on a hosted
/// tenant it is the third tier that answers — nobody pastes a BYO token there.
/// So `false` from here is routinely true of a company whose Composio tools are
/// wired and working, which is precisely what #886 was filed about: the
/// capabilities panel reported `composioTokenConfigured: false` while agents
/// were calling `GITHUB_*` tools successfully in the same session.
///
/// **If you want to know whether Composio will work, call
/// [`resolve_credential`] and ask the returned [`Credential`] — `configured()`
/// for the boolean, [`source`](Credential::source) for the tier.** That is the
/// same derivation the toolbelt gates on
/// ([`TenantComposio::resolve`](crate::harness::composio::TenantComposio::resolve)),
/// so it cannot disagree with what the agents actually hold. Use this function
/// only where the BYO slot itself is the subject — a console field that says
/// whether *this company pasted a token*, not whether it has one.
pub async fn token_configured(company: &CompanyId, secrets: &dyn SecretStore) -> Result<bool> {
Ok(secrets
.get(company, TOKEN_KEY)
Expand Down
21 changes: 15 additions & 6 deletions src/harness/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,12 +398,16 @@ pub fn build_agent(
// `media`, the catch-all `*` does NOT grant it, so a broadly-permissioned
// company never accidentally hands its agents a live account-reaching
// surface; it must opt in by name.
// 2. a resolved per-tenant token on the deps (`deps.composio`), read from the
// company secret store by `HarnessPool::ensure` — never an env/platform
// key. The backend derives the Composio entity from THIS token, so it is
// the entire tenant-isolation lever.
// 2. a resolved credential on the deps (`deps.composio`), produced by
// `HarnessPool::ensure` through `composio::resolve_credential` — the BYO
// `composio/token` override, else the company's own TinyHumans key, else
// this instance's platform identity (issue #586). The backend derives the
// Composio entity from whichever tier answered, so this resolution is the
// entire tenant-isolation lever. It is NOT "a stored token": on a hosted
// tenant nobody pastes one and the platform identity is what wires the
// tools (issue #886).
//
// Granted-but-tokenless wires nothing and warns (fail-closed). The
// Granted-but-credential-less wires nothing and warns (fail-closed). The
// `authorize` / `execute` tools additionally park for operator approval via
// the `ApprovalPolicy`. Gated on the `composio` feature; the default/
// `openhuman` build never compiles this.
Expand All @@ -425,7 +429,12 @@ pub fn build_agent(
None => tracing::warn!(
company = %company,
agent = %manifest_agent.id,
"[build] agent explicitly grants `composio` but no per-tenant Composio token is configured; composio tools NOT wired (fail-closed)"
// Issue #886: the gate is `deps.composio.is_none()`, which is a
// *resolver* outcome over three tiers (BYO `composio/token`,
// the company's TinyHumans key, this instance's platform
// identity) — not "no token is stored". Naming the stored token
// sent operators to paste one they did not need.
"[build] agent explicitly grants `composio` but no Composio credential could be resolved for this company; composio tools NOT wired (fail-closed)"
),
}
}
Expand Down
Loading