Skip to content

Add local mode setup, keychain credentials, and release packaging#131

Merged
simantak-dabhade merged 26 commits into
tinyfish-io:mainfrom
AdamEXu:main
Jun 8, 2026
Merged

Add local mode setup, keychain credentials, and release packaging#131
simantak-dabhade merged 26 commits into
tinyfish-io:mainfrom
AdamEXu:main

Conversation

@AdamEXu

@AdamEXu AdamEXu commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR makes BigSet easier to run locally without storing TinyFish or OpenRouter keys in the root .env, while keeping production deployments on env-based configuration. Local development now starts with a setup flow that verifies service credentials, saves them in the host OS keychain, and only allows data-building workflows once both required services are connected.

What changed

  • Adds local mode detection for PROD != 1, including a local auth adapter so the app can run in development without a live Clerk user session.
  • Adds backend local setup endpoints for TinyFish and OpenRouter credential verification, OpenRouter OAuth exchange, setup status, and credential persistence.
  • Adds a host-side local keychain bridge so Dockerized backend code can read and write this workspace's OS keychain entries without committing secrets.
  • Adds setup and settings UI for connecting or updating TinyFish and OpenRouter credentials, plus a local setup gate that sends incomplete local environments to /setup.
  • Updates TinyFish and OpenRouter consumers to resolve credentials from the keychain in local mode and from env vars in production.
  • Updates make dev to auto-create local env defaults, start and stop the keychain bridge, and run Convex CLI operations through Docker.
  • Adds make build-release and scripts/build-release.mjs to assemble the standalone frontend, bundled backend, Convex runtime files, and a zipped local release artifact.

Notes

  • TINYFISH_API_KEY and OPENROUTER_API_KEY are now production-only requirements; local users enter them in the app and BigSet saves them per workspace.
  • Local pipeline and refresh paths check setup completion before attempting to call TinyFish or OpenRouter.
  • The release artifact intentionally does not bundle the Convex binary; it expects the CLI launcher or another local Convex process.

Testing

Not run, per request.

Copilot AI review requested due to automatic review settings June 5, 2026 19:05
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Ready to act? Review this PR in Change Stack to turn feedback into patch suggestions you can inspect and refine.

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR implements local development mode: it adds an OS-keychain-backed local keychain bridge and HTTP client, per-workspace Convex storage/mutations for credential metadata, backend local-credential resolution/verification (TinyFish/OpenRouter) and OpenRouter OAuth exchange, mode flags and auth stubs for the frontend, setup UI and gating, threads OpenRouter API keys into agents/workflows, updates tooling (Makefile/docker/compose) to start/stop the keychain, and adds a release packaging script.

Sequence Diagram(s)

sequenceDiagram
  participant SetupPage as frontend/app/setup/page.tsx
  participant BackendLocalSetup as backend/src/index.ts:/local-setup
  participant LocalCredentials as backend/src/local-credentials.ts
  participant KeychainClient as backend/src/local-keychain-client.ts
  participant KeychainServer as backend/src/keychain-server.ts
  participant ConvexInternal as frontend/convex/localCredentials.ts

  SetupPage->>BackendLocalSetup: POST /local-setup/tinyfish (apiKey)
  BackendLocalSetup->>LocalCredentials: saveLocalCredential("tinyfish", apiKey, "api_key")
  LocalCredentials->>KeychainClient: setKeychainCredential("tinyfish", apiKey)
  KeychainClient->>KeychainServer: POST /credentials/set (Bearer token, {service, apiKey})
  KeychainServer-->>KeychainClient: { keychainAccount }
  KeychainClient-->>LocalCredentials: { keychainAccount }
  LocalCredentials->>ConvexInternal: upsertInternal(service, keychainAccount, connectionMethod, verifiedAt)
  ConvexInternal-->>LocalCredentials: _id
  LocalCredentials-->>BackendLocalSetup: return LocalSetupStatus
  BackendLocalSetup-->>SetupPage: 200 { LocalSetupStatus }
Loading

