Skip to content

Re-anchor the composio scopes refusal on the message it now emits - #5818

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/reanchor-composio-scopes-refusal
Aug 27, 2026
Merged

M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/reanchor-composio-scopes-refusal

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Rust Core Coverage is red on main. This unblocks it.
  • One orphaned assertion: it pins "memory client not initialised", a string that no longer exists anywhere in src/.
  • Re-anchored on the message the path actually emits — measured, not guessed; the first attempt at this fix guessed the wrong arm.
  • Swept the rest of tests/raw_coverage rather than assuming this was the only one. It is.

Problem

composio_raw_coverage_e2e::composio_controller_registry_and_scope_handlers_cover_validation_edges
assertion failed: memory_missing.contains("memory client not initialised")
tests/raw_coverage/composio_raw_coverage_e2e.rs:877
$ grep -rn "memory client not initialised" src/ | wc -l
0

1bf2037a0"Stop booting the second in-process memory engine", merged via #5725 at 2026-08-26 15:39 — removed the in-process engine handle that produced that refusal and moved composio.set_user_scopes's storage half onto the bound memory driver. The handler's own comment records the change: "the old code refused with 'memory client not initialised'". The assertion was never re-anchored.

It merged green because the coverage lane is changed-modules-scoped (rust-coverage-changed.sh) and that PR touched nothing routing to tests/raw_coverage, so the target never ran. Same shape as #5757 and #5776.

Solution

ops::user_scopes::save refuses on three grounds, kept deliberately distinguishable:

arm condition
memory driver unavailable nothing bound
does not serve Graph, cannot persist pref bound, wrong family
kv_put failed: … bound, right family, the write itself failed

I re-anchored on the Graph arm first and it was wrong — the assertion carried the observed value, and the run reported:

[composio][scopes] kv_put failed: the module host policy was never published,
so module 'tinymemory' cannot be loaded; call modules::memory::set_modules_policy during boot

The module provider binds and does serve Graph; the cdylib is simply never loaded in a test binary that runs no boot sequence. So the third arm is correct.

Pinned: the [composio][scopes] tag and the kv_put failed arm. Together those are the fail-closed property — the handler must refuse rather than report a save it did not perform, and it must refuse on the write rather than on either of the other two grounds. The old string satisfies neither, so this is a genuine re-anchor and not a substring that would pass either way.

Deliberately not pinned: the reason after the colon. That text belongs to the module loader, not to this handler, and pinning another component's wording here is exactly how the previous assertion came to be orphaned.

Is this the only one?

Swept rather than assumed. Of 399 distinct .contains("…") assertions of 18+ chars under tests/raw_coverage, 202 name strings absent from src/ — but all 202 are fixture data (names, addresses, payload fragments): none was present in src/ at the 2026-08-20 baseline and removed since.

Method validated against unfixed main, where the same sweep returns exactly one true orphan — this one. So it sees what it is meant to see, and there is nothing else of this shape today.

Impact

Test-only. No product code, no vendor/ gitlink, no manifest. Unblocks Rust Core Coverage for every open PR.

Related

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) — this is the test change; the assertion now pins the fail-closed refusal on the arm the path reaches.
  • Diff coverage ≥ 80% — changed lines are test code, executed by the target this fixes.
  • Coverage matrix updated — N/A: no feature row added, removed or renamed; re-anchors an existing assertion.
  • All affected feature IDs from the matrix are listed under ## Related — N/A: no matrix feature IDs affected.
  • No new external network dependencies introduced — none; no dependency change of any kind.
  • Manual smoke checklist updated if this touches release-cut surfaces — N/A: test-only.
  • Linked issue closed via Closes #NNN — N/A: no issue filed for the red lane; cause named under ## Related.

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/reanchor-composio-scopes-refusal
  • Commit SHA: 2be4c25c1

Validation Run

  • pnpm --filter openhuman-app format:check — N/A: no file under app/ changed.
  • pnpm typecheck — N/A: no TypeScript changed.
  • Focused tests: --test raw_coverage_all -- composio_raw_coverage_e2e24 passed, 0 failed with the product feature set. The single case fails without the fix.
  • Rust fmt/check (if changed): no src/ change; the edited test compiles and runs as above.
  • Tauri fmt/check (if changed) — N/A: no Tauri shell file changed.

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: none. Product behaviour is untouched; the test now asserts what the product does.
  • User-visible effect: none.

Parity Contract

  • Legacy behavior preserved: yes — the assertion still requires the handler to fail closed, which is what the original was for. Only the message it pins has moved, following the code.
  • Guard/fallback/dispatch parity checks: the refusal remains mandatory; the arm asserted is now the one this path reaches, and the other two arms stay distinguishable.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none.
  • Canonical PR: this one.
  • Resolution: N/A.

