Skip to content

type error fixes + cleanup#218

Open
bmdavis419 wants to merge 1 commit into
mainfrom
davis/rwx
Open

type error fixes + cleanup#218
bmdavis419 wants to merge 1 commit into
mainfrom
davis/rwx

Conversation

@bmdavis419
Copy link
Copy Markdown
Collaborator

@bmdavis419 bmdavis419 commented Mar 20, 2026

Greptile Summary

This PR adds a new RWX-based CI/CD pipeline (.rwx/ + .captain/), wires up a lint:all script through Turbo, and fixes a TypeScript error in snapshot.ts by replacing import.meta.main with a manual URL comparison. The infrastructure changes are additive and don't touch existing runtime behavior, but there are a few things to address:

  • Tests skipped on PRs.rwx/pr.yml runs lint and type-check but never the test suite. Broken tests will only surface after merging to main.
  • Lockfile unenforced in CI--frozen-lockfile is commented out in .rwx/ci.yml, making dependency installs non-reproducible until the TODO is resolved.
  • snapshot.ts deviates from Bun-first style — swapping import.meta.main for a process.argv[1] URL comparison works in practice but goes against AGENTS.md, and is fragile with special-character paths. Fixing the tsconfig to include bun-types is likely the cleaner solution.
  • No plan .md files were found in the repository.

Confidence Score: 3/5

  • Safe to merge with low risk to existing runtime behavior, but the PR workflow gap means tests aren't gating PRs.
  • All existing features are unaffected — the changes are additive (CI config, new Turbo task) or isolated to a sandbox helper. The main concerns are the missing test gate in pr.yml and the disabled lockfile check, both of which are process/CI issues rather than runtime regressions.
  • .rwx/pr.yml (missing test step) and .rwx/ci.yml (disabled --frozen-lockfile)

Important Files Changed

Filename Overview
apps/sandbox/src/snapshot.ts Replaced Bun-native import.meta.main with a manual process.argv[1] URL comparison; functional in common cases but fragile with special-character paths and deviates from the project's Bun-first style guide.
.rwx/pr.yml New PR CI workflow — missing a test step, so broken tests won't be caught before merging to main.
.rwx/ci.yml New main-branch CI workflow; --frozen-lockfile is disabled with a TODO, making dependency installs non-reproducible until resolved.
.rwx/main.yml New orchestration file that fans out CI runs for x86_64 and arm64 on pushes to main — no issues found.
.captain/config.yml New Captain test-suite config for retry support — straightforward, no issues.
package.json Added lint:all script wired to turbo lint — clean, additive change.
turbo.json Added lint task with dependsOn: ["^lint"] to match the new lint:all script — no issues.

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/sandbox/src/snapshot.ts
Line: 56-58

Comment:
**Deviation from Bun-idiomatic approach**

`import.meta.main` is the Bun-native way to check if a module is the entry point (documented in Bun's APIs). The new code uses `process.argv[1]`, which is a Node.js-style API, and AGENTS.md says to prefer Bun APIs over Node equivalents.

If the original code had a TypeScript type error, the simpler fix would be to ensure the package's `tsconfig.json` has `types: ["bun-types"]` (since `@types/bun` is already a root dev dependency). The manual URL comparison is also fragile — `encodeURI` and Bun's internal URL encoding for `import.meta.url` can differ for paths containing characters like spaces or brackets, causing `isMainModule` to be `false` even when run directly.

```suggestion
if (import.meta.main) {
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .rwx/ci.yml
Line: 49

Comment:
**Lockfile enforcement disabled**

`--frozen-lockfile` is commented out with a TODO. Without it, `bun install` can silently update packages during CI, making builds non-reproducible and potentially introducing unexpected dependency changes. This should be re-enabled as soon as the packages are updated.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .rwx/pr.yml
Line: 68-73

Comment:
**Tests not run on PRs**

The PR workflow ends with the `check` (type-check) step and never calls the `test` task. By contrast, `.rwx/ci.yml` does run tests on the main branch. This means broken tests can be merged through a PR without being caught until after they land on `main`.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: "rwx added in"

Greptile also left 1 inline comment on this PR.

Context used:

  • Context used - AGENTS.md (source)

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
better-context Error Error Mar 20, 2026 6:32am

Request Review

Copy link
Copy Markdown
Collaborator Author

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

@bmdavis419 bmdavis419 changed the title rwx added in type error fixes + cleanup Mar 20, 2026
@bmdavis419 bmdavis419 marked this pull request as ready for review March 20, 2026 06:31
Comment thread .rwx/pr.yml
Comment on lines +68 to +73
- key: check
use: install-mintlify
run: bun run check:all
env:
PUBLIC_CONVEX_URL: https://example.com
PUBLIC_CLERK_PUBLISHABLE_KEY: abc123
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.

P2 Tests not run on PRs

The PR workflow ends with the check (type-check) step and never calls the test task. By contrast, .rwx/ci.yml does run tests on the main branch. This means broken tests can be merged through a PR without being caught until after they land on main.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .rwx/pr.yml
Line: 68-73

Comment:
**Tests not run on PRs**

The PR workflow ends with the `check` (type-check) step and never calls the `test` task. By contrast, `.rwx/ci.yml` does run tests on the main branch. This means broken tests can be merged through a PR without being caught until after they land on `main`.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

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