Possibly related PRs

  • tinyfish-io/bigset#104: Touches dataset /update and notification flows that this PR also gates for local mode.
  • tinyfish-io/bigset#110: Overlaps on refresh-scheduler and background refresh behavior that this PR conditions on local-setup readiness.
  • tinyfish-io/bigset#15: Related to Convex authz changes — this PR extends authz with a local-mode identity fallback.

Suggested Reviewers

  • simantak-dabhade
  • giaphutran12
  • hwennnn
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

@coderabbitai coderabbitai 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.

Actionable comments posted: 6

🧹 Nitpick comments (6)
scripts/build-release.mjs (1)

22-22: ⚡ Quick win

Copying @esbuild scope may bundle unnecessary platform binaries.

Line 22 includes "@esbuild" in convexRuntimePackages, and lines 169-175 copy the entire node_modules/@esbuild/ directory recursively. This directory contains platform-specific esbuild binaries for all platforms (linux-x64, darwin-arm64, win32-x64, etc.), each ~10MB. The release artifact only needs the binary for the target platform. Copying all platforms unnecessarily bloats the artifact size and could push it over the 50MB limit on line 21.

♻️ Suggested fix to copy only the current platform's esbuild binary

Replace "@esbuild" with a dynamic lookup of the current platform's package:

-const convexRuntimePackages = ["convex", "esbuild", "`@esbuild`", "prettier", "ws"];
+const currentPlatformEsbuild = `@esbuild/${process.platform}-${process.arch}`;
+const convexRuntimePackages = ["convex", "esbuild", currentPlatformEsbuild, "prettier", "ws"];

This ensures only the required platform-specific binary is included in the release artifact.

Also applies to: 169-175

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/build-release.mjs` at line 22, The script currently includes the
entire "`@esbuild`" scope in convexRuntimePackages and then recursively copies
node_modules/@esbuild/, which pulls all platform binaries; change
convexRuntimePackages to omit the scoped package and instead modify the copy
logic that handles "`@esbuild`" (the code around the recursive copy at lines
169-175) to detect the current runtime via process.platform and process.arch (or
equivalent runtime identifiers) and copy only the matching esbuild
binary/subfolder for that platform/arch into the release artifact; ensure the
copy step uses the dynamic platform key (e.g.,
"node_modules/@esbuild/<platform>-<arch>" or the package's binary path) so only
the single required binary is included.
frontend/convex/lib/authz.ts (1)

37-38: 💤 Low value

Consider adding LOCAL_USER_ID to RESERVED_OWNER_IDS for defense-in-depth.

The LOCAL_USER_ID constant is used as a synthetic owner in local mode. While Clerk subjects typically follow a user_<27 chars> format, adding this ID to the reserved set would prevent any theoretical collision.

🛡️ Suggested change
-export const RESERVED_OWNER_IDS: ReadonlySet<string> = new Set(["system"]);
+export const RESERVED_OWNER_IDS: ReadonlySet<string> = new Set([
+  "system",
+  LOCAL_USER_ID,
+]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/convex/lib/authz.ts` around lines 37 - 38, Add the LOCAL_USER_ID
constant to the RESERVED_OWNER_IDS set/array to prevent synthetic local owner
collisions: locate the RESERVED_OWNER_IDS declaration in authz.ts and include
LOCAL_USER_ID among its entries (e.g., push/add the symbol LOCAL_USER_ID),
keeping the existing Clerk pattern checks and usages of LOCAL_USER_ID and
RESERVED_OWNER_IDS unchanged; ensure any type annotations or exports still
compile after the addition.
frontend/app/setup/page.tsx (1)

303-307: 💤 Low value

Identical ternary branches.

Both branches return the same string, making the ternary unnecessary.

♻️ Suggested fix
-            <p className="mt-1 text-xs text-muted">
-              {isTinyFish
-                ? "BigSet verifies the key and stores it in your OS keychain."
-                : "BigSet verifies the key and stores it in your OS keychain."}
-            </p>
+            <p className="mt-1 text-xs text-muted">
+              BigSet verifies the key and stores it in your OS keychain.
+            </p>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/app/setup/page.tsx` around lines 303 - 307, The JSX paragraph uses a
ternary on isTinyFish but both branches are identical; replace the ternary
expression inside the <p className="mt-1 text-xs text-muted"> with the single
literal string ("BigSet verifies the key and stores it in your OS keychain.") to
remove the unnecessary conditional (locate where isTinyFish is used in that JSX
block in page.tsx).
frontend/app/setup/openrouter/callback/page.tsx (1)

22-27: 💤 Low value

Unnecessary setTimeout wrapper.

Setting state inside useEffect is safe and doesn't require deferral. The setTimeout adds no value here.

♻️ Suggested simplification
     if (!code || !verifier) {
-      setTimeout(() => {
-        setError("OpenRouter did not return the expected OAuth values.");
-      }, 0);
+      setError("OpenRouter did not return the expected OAuth values.");
       return;
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/app/setup/openrouter/callback/page.tsx` around lines 22 - 27, The
setTimeout wrapper around setError in the early-return branch is unnecessary;
remove the setTimeout and call setError("OpenRouter did not return the expected
OAuth values.") directly (inside the same conditional that checks !code ||
!verifier) and then return, updating the failing block in page.tsx where
setError is currently invoked inside setTimeout.
frontend/components/settings/LocalCredentialsPanel.tsx (1)

214-240: ⚖️ Poor tradeoff

Consider extracting shared components.

OpenRouterBrand and ApiKeyModal are duplicated between this file and frontend/app/setup/page.tsx. Extracting them into a shared module (e.g., @/components/credentials/) would reduce maintenance burden.

Also applies to: 281-383

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/components/settings/LocalCredentialsPanel.tsx` around lines 214 -
240, Duplicate UI components OpenRouterBrand and ApiKeyModal should be extracted
into a single shared component module: create a new file exporting both
components (preserving props and behavior), replace the duplicated definitions
in LocalCredentialsPanel.tsx and the other file with imports from the shared
module, update any local references to use the imported components, and ensure
tests/types/props remain unchanged so behavior and styling are identical after
removing the duplicates.
frontend/convex/schema.ts (1)

143-153: Run Convex deployment after this schema change.

Please run make convex-push so the new localCredentials table/index is applied in the self-hosted Convex instance.

As per coding guidelines: "After changing any Convex function, schema, or auth config, run make convex-push to deploy the updated code to the self-hosted Convex instance (hot-reload is not enabled)."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/convex/schema.ts` around lines 143 - 153, You added/modified the
Convex schema for the localCredentials table (defined via defineTable and its
"by_service" index); run the Convex deployment to apply this schema change by
executing the project deployment command (make convex-push) against the
self-hosted Convex instance so the new table/index is created and available
(ensure the deployment completes without errors).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/src/env.ts`:
- Around line 43-44: Remove the silent default for BIGSET_LOCAL_WORKSPACE_ID in
env.ts and make it required: replace the fallback expression for
BIGSET_LOCAL_WORKSPACE_ID so that if process.env.BIGSET_LOCAL_WORKSPACE_ID is
undefined the app fails fast (throw or assert) with a clear error message;
update any references to the exported BIGSET_LOCAL_WORKSPACE_ID constant (the
symbol BIGSET_LOCAL_WORKSPACE_ID) to rely on the required env var rather than a
default value.

In `@backend/src/keychain-server.ts`:
- Around line 148-151: The catch block in the keychain request handler currently
maps every exception to HTTP 400; change it to distinguish client (4xx) vs
server (5xx) errors by inspecting the thrown error (e.g. check for
err.statusCode/err.status === 400-499, err.name === 'ValidationError', or
implement an isClientError(err) helper) and call writeJson(res,
<appropriate-status>, { error: message, requestId }) with 4xx for
validation/client faults and 500 for internal failures; also ensure the full
error is logged via console.warn or processLogger alongside requestId before
returning the response.
- Around line 130-137: Validation trims whitespace from body.apiKey but the code
then stores the raw value; before calling credentialEntry(body.service) and
entry.setPassword(...) normalize the API key by assigning a trimmed value (e.g.,
const apiKey = body.apiKey.trim()) and use that trimmed apiKey when calling
entry.setPassword and any further logic; update references where body.apiKey is
used to use the trimmed variable and keep credentialEntry(body.service), entry,
and account usage unchanged.

In `@backend/src/local-credentials.ts`:
- Around line 206-222: Verification network calls can hang because fetch is used
without a timeout; update verifyTinyFishApiKey, verifyOpenRouterApiKey, and
exchangeOpenRouterOAuthCode to use an AbortController with the shared
FETCH_TIMEOUT_MS timeout (as used in web-tools.ts): create an AbortController,
set a setTimeout that calls controller.abort() after FETCH_TIMEOUT_MS, pass
controller.signal to fetch, clear the timeout on success, and surface a clear
timeout/abort-related error when the request is aborted (e.g., wrap or rethrow
as a validation/network error) so these functions never block indefinitely.

In `@backend/src/local-keychain-client.ts`:
- Around line 52-60: The code currently casts the parsed `payload` to T and
returns it without runtime validation, so a 2xx response with empty/malformed
JSON can crash callers; update the response handling in local-keychain-client.ts
to validate `payload` before returning: ensure `payload` is non-null and is an
object (and, if possible, check for expected properties or accept a validator
callback) and throw a descriptive error if validation fails instead of blindly
returning `payload as T` (change the logic around the `payload` variable and the
`return payload as T` to perform this check).

In `@makefiles/Makefile`:
- Line 137: The Makefile starts the keychain via scripts/with-root-env.mjs which
uses child_process.spawn to launch the actual server as a child process, but the
wrapper currently doesn't forward termination signals or write the child's PID,
so stop-local-keychain only kills the wrapper and leaves the server running;
update scripts/with-root-env.mjs to (a) after spawn(child) write the child's PID
to the same .local/keychain.pid file (child.pid) or replace the wrapper PID
write with child.pid, and (b) install listeners for SIGINT and SIGTERM in
with-root-env.mjs that call child.kill(signal) (and await child exit) so signals
propagate and the spawned keychain server process is terminated reliably.

---

Nitpick comments:
In `@frontend/app/setup/openrouter/callback/page.tsx`:
- Around line 22-27: The setTimeout wrapper around setError in the early-return
branch is unnecessary; remove the setTimeout and call setError("OpenRouter did
not return the expected OAuth values.") directly (inside the same conditional
that checks !code || !verifier) and then return, updating the failing block in
page.tsx where setError is currently invoked inside setTimeout.

In `@frontend/app/setup/page.tsx`:
- Around line 303-307: The JSX paragraph uses a ternary on isTinyFish but both
branches are identical; replace the ternary expression inside the <p
className="mt-1 text-xs text-muted"> with the single literal string ("BigSet
verifies the key and stores it in your OS keychain.") to remove the unnecessary
conditional (locate where isTinyFish is used in that JSX block in page.tsx).

In `@frontend/components/settings/LocalCredentialsPanel.tsx`:
- Around line 214-240: Duplicate UI components OpenRouterBrand and ApiKeyModal
should be extracted into a single shared component module: create a new file
exporting both components (preserving props and behavior), replace the
duplicated definitions in LocalCredentialsPanel.tsx and the other file with
imports from the shared module, update any local references to use the imported
components, and ensure tests/types/props remain unchanged so behavior and
styling are identical after removing the duplicates.

In `@frontend/convex/lib/authz.ts`:
- Around line 37-38: Add the LOCAL_USER_ID constant to the RESERVED_OWNER_IDS
set/array to prevent synthetic local owner collisions: locate the
RESERVED_OWNER_IDS declaration in authz.ts and include LOCAL_USER_ID among its
entries (e.g., push/add the symbol LOCAL_USER_ID), keeping the existing Clerk
pattern checks and usages of LOCAL_USER_ID and RESERVED_OWNER_IDS unchanged;
ensure any type annotations or exports still compile after the addition.

In `@frontend/convex/schema.ts`:
- Around line 143-153: You added/modified the Convex schema for the
localCredentials table (defined via defineTable and its "by_service" index); run
the Convex deployment to apply this schema change by executing the project
deployment command (make convex-push) against the self-hosted Convex instance so
the new table/index is created and available (ensure the deployment completes
without errors).

In `@scripts/build-release.mjs`:
- Line 22: The script currently includes the entire "`@esbuild`" scope in
convexRuntimePackages and then recursively copies node_modules/@esbuild/, which
pulls all platform binaries; change convexRuntimePackages to omit the scoped
package and instead modify the copy logic that handles "`@esbuild`" (the code
around the recursive copy at lines 169-175) to detect the current runtime via
process.platform and process.arch (or equivalent runtime identifiers) and copy
only the matching esbuild binary/subfolder for that platform/arch into the
release artifact; ensure the copy step uses the dynamic platform key (e.g.,
"node_modules/@esbuild/<platform>-<arch>" or the package's binary path) so only
the single required binary is included.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7434b80f-b476-457b-bc0e-17eab46af2eb

📥 Commits

Reviewing files that changed from the base of the PR and between 0567771 and 7d993dd.

⛔ Files ignored due to path filters (2)
  • backend/package-lock.json is excluded by !**/package-lock.json
  • frontend/public/logos/engines/tinyfish-wordmark-dark.svg is excluded by !**/*.svg
📒 Files selected for processing (55)
  • .env.example
  • .gitignore
  • AGENTS.md
  • README.md
  • backend/package.json
  • backend/src/clerk-auth.ts
  • backend/src/config/models.ts
  • backend/src/env.ts
  • backend/src/index.ts
  • backend/src/keychain-server.ts
  • backend/src/local-credential-types.ts
  • backend/src/local-credentials.ts
  • backend/src/local-keychain-client.ts
  • backend/src/mastra/agents/investigate.ts
  • backend/src/mastra/agents/populate.ts
  • backend/src/mastra/agents/refresh.ts
  • backend/src/mastra/tools/investigate-tool.ts
  • backend/src/mastra/tools/web-tools.ts
  • backend/src/mastra/workflows/populate.ts
  • backend/src/mastra/workflows/update.ts
  • backend/src/pipeline/schema-inference.ts
  • docker-compose.dev.yml
  • frontend/app/convex-provider.tsx
  • frontend/app/dashboard/page.tsx
  • frontend/app/dashboard/settings/layout.tsx
  • frontend/app/dashboard/settings/models/page.tsx
  • frontend/app/dataset/[id]/page.tsx
  • frontend/app/dataset/new/page.tsx
  • frontend/app/layout.tsx
  • frontend/app/local-setup-gate.tsx
  • frontend/app/page.tsx
  • frontend/app/setup/openrouter/callback/page.tsx
  • frontend/app/setup/page.tsx
  • frontend/app/sign-in/[[...sign-in]]/page.tsx
  • frontend/app/sign-up/[[...sign-up]]/page.tsx
  • frontend/components/LocalUtilityMenu.tsx
  • frontend/components/QuotaBadge.tsx
  • frontend/components/ThemeToggle.tsx
  • frontend/components/settings/LocalCredentialsPanel.tsx
  • frontend/components/settings/SettingsPageLayout.tsx
  • frontend/components/table/use-row-change-detection.ts
  • frontend/convex/auth.config.ts
  • frontend/convex/lib/authz.ts
  • frontend/convex/lib/quota.ts
  • frontend/convex/localCredentials.ts
  • frontend/convex/schema.ts
  • frontend/lib/analytics-provider.tsx
  • frontend/lib/app-auth.tsx
  • frontend/lib/app-mode.ts
  • frontend/lib/backend.ts
  • frontend/lib/openrouter-oauth.ts
  • frontend/next.config.ts
  • frontend/proxy.ts
  • makefiles/Makefile
  • scripts/build-release.mjs
💤 Files with no reviewable changes (2)
  • .env.example
  • AGENTS.md

Comment thread backend/src/env.ts Outdated
Comment thread backend/src/keychain-server.ts
Comment on lines +148 to +151
} catch (err) {
const message = err instanceof Error ? err.message : "Keychain bridge failed.";
console.warn({ requestId, err }, "Local keychain bridge request failed");
writeJson(res, 400, { error: message, requestId });

@coderabbitai coderabbitai Bot Jun 5, 2026

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not map all handler failures to HTTP 400.

Line 151 returns 400 for every exception, including internal keychain/runtime failures. That masks bridge outages as client input errors and degrades retry/alert behavior. Separate validation errors (4xx) from internal failures (5xx).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/keychain-server.ts` around lines 148 - 151, The catch block in
the keychain request handler currently maps every exception to HTTP 400; change
it to distinguish client (4xx) vs server (5xx) errors by inspecting the thrown
error (e.g. check for err.statusCode/err.status === 400-499, err.name ===
'ValidationError', or implement an isClientError(err) helper) and call
writeJson(res, <appropriate-status>, { error: message, requestId }) with 4xx for
validation/client faults and 500 for internal failures; also ensure the full
error is logged via console.warn or processLogger alongside requestId before
returning the response.

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.

@AdamEXu take a look, is this valid.

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.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Comment thread backend/src/local-credentials.ts
Comment thread backend/src/local-keychain-client.ts Outdated
Comment thread makefiles/Makefile Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@simantak-dabhade simantak-dabhade 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.

This, also wanted to check in and ask where the code for the npx bigset command is at. Other than that, awesome stuff!

Comment thread backend/src/index.ts Outdated

await backfillDatasetRefreshSettings(fastify.log);
const refreshScheduler = startLocalRefreshScheduler(fastify.log);
let refreshScheduler: ReturnType<typeof setInterval> | null = null;

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.

Explain/remove

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Line 288: The contributors image tag is missing an alt attribute which breaks
accessibility and triggers markdownlint MD045; update the <img
src="https://contrib.rocks/image?repo=tinyfish-io/bigset" /> element to include
a meaningful alt (e.g., alt="Contributors to tinyfish-io/bigset" or similar) so
screen readers and linters get descriptive text while keeping the same src URL.
- Line 291: Fix the typo and polish the contributor sentence by changing
"behing" to "behind" and optionally rephrasing the line "This awesome team is
behing BigSet! We'd love to have you on board :)" to a cleaner variant such as
"This awesome team is behind BigSet — we'd love to have you on board!" so the
README.md contributor sentence reads correctly and more professionally.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6c38dccd-dfe1-4b93-8405-ca40e20e4548

