Skip to content

chore: RivetKit perf fixes#4523

Open
NathanFlurry wants to merge 1 commit intomainfrom
rivetkit-perf-fixes
Open

chore: RivetKit perf fixes#4523
NathanFlurry wants to merge 1 commit intomainfrom
rivetkit-perf-fixes

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app bot commented Mar 29, 2026

🚅 Deployed to the rivet-pr-4523 environment in rivet-frontend

Service Status Web Updated (UTC)
mcp-hub ✅ Success (View Logs) Web Mar 31, 2026 at 2:47 am
website ❌ Build Failed (View Logs) Web Mar 31, 2026 at 2:46 am
kitchen-sink ❌ Build Failed (View Logs) Web Mar 31, 2026 at 2:46 am
frontend-cloud ❌ Build Failed (View Logs) Web Mar 30, 2026 at 9:59 pm
frontend-inspector ❌ Build Failed (View Logs) Web Mar 30, 2026 at 9:59 pm
ladle ❌ Build Failed (View Logs) Web Mar 30, 2026 at 9:58 pm

Copy link
Copy Markdown
Member Author

NathanFlurry commented Mar 29, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link
Copy Markdown

claude bot commented Mar 29, 2026

PR Review

Note: The PR title "chore: RivetKit perf fixes" does not match the actual scope — this is a large new feature adding Agent OS integration (new agent-os/ packages, rivetkit/agent-os subpath, website pages, examples, docs). Please update the title and fill in the PR description so reviewers understand the changes.


Bugs / Correctness

Sequence number race condition in persistSessionEvent (rivetkit-typescript/packages/rivetkit/src/agent-os/actor/session.ts)

The SELECT MAX + INSERT pattern is not atomic. If two session events arrive concurrently, both reads can see the same max_seq and produce duplicate sequence numbers. Use an AUTOINCREMENT primary key on seq and omit it from the INSERT instead.

SQL wildcard injection in getChildEntries (rivetkit-typescript/packages/rivetkit/src/agent-os/fs/sqlite-vfs.ts)

The query will misfire for paths containing % or _ (SQLite LIKE wildcards). Escape special characters in prefix before interpolating, or use a range comparison.

Incorrect mount config in agent-os/examples/quickstart/src/file-system.ts

Uses { type: "s3", path: "/data", bucket: "my-bucket" } but MountConfig requires a driver property, not type/bucket. The correct filesystem.ts example exists right next to it — delete file-system.ts or correct it.

lstat is not a true lstat (rivetkit-typescript/packages/rivetkit/src/agent-os/fs/sqlite-vfs.ts)

The lstat implementation calls backend.stat(p) which follows symlinks, violating the POSIX contract that lstat should return information about the symlink itself, not its target.

Silent error swallowing in spawn (rivetkit-typescript/packages/rivetkit/src/agent-os/actor/process.ts)

The .catch(() => {}) on waitProcess discards all errors, not just the expected "killed during dispose" case. Use a more targeted catch (e.g. check error message/type) so unexpected failures are not silently hidden.


Development Artifacts That Should Not Be Committed

Debug test scripts in agent-os/examples/quickstart/test2.ts, test3.ts, and test-signal-exit.ts are investigation scripts that reference a private package (@mariozachner/pi-coding-agent), use inconsistent indentation, and contain raw console.log debugging output. Remove before merge.

Binary blob (agent-os/packages/sandbox/simple-icons-16.14.0.tgz) — Committing a tarball inflates git history permanently. Fetch from npm at build time or add to .gitignore.


Broken Dependencies

Several package.json files use link: paths to directories outside the monorepo:

  • agent-os/packages/google-drive and agent-os/packages/s3 both reference "@secure-exec/core": "link:../../../../secure-exec-1/packages/core"
  • examples/agent-os links to agent-os-registry/software/common, agent-os-registry/software/git, and agent-os/packages/agent-pi

These assume sibling-repo checkouts that will not exist in CI or for contributors. Bring these packages into the monorepo, publish to npm, or exclude them from the workspace until ready.


Duplicate / Redundant Files

The quickstart examples directory has near-duplicate pairs: file-system.ts/filesystem.ts, network.ts/networking.ts, agent-session.ts/sessions.ts, expose-tools.ts/tools.ts. Pick one canonical version per topic and remove the others.


Known Gap That Needs Visibility

In-memory filesystem is lost on sleep/wake (rivetkit-typescript/packages/rivetkit/src/agent-os/actor/index.ts):

The TODO comment confirms the working directory (/home/user) uses an in-memory VFS and will be wiped on every sleep/wake cycle. Users relying on filesystem persistence will lose data silently. Document this prominently in the Agent OS docs until the persistent backend is implemented.


Minor Points

  • CORS wildcard on preview proxy (actor/preview.ts): Access-Control-Allow-Origin: * grants any origin access to running VM processes. Worth a comment confirming this is the intended security posture for preview URLs.
  • Token generation modular bias (actor/preview.ts generateToken): bytes[i]! % 36 has slight bias (256 % 36 != 0). Entropy is still ~164 bits so not a practical issue, but rejection sampling would be cleaner.
  • link() throws ENOSYS (sqlite-vfs.ts): Hard links are silently unsupported. Consider documenting this in the VFS type or throwing a more descriptive error so callers know it is a limitation rather than a bug.
  • agentOs driver tests correctly use describe.skipIf(driverTestConfig.skip?.agentOs) — the skip mechanism is properly wired.
  • PR description checklist is completely empty. For a PR of this scope, filling in the type of change and test plan helps reviewers.

@NathanFlurry NathanFlurry changed the title feat: US-001 - Scaffold module structure and package exports chore: RivetKit perf fixes Mar 29, 2026
@NathanFlurry NathanFlurry marked this pull request as ready for review March 30, 2026 11:38
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.

1 participant