Revert check

Reverting the assertion to the old string fails the target, naming this assertion rather than incidentally:

tests/raw_coverage/composio_raw_coverage_e2e.rs:903:5:
set_user_scopes must fail CLOSED on the backing write rather than reporting a save
it did not perform; got: [composio][scopes] kv_put failed: ...

Summary by CodeRabbit

  • Bug Fixes
    • Updated scope-setting validation to report clearer storage write failures.
    • Ensured failures are handled safely when the backing storage operation cannot be completed.
  • Tests
    • Expanded coverage for scope-handling error conditions and updated expectations for the revised error messaging.

`Rust Core Coverage` is red on main:

    composio_raw_coverage_e2e::composio_controller_registry_and_scope_handlers_cover_validation_edges
    assertion failed: memory_missing.contains("memory client not initialised")

The string does not exist anywhere in `src/`. `1bf2037a0` ("Stop booting the
second in-process memory engine", tinyhumansai#5725, merged 15:39) removed the
in-process engine handle that produced it and moved the storage half onto
the bound driver. The assertion was never re-anchored, so it has been
asserting a message the product no longer emits ever since — an orphaned
test in the sense of tinyhumansai#5757 / tinyhumansai#5776.

It survived its own PR because the coverage lane is changed-modules-scoped
and that PR did not touch a module which routes to `tests/raw_coverage`.

`ops::user_scopes::save` refuses on three deliberately distinguishable
grounds: "memory driver unavailable" (nothing bound), "does not serve Graph"
(bound, wrong family), and "kv_put failed" (bound, right family, the write
itself failed). Measured rather than assumed — the first re-anchor here
guessed the Graph arm and was wrong. What this path actually reaches is:

    [composio][scopes] kv_put failed: the module host policy was never
    published, so module 'tinymemory' cannot be loaded; ...

The module provider binds and does serve Graph; the cdylib is simply never
loaded in a test binary that runs no boot sequence.

Pinned: the `[composio][scopes]` tag and the `kv_put failed` arm, which is
what makes this a fail-closed assertion — the handler must refuse rather
than report a save it did not perform, and it must refuse on the write
rather than on one of the other two grounds. Deliberately NOT pinned: the
reason after the colon, which belongs to the module loader rather than to
this handler. Pinning another component's wording here is how the previous
assertion came to be orphaned in the first place.

Swept the rest of the suite rather than assuming this was the only one: of
399 distinct long `.contains("…")` assertions under tests/raw_coverage, 202
name strings absent from `src/`, but all 202 are fixture data — none was
present in `src/` at 2026-08-20 and removed since. Against unfixed main the
same sweep returns exactly one true orphan, this one, which is how I know
the method sees what it is meant to see.

Verified: the target passes with the fix and fails without it, naming this
assertion ("must fail CLOSED on the backing write ...").
@M3gA-Mind
M3gA-Mind requested a review from a team August 27, 2026 06:20
@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Aug 27, 2026

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tinysweeper found nothing blocking. Approving.

$0.0000 · 0 in / 0 out

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 14894453-8072-4763-a396-5b7ac6296fc0

📥 Commits

Reviewing files that changed from the base of the PR and between ae99c67 and 2be4c25.

📒 Files selected for processing (1)
  • tests/raw_coverage/composio_raw_coverage_e2e.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The end-to-end scope coverage test now checks the [composio][scopes] prefix and kv_put failed text when bound-driver storage rejects a write.

Changes

Composio scope validation

Layer / File(s) Summary
Scope storage failure assertion
tests/raw_coverage/composio_raw_coverage_e2e.rs
The set_user_scopes test replaces the removed memory-client error expectation with the bound-driver failure contract. The comment records that only the error tag and failure arm are pinned.

Estimated code review effort: 2 (Simple) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 2be4c

This test-only change re-anchors an assertion without changing product behavior or runtime configuration, so no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: senamakel

Poem

A rabbit checks the scope write twice
The driver answers, clear and precise
The old memory tale hops away
[composio][scopes] marks the day
kv_put failed keeps tests bright

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: updating the Composio scopes test assertion to match the refusal message now emitted by the handler.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Warning

Your free Security trial is over. An organization admin can activate billing to continue.


Comment @coderabbitai help to get the list of available commands.

@M3gA-Mind
M3gA-Mind merged commit 04075d5 into tinyhumansai:main Aug 27, 2026
29 of 33 checks passed
senamakel pushed a commit to nocstah/openhuman that referenced this pull request Sep 11, 2026
…osio-scopes-refusal\n\nRe-anchor the composio scopes refusal on the message it now emits\n
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant