Skip to content

test(workspace): add direct regression tests for scoped_to_user rebinding (#1652) - #1875

Merged
serrrfirat merged 1 commit into
nearai:stagingfrom
reidliu41:test/workspace-scoped-rebind-1652
Apr 5, 2026
Merged

serrrfirat merged 1 commit into
nearai:stagingfrom
reidliu41:test/workspace-scoped-rebind-1652

Conversation

@reidliu41

@reidliu41 reidliu41 commented Apr 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Verify primary user_id switches correctly and original workspace is unchanged
  • Verify private memory layers rescope to new user while shared layers stay intact
  • Verify secondary read scopes are preserved exactly and old primary is removed with no duplicates
  • Verify identity reads via both read_primary() and read() pin to new primary, not old or shared scopes
  • Verify non-identity reads still span preserved shared scopes after old primary removal
  • Verify bootstrap flags (pending + completed) preserve on same-user rebind and reset on different-user rebind
  • Establish precondition probes to ensure bootstrap reset tests validate true→false, not default false

Change Type

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • CI/Infrastructure
  • Security
  • Dependencies

Linked Issue

Closes #1652

Validation

  • cargo fmt --all -- --check
  • cargo clippy --all --benches --tests --examples --all-features -- -D warnings
  • cargo build
  • Relevant tests pass:
  • cargo test --features integration if database-backed or integration behavior changed
  • Manual testing:
  • If a coding agent was used and supports it, review-pr or pr-shepherd --fix was run before requesting review

Security Impact

None.

Database Impact

None.

Blast Radius

  • New file only: tests/workspace_scoped_rebind.rs
  • Zero production code changes
  • No impact on existing tests or behavior

Rollback Plan

Delete tests/workspace_scoped_rebind.rs.

Review Follow-Through

  • Tests are gated behind #![cfg(feature = "libsql")] — cargo test alone will not run them
  • If new rebinding invariants are added to scoped_to_user(), this file should be extended

Review track:

A

…ding (nearai#1652)

  - Verify primary user_id switches correctly and original workspace is unchanged
  - Verify private memory layers rescope to new user while shared layers stay intact
  - Verify secondary read scopes are preserved exactly and old primary is removed with no duplicates
  - Verify identity reads via both read_primary() and read() pin to new primary, not old or shared scopes
  - Verify non-identity reads still span preserved shared scopes after old primary removal
  - Verify bootstrap flags (pending + completed) preserve on same-user rebind and reset on different-user rebind
  - Establish precondition probes to ensure bootstrap reset tests validate true→false, not default false
@github-actions github-actions Bot added size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Apr 1, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds a comprehensive suite of regression tests in tests/workspace_scoped_rebind.rs to verify the behavior of Workspace::scoped_to_user(). The tests cover primary user ID changes, layer rescoping, read scope preservation, and bootstrap flag management. A review comment identifies a brittle implementation reference in a code comment and suggests a more descriptive alternative for better maintainability.

"read_primary must return exactly the new primary's identity"
);

// read — identity paths get special primary-only treatment (mod.rs:665).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Referencing a specific line number in another file (mod.rs:665) makes this comment brittle and hard to maintain. If src/workspace/mod.rs changes, this reference will become outdated and misleading. It's better to describe the behavior itself rather than pointing to a specific line of implementation.

Suggested change
// read — identity paths get special primary-only treatment (mod.rs:665).
// read — identity paths are specially handled to read from the primary scope only.

@zmanian zmanian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well-structured regression tests for scoped_to_user rebinding. 7 focused tests covering user ID change, layer rescoping, identity pinning, and bootstrap flag behavior. Test-only, zero production changes. Good.

@reidliu41

Copy link
Copy Markdown
Contributor Author

can it merge?

@serrrfirat
serrrfirat merged commit e169591 into nearai:staging Apr 5, 2026
14 checks passed
@claude

claude Bot commented Apr 5, 2026

Copy link
Copy Markdown

Code review

Found 2 issues:

  1. [LOW:100] Line 46: Assertion message uses literal string {scopes:?} instead of format interpolation — when the assertion fails, the error message will show the literal string instead of the actual scope list, reducing debugging value.
https://github.com/nearai/ironclaw/blob/437d7373a6df2c924a4b5162715ed59ff67d7170/tests/workspace_scoped_rebind.rs#L44-L48

43: fn assert_no_duplicates(scopes: &[String]) {
44: let set: HashSet<&String> = scopes.iter().collect();
45: assert_eq!(
46: set.len(),
47: scopes.len(),
48: "read_user_ids contains duplicates: {scopes:?}"
49: );
50: }

  1. [LOW:85] Line 180: Test comment references mod.rs:665 but the actual identity file read handling logic is at line 766 in src/workspace/mod.rs. This is a documentation accuracy issue only — the test behavior is correct.
https://github.com/nearai/ironclaw/blob/437d7373a6df2c924a4b5162715ed59ff67d7170/tests/workspace_scoped_rebind.rs#L177-L182

177: // read — identity paths get special primary-only treatment (mod.rs:665).
178: let doc2 = rebound
179: .read(paths::IDENTITY)
180: .await
181: .expect("read should succeed");

Assessment: This is a well-structured regression test suite for scoped_to_user() rebinding. The tests correctly validate:

  • Primary user ID switching with independent workspace instances
  • Private layer rescoping while preserving shared layers
  • Bootstrap flag behavior (preserved on same-user rebind, reset on different-user)
  • Identity file pinning to the new primary
  • Shared scope preservation after old primary removal

Zero production code changes, no security issues, no blocking logic errors. The reported issues are cosmetic — an uninformative error message and a stale comment reference.

serrrfirat pushed a commit that referenced this pull request Apr 5, 2026
…ding (#1652) (#1875)

- Verify primary user_id switches correctly and original workspace is unchanged
  - Verify private memory layers rescope to new user while shared layers stay intact
  - Verify secondary read scopes are preserved exactly and old primary is removed with no duplicates
  - Verify identity reads via both read_primary() and read() pin to new primary, not old or shared scopes
  - Verify non-identity reads still span preserved shared scopes after old primary removal
  - Verify bootstrap flags (pending + completed) preserve on same-user rebind and reset on different-user rebind
  - Establish precondition probes to ensure bootstrap reset tests validate true→false, not default false
This was referenced Apr 5, 2026
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
…ding (nearai#1652) (nearai#1875)

- Verify primary user_id switches correctly and original workspace is unchanged
  - Verify private memory layers rescope to new user while shared layers stay intact
  - Verify secondary read scopes are preserved exactly and old primary is removed with no duplicates
  - Verify identity reads via both read_primary() and read() pin to new primary, not old or shared scopes
  - Verify non-identity reads still span preserved shared scopes after old primary removal
  - Verify bootstrap flags (pending + completed) preserve on same-user rebind and reset on different-user rebind
  - Establish precondition probes to ensure bootstrap reset tests validate true→false, not default false
@ironclaw-ci ironclaw-ci Bot mentioned this pull request Apr 10, 2026
@ironclaw-ci ironclaw-ci Bot mentioned this pull request Apr 18, 2026
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…ding (nearai#1652) (nearai#1875)

- Verify primary user_id switches correctly and original workspace is unchanged
  - Verify private memory layers rescope to new user while shared layers stay intact
  - Verify secondary read scopes are preserved exactly and old primary is removed with no duplicates
  - Verify identity reads via both read_primary() and read() pin to new primary, not old or shared scopes
  - Verify non-identity reads still span preserved shared scopes after old primary removal
  - Verify bootstrap flags (pending + completed) preserve on same-user rebind and reset on different-user rebind
  - Establish precondition probes to ensure bootstrap reset tests validate true→false, not default false
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules size: L 200-499 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add direct tests for Workspace scoped-to-user rebinding

3 participants