fix: resolve Makefile merge and centralize .env CRLF handling

[balasiddarthan22]

Resolves merge conflicts in makefiles/Makefile by centralizing .env value reads with automatic CRLF stripping. All env reads now handle Windows line endings transparently.

Test:

1. Create a .env file with CRLF line endings on Windows
2. Run make validate-dev-env — should correctly parse env vars without CRLF corruption
3. Run make ensure-admin-key — should read CONVEX_SELF_HOSTED_ADMIN_KEY without trailing carriage returns

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds an in-process abort-controller registry and integrates it into dataset runs, exposing an authenticated POST /stop endpoint that triggers dataset-scoped cancellation or forces cleanup for orphaned runs. It threads a per-dataset maxRowCount through populate agents, subagent tools, and workflows, enforces maxRowCount at Convex insertion, and adds create/update APIs and schema support. The frontend receives UI for configuring max rows, a Stop button, and a SideSheet cell-detail view; client APIs now support populate(maxRowCount) and stopPopulation. Makefile .env reads are sanitized to strip CRs.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Frontend
  participant BackendAPI
  participant AbortRegistry
  participant BackgroundRunner
  participant ConvexDB
  User->>Frontend: Click "Stop" for dataset
  Frontend->>BackendAPI: POST /stop { datasetId } (auth)
  BackendAPI->>ConvexDB: verify dataset & ownership
  BackendAPI->>AbortRegistry: abortDataset(datasetId)
  AbortRegistry-->>BackgroundRunner: signal.aborted
  BackgroundRunner->>ConvexDB: finaliseRunAsLive or mark failed
  BackendAPI-->>Frontend: 202 (cancellation requested) / 200 (forced cleanup)

Loading

Possibly related PRs

• tinyfish-io/bigset#111: Related changes to populate/investigate flow stopping generation when dataset hits a row limit.
• tinyfish-io/bigset#104: Related backend update flow changes affecting background update behavior and abort handling.
• tinyfish-io/bigset#115: Related UI work for cell expansion and SideSheet behavior.

Suggested reviewers

• simantak-dabhade
• manav-tf

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately summarizes the primary change: a Makefile fix addressing CRLF handling from .env files. It is specific, concise, and clearly reflects the main objective described in the PR.
Description check	✅ Passed	The PR description accurately explains the problem (CRLF line endings in .env files on Windows) and the solution (stripping carriage returns via env_value helper in Makefile recipes).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

makefiles/Makefile (1)

37-37: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Apply the same carriage return sanitization to validation.

The validate-dev-env target extracts environment variable values without stripping carriage returns. On Windows systems with CRLF line endings, this causes placeholder detection to fail. For example, if .env contains CLERK_JWT_ISSUER_DOMAIN=https://your-app.clerk.accounts.dev\r, the extracted value won't match the placeholder string, and validation will incorrectly pass, allowing the placeholder value to propagate to downstream services where it will cause authentication failures.

🔧 Proposed fix

-		value="$$(grep "^$$key=" .env | cut -d= -f2-)"; \
+		value="$$(grep "^$$key=" .env | cut -d= -f2- | tr -d '\r')"; \

🤖 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 `@makefiles/Makefile` at line 37, In the validate-dev-env Makefile target the
extracted variable assignment (the shell variable named value) doesn't strip CR
characters, so on Windows CRLF endings the placeholder check can fail; update
the extraction pipeline that sets the shell variable value (the line using grep
"^$key=" and cut -d= -f2-) to also remove carriage returns (e.g., pipe the
result through tr -d '\r' or sed to strip trailing CR) so the placeholder
comparison in validate-dev-env works correctly on CRLF files.

🤖 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.

Outside diff comments:
In `@makefiles/Makefile`:
- Line 37: In the validate-dev-env Makefile target the extracted variable
assignment (the shell variable named value) doesn't strip CR characters, so on
Windows CRLF endings the placeholder check can fail; update the extraction
pipeline that sets the shell variable value (the line using grep "^$key=" and
cut -d= -f2-) to also remove carriage returns (e.g., pipe the result through tr
-d '\r' or sed to strip trailing CR) so the placeholder comparison in
validate-dev-env works correctly on CRLF files.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d336f124-9d94-40f1-9c84-8c229d78cd2a

📥 Commits

Reviewing files that changed from the base of the PR and between c915897 and 7510b06.

📒 Files selected for processing (1)

• makefiles/Makefile

[simantak-dabhade]

Thanks for catching this. The diagnosis is right: cut -d= -f2- preserves the trailing \r from CRLF .env files, and that can poison CLERK_JWT_ISSUER_DOMAIN when we push it into Convex.

Can we broaden this before merging? The same parsing pattern is used for other .env values in this Makefile too, including PROD, LOCAL_KEYCHAIN_PORT, and CONVEX_SELF_HOSTED_ADMIN_KEY. Convex auth is where this showed up, but the bug is not Convex-specific. For example, PROD=1\r will not compare equal to "1".

I think the right shape is to centralize .env reads and strip \r everywhere, instead of adding tr -d '\r' to only the affected Convex lines. An inline shell helper inside the recipes would fit the current Makefile style, e.g.:

env_value() {
  grep "^293771=" .env | tail -n1 | cut -d= -f2- | tr -d '\r'
}
prod="29377(env_value PROD)"
admin_key="29377(env_value CONVEX_SELF_HOSTED_ADMIN_KEY)"
issuer="29377(env_value CLERK_JWT_ISSUER_DOMAIN)"

Or use an equivalent Make helper if you prefer. Main request: make all Makefile .env reads CRLF-safe so this does not come back under a different variable.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

.github/workflows/secrets-scanner.yml (1)

1-17: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Process violation: file is managed by Terraform in another repository.

The file header (lines 1-4) explicitly states this workflow is managed by Terraform in the github-control repository and instructs developers to make changes there, not here. Direct edits to this file will be overwritten and bypass the intended governance process.

Move this change to https://github.com/tinyfish-io/github-control as indicated in the header.

🤖 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 @.github/workflows/secrets-scanner.yml around lines 1 - 17, This workflow
file is managed by Terraform in the external repo and must not be edited here;
instead, revert any edits to the .github/workflows/secrets-scanner.yml in this
PR and reapply your intended change as a pull request in the github-control
repository where Terraform manages this file; specifically ensure changes to the
"Leaked Secrets Scan" workflow (e.g., the TruffleHog job or the actions/checkout
pin) are made in that repo so they won’t be overwritten by Terraform.

frontend/lib/backend.ts (1)

210-234: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use the typed backend fetch wrapper instead of duplicating per-endpoint fetch logic

The new populate/stopPopulation additions repeat raw fetch + error parsing instead of using a shared typed wrapper in this module, which increases drift risk and weakens type safety across endpoints.

♻️ Suggested refactor

+async function backendJson<T>(
+  path: string,
+  method: "GET" | "POST",
+  token?: string,
+  body?: unknown,
+): Promise<T> {
+  const res = await fetch(`${BACKEND_URL}${path}`, {
+    method,
+    headers: {
+      "Content-Type": "application/json",
+      ...(token ? { Authorization: `Bearer ${token}` } : {}),
+    },
+    ...(body !== undefined ? { body: JSON.stringify(body) } : {}),
+  });
+
+  if (!res.ok) {
+    const payload = await res.json().catch(() => null);
+    throw new Error(payload?.error || `Backend error (${res.status})`);
+  }
+
+  return res.json() as Promise<T>;
+}
@@
 export async function populate(
@@
 ): Promise<PopulateStartResult> {
-  const res = await fetch(`${BACKEND_URL}/populate`, {
-    method: "POST",
-    headers: {
-      "Content-Type": "application/json",
-      Authorization: `Bearer ${token}`,
-    },
-    body: JSON.stringify({ datasetId, datasetName, description, maxRowCount, columns }),
-  });
-
-  if (!res.ok) {
-    const body = await res.json().catch(() => null);
-    const message = body?.error || `Backend error (${res.status})`;
-    throw new Error(message);
-  }
-
-  return res.json();
+  return backendJson<PopulateStartResult>("/populate", "POST", token, {
+    datasetId,
+    datasetName,
+    description,
+    maxRowCount,
+    columns,
+  });
 }
@@
 export async function stopPopulation(
@@
 ): Promise<{ success: boolean }> {
-  const res = await fetch(`${BACKEND_URL}/stop`, {
-    method: "POST",
-    headers: {
-      "Content-Type": "application/json",
-      Authorization: `Bearer ${token}`,
-    },
-    body: JSON.stringify({ datasetId }),
-  });
-
-  if (!res.ok) {
-    const body = await res.json().catch(() => null);
-    const message = body?.error || `Backend error (${res.status})`;
-    throw new Error(message);
-  }
-
-  return res.json();
+  return backendJson<{ success: boolean }>("/stop", "POST", token, { datasetId });
 }

As per coding guidelines, “Implement typed fetch wrapper in lib/backend.ts for calling the Fastify backend with Bearer token authentication.”

Also applies to: 264-284

🤖 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/lib/backend.ts` around lines 210 - 234, The populate function is
using raw fetch/error parsing instead of the module's typed fetch
wrapper—replace the manual fetch in populate with the existing typed wrapper
(the common function used elsewhere in lib/backend.ts for calling the Fastify
backend with Bearer token auth), passing the same POST payload ({ datasetId,
datasetName, description, maxRowCount, columns }) and token so the wrapper
attaches Authorization and handles error parsing/typing; do the same refactor
for stopPopulation so both endpoints use the shared typed wrapper and return the
wrapper's typed response instead of calling fetch directly.

Source: Coding guidelines

🤖 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/index.ts`:
- Line 319: The registration of the dataset is currently done inside
runScheduledUpdateWorkflowInBackground (registerDataset), creating a TOCTOU
window because the background task is already void-ed; move the
registerDataset(datasetId) call out of runScheduledUpdateWorkflowInBackground
and invoke registerDataset(datasetId) immediately before the code path that
voids the background task (the earlier "void" call that starts the background
workflow) so registration happens synchronously prior to voiding, and remove the
redundant registerDataset call from inside
runScheduledUpdateWorkflowInBackground.

In `@frontend/app/dataset/`[id]/page.tsx:
- Around line 236-241: The quota check incorrectly compares the absolute
maxRowCount to usage.remaining; change it to compute the incremental rows needed
= Math.max(0, maxRowCount - currentRowCount) and validate that against
usage.remaining before calling setMaxRowCountSaveError. Locate the validation
that uses maxRowCount and usage.remaining (and the handler that calls
setMaxRowCountSaveError) and replace the direct comparison with the incremental
calculation so reductions or cases where currentRowCount already exceeds
maxRowCount are allowed.

In `@makefiles/Makefile`:
- Around line 88-93: The Makefile contains unresolved git merge markers that
break parsing; remove the conflict markers (<<<<<<<, =======, >>>>>>>) and keep
the centralized env_value helper implementation and its usages so the recipe
sets admin_key via env_value (admin_key="$$(env_value
CONVEX_SELF_HOSTED_ADMIN_KEY)") and similar calls in the ensure-admin-key,
convex-env, and convex-push recipes; ensure no leftover conflict tokens remain
and that env_value is defined once before those recipes.

In `@README.md`:
- Line 288: Typo in the README: change the string "This awesome team is behing
BigSet! We'd love to have you on board :)" to "This awesome team is behind
BigSet! We'd love to have you on board :)" by updating the literal in the README
(search for the exact quoted phrase to locate it).
- Line 285: The contributor image tag (<img
src="https://contrib.rocks/image?repo=tinyfish-io/bigset" />) lacks an alt
attribute; add a concise, descriptive alt attribute (for example:
alt="Contributors to tinyfish-io/bigset") to the <img> element so screen readers
can describe the image, ensuring the README's contributor image includes that
alt text.

---

Outside diff comments:
In @.github/workflows/secrets-scanner.yml:
- Around line 1-17: This workflow file is managed by Terraform in the external
repo and must not be edited here; instead, revert any edits to the
.github/workflows/secrets-scanner.yml in this PR and reapply your intended
change as a pull request in the github-control repository where Terraform
manages this file; specifically ensure changes to the "Leaked Secrets Scan"
workflow (e.g., the TruffleHog job or the actions/checkout pin) are made in that
repo so they won’t be overwritten by Terraform.

In `@frontend/lib/backend.ts`:
- Around line 210-234: The populate function is using raw fetch/error parsing
instead of the module's typed fetch wrapper—replace the manual fetch in populate
with the existing typed wrapper (the common function used elsewhere in
lib/backend.ts for calling the Fastify backend with Bearer token auth), passing
the same POST payload ({ datasetId, datasetName, description, maxRowCount,
columns }) and token so the wrapper attaches Authorization and handles error
parsing/typing; do the same refactor for stopPopulation so both endpoints use
the shared typed wrapper and return the wrapper's typed response instead of
calling fetch directly.

🪄 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: e8b64c19-e6f1-42be-9f82-ee8e31b10e6a

📥 Commits

Reviewing files that changed from the base of the PR and between 7510b06 and 52a47e4.

📒 Files selected for processing (22)

• .github/workflows/secrets-scanner.yml
• README.md
• backend/src/abort-registry.ts
• backend/src/index.ts
• backend/src/mastra/agents/populate.ts
• backend/src/mastra/tools/investigate-tool.ts
• backend/src/mastra/workflows/populate.ts
• backend/src/mastra/workflows/update.ts
• backend/src/pipeline/populate.ts
• frontend/app/dataset/[id]/page.tsx
• frontend/app/dataset/new/page.tsx
• frontend/components/SideSheet.tsx
• frontend/components/ThemeToggle.tsx
• frontend/components/table/DataRow.tsx
• frontend/components/table/DatasetTable.tsx
• frontend/components/table/types.ts
• frontend/convex/datasetRows.ts
• frontend/convex/datasets.ts
• frontend/convex/schema.ts
• frontend/lib/analytics.ts
• frontend/lib/backend.ts
• makefiles/Makefile

[balasiddarthan22]

Thanks for the suggestion! I've replaced the scattered cut/tr patterns with an inline env_value() helper defined in each recipe. All .env reads across validate-dev-env, ensure-admin-key, convex-env, and convex-push now go through it, so CRLF stripping is consistent everywhere.

[paveldudka]

Pr content doesnt match pr description

[paveldudka]

PR comments not addressed. Doesn't look like this PR (only) strips carriage returns from .env values in Makefile

Break it down