📥 Commits

Reviewing files that changed from the base of the PR and between 875cda1 and 7f074ec.

📒 Files selected for processing (1)
  • README.md

@coderabbitai coderabbitai 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.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Line 288: The contributors image tag is missing an alt attribute which breaks
accessibility and triggers markdownlint MD045; update the <img
src="https://contrib.rocks/image?repo=tinyfish-io/bigset" /> element to include
a meaningful alt (e.g., alt="Contributors to tinyfish-io/bigset" or similar) so
screen readers and linters get descriptive text while keeping the same src URL.
- Line 291: Fix the typo and polish the contributor sentence by changing
"behing" to "behind" and optionally rephrasing the line "This awesome team is
behing BigSet! We'd love to have you on board :)" to a cleaner variant such as
"This awesome team is behind BigSet — we'd love to have you on board!" so the
README.md contributor sentence reads correctly and more professionally.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6c38dccd-dfe1-4b93-8405-ca40e20e4548

📥 Commits

Reviewing files that changed from the base of the PR and between 875cda1 and 7f074ec.

📒 Files selected for processing (1)
  • README.md
🛑 Comments failed to post (2)
README.md (2)

288-288: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add alt text to the contributors image.

The <img> tag is missing alt, which hurts accessibility and triggers markdownlint MD045.

Suggested patch
-  <img src="https://contrib.rocks/image?repo=tinyfish-io/bigset" />
+  <img src="https://contrib.rocks/image?repo=tinyfish-io/bigset" alt="BigSet contributors" />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  <img src="https://contrib.rocks/image?repo=tinyfish-io/bigset" alt="BigSet contributors" />
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 288-288: Images should have alternate text (alt text)

