Skip to content

Clean up connect - #405

Open
kentbull wants to merge 4 commits into
WebOfTrust:mainfrom
kentbull:clean-up-connect
Open

Clean up connect#405
kentbull wants to merge 4 commits into
WebOfTrust:mainfrom
kentbull:clean-up-connect

Conversation

@kentbull

@kentbull kentbull commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

This PR cleans up some long-standing gaps we've had in SignifyController booting and connecting:

  • Adds cancellation with AbortSignal to boot and connect flows.
  • Adds error handling to POST /agent/<ctrl Pre>?type=ixn delegation request.
  • Reject malformed sequence data (non-string) and removes overly permissive numeric coercion.

kentbull added 2 commits July 25, 2026 15:38
Separate ordinary reconnection from the post-boot path so a newly booted client can reuse its existing Controller while reconnecting clients restore authoritative state from KERIA. Validate the establishment state before allowing Controller reuse.

Require KERIA to accept the Agent delegation approval, then verify that the Controller interaction event at sequence 1 anchors the expected Agent seal before exposing authenticated client services. Reject malformed sequence data and remove permissive numeric coercion.

Add focused coverage for Controller reuse, existing approvals, rejected approvals, invalid sequence data, and mismatched Agent delegation seals.
@kentbull
kentbull requested a review from iFergal July 25, 2026 22:18
@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.75%. Comparing base (9d35b06) to head (b60cf59).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #405   +/-   ##
=======================================
  Coverage   93.75%   93.75%           
=======================================
  Files           1        1           
  Lines          80       80           
  Branches       28       28           
=======================================
  Hits           75       75           
  Misses          3        3           
  Partials        2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/keri/app/clienting.ts Outdated
};
}

/** Verify the local interaction event anchors this exact Agent. */

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.

Is this implying there is a use case to connect a client to multiple agents? If not, sn > 0 was if anything safer as it avoids accidentally connecting to a different one.

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.

At least until a point where we have the idea of moving to a new agent.

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.

No, it doesn't imply multiple agents, it just verifies the actual interaction event rather than making only a sequence number assertion.

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.

The main value here in my mind comes from being explicit and verifying the delegation approval. It would have helped me learn the library when I was first learning the Signify edge client and agent delegation model.

It isn't as hard to learn that these days by using AI.

I can strip this down to just the ConnectionOptions with AbortSignal if we don't believe the delegation anchor seal validation is worth the increased complexity.

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.

Sorry, you're right. But still, I'm unsure on the exact value of the check:

  • clienting.approveDelegation(this.agent!) directly reads agent.pre, agent.sn, agent.said and creates and signs an event locally (no calls to KERIA).
  • Then immediately call _verifyAgentDelegationSeal on the result, which compares against agent.pre, agent.sn, agent.said.

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.

Yeah, that's true. I'll remove that from this PR and later we can consider whether we want to explicitly validate, post delegation completion, the delegator and delegate KELs.

Comment thread src/keri/app/clienting.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants