Skip to content

fix(ssx): inject dummy preclaimop action for catch-all sessions#412

Open
highskore wants to merge 2 commits intomainfrom
fix/preclaimops-no-action-sessions
Open

fix(ssx): inject dummy preclaimop action for catch-all sessions#412
highskore wants to merge 2 commits intomainfrom
fix/preclaimops-no-action-sessions

Conversation

@highskore
Copy link
Copy Markdown
Contributor

Summary

  • Sessions without explicit actions previously only got sudoAction (catch-all) and missed the dummy preclaimop action, causing bundle simulation failures
  • Now the dummy preclaimop action (0x0000...0001 / 0x69123456 with sudo policy) is always registered — for catch-all sessions it's added explicitly alongside sudoAction
  • Removed an overly-restrictive session.actions?.length guard that was added in prepareTransactionAsIntent (it's no longer needed since the action is always present)

Root cause

When a session has no explicit actions, getSessionData uses [sudoAction] only — injectedActions (which includes the dummy preclaimop action) was only appended for explicit-action sessions. So verifyExecutions: true + no actions → preclaimop injected into the mandate, but the session didn't have the action registered → filler's eth_estimateGas reverted.

Test plan

  • bun run test -- src/modules/validators/smart-sessions.test.ts — updated tests verify 2 actions for no-action sessions (sudoAction + dummy)
  • bun run test -- src/execution/utils.test.ts — all 14 tests pass
  • Re-run ssx-preclaimop-enable integration tests in orchestrator-test

🤖 Generated with Claude Code

Sessions without explicit actions previously only received the sudoAction
(catch-all) and missed the dummy preclaimop action. This meant the filler
couldn't trigger verifyExecution in ENABLE mode for those sessions, causing
bundle simulation failures.

Now the dummy preclaimop action is always registered — both for
explicit-action sessions (via injectedActions) and for catch-all sessions
(explicitly added alongside sudoAction).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@greg-rhinestone greg-rhinestone left a comment

Choose a reason for hiding this comment

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

🤖 Greg · ✅ LGTM

Fixes catch-all sessions missing the dummy preclaimop action — the false branch of the actions ternary now appends the dummy alongside sudoAction. The mapActions extraction cleanly eliminates the duplicated inline mapping logic without changing behaviour for explicit-action sessions.

Risk: Low — targeted fix with good test coverage; no shared state or async paths touched.

SMART_SESSIONS_FALLBACK_TARGET_FLAG is 0x0000000000000000000000000000000000000001
and SmartSession's _enforceActionPolicies explicitly reverts with InvalidTarget()
when the execution target equals this flag. Using SHA256 precompile (0x...0002)
avoids the check while keeping the same "harmless call" semantics.

Also adds extraHeaders support to getIntentRoute and passes the new-path-generation
feature flag when preClaimExecutions are present.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@greg-rhinestone greg-rhinestone left a comment

Choose a reason for hiding this comment

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

🤖 Greg · 🔄 Re-review · ⚠️ Needs changes

Two changes since last review: DUMMY_PRECLAIMOP_TARGET 0x0001 → 0x0002 is a correct fix (0x0001 is SMART_SESSIONS_FALLBACK_TARGET_FLAG and gets rejected as InvalidTarget by the SmartSession contract); the x-feature-flags header injection in utils.ts needs a look.

Comment thread src/execution/utils.ts
const intentRoute = await orchestrator.getIntentRoute(metaIntent)
const routeHeaders =
Object.keys(preClaimExecutions).length > 0
? { 'x-feature-flags': 'new-path-generation=new' }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

x-feature-flags was removed from the orchestrator in favour of x-api-version versioning (the PathGenerationMode / featureFlags middleware is gone). If orchestrator#1310 has merged, this header is silently ignored and preClaimExecutions flows would get default Alps path generation regardless of this flag. Should this be { 'x-api-version': '<blanc-version>' } instead, or is there a specific orchestrator version still reading this flag?

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.

2 participants