(MD045, no-alt-text)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` at line 288, The contributors image tag is missing an alt
attribute which breaks accessibility and triggers markdownlint MD045; update the
<img src="https://contrib.rocks/image?repo=tinyfish-io/bigset" /> element to
include a meaningful alt (e.g., alt="Contributors to tinyfish-io/bigset" or
similar) so screen readers and linters get descriptive text while keeping the
same src URL.

291-291: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix typo in contributor sentence.

“behing” should be “behind” (and this line reads more polished with a small rewrite).

Suggested patch
-^ This awesome team is behing BigSet! We'd love to have you on board :) 
+^ This awesome team is behind BigSet! We'd love to have you on board :)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

^ This awesome team is behind BigSet! We'd love to have you on board :)
🧰 Tools
🪛 LanguageTool

[style] ~291-~291: Consider using a more formal and expressive alternative to ‘awesome’.
Context: ...epo=tinyfish-io/bigset" /> ^ This awesome team is behing BigSet! We'd love to hav...

(AWESOME)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` at line 291, Fix the typo and polish the contributor sentence by
changing "behing" to "behind" and optionally rephrasing the line "This awesome
team is behing BigSet! We'd love to have you on board :)" to a cleaner variant
such as "This awesome team is behind BigSet — we'd love to have you on board!"
so the README.md contributor sentence reads correctly and more professionally.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/build-release-artifacts.yml:
- Line 75: The current workflow interpolates `${{ github.event.release.tag_name
|| inputs.release_tag }}` directly into the `run` shell command (the `gh release
upload "${{ github.event.release.tag_name || inputs.release_tag }}" "dist/${{
env.ARTIFACT_NAME }}" --clobber` invocation), which can lead to shell injection;
instead expose that value as a workflow environment variable (e.g. set
RELEASE_TAG: ${{ github.event.release.tag_name || inputs.release_tag }} under
`env:` or via `echo "RELEASE_TAG=..." >> $GITHUB_ENV`) and then use the safe
shell variable `$RELEASE_TAG` in the `run` step, and add a simple
validation/sanitization step (check non-empty and reject/escape quotes or
metacharacters) before calling `gh release upload`; apply the same change to the
other occurrence mentioned (the second `run` that interpolates the release tag).
- Around line 42-43: The Checkout step using actions/checkout@v4 currently
leaves credentials persisted; update the Checkout step (the actions/checkout@v4
usage) to disable credential persistence by adding persist-credentials: false so
the runner does not retain the checkout token in git config before later
artifact/release upload steps.
- Around line 43-46: The workflow currently references mutable tag refs for
third-party actions (actions/checkout@v4, actions/setup-node@v4,
actions/upload-artifact@v4); replace those tag refs with the corresponding
immutable commit SHAs for each action to mitigate supply-chain risk. Locate the
steps that use actions/checkout, actions/setup-node, and actions/upload-artifact
in the workflow and update their uses: values should be
actions/checkout@<commit-sha>, actions/setup-node@<commit-sha>, and
actions/upload-artifact@<commit-sha> respectively (obtain the exact SHAs from
each action's GitHub repo or marketplace), then commit the updated workflow.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9b1bb5be-c6f1-4aa2-9840-def0c17bed0f

📥 Commits

Reviewing files that changed from the base of the PR and between d918c06 and d596115.

📒 Files selected for processing (2)
  • .github/workflows/build-release-artifacts.yml
  • scripts/build-release.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/build-release.mjs

Comment thread .github/workflows/build-release-artifacts.yml Outdated
Comment thread .github/workflows/build-release-artifacts.yml Outdated
Comment thread .github/workflows/build-release-artifacts.yml Outdated

@simantak-dabhade simantak-dabhade 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.

LGTM, fix final code rabbit issues.

@giaphutran12

Copy link
Copy Markdown
Collaborator

Rechecked current head because latest CodeRabbit review text was stale.

Current state:

  • CodeRabbit, TruffleHog, and OSV all pass.
  • Release workflow now uses RELEASE_TAG as a shell env var, validates it before upload, disables checkout credential persistence, and pins checkout/setup-node/upload-artifact to commit SHAs.
  • git diff --check origin/main...HEAD passes.
  • node scripts/build-release.mjs passes locally; artifact built at 34.4 MB.

@MMeteorL @simantak-dabhade I think this is in good shape after the final release-workflow fixes. Could you give it one more human sanity pass if you want before merge?

@simantak-dabhade simantak-dabhade 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.

LGTM

@simantak-dabhade simantak-dabhade merged commit e969b19 into tinyfish-io:main Jun 8, 2026
3 checks passed
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.

4 participants