Skip to content

Report marketplace download via TemplateMarket edge function#142

Merged
CarmenDou merged 3 commits into
mainfrom
feat/report-download-to-templatemarket
May 27, 2026
Merged

Report marketplace download via TemplateMarket edge function#142
CarmenDou merged 3 commits into
mainfrom
feat/report-download-to-templatemarket

Conversation

@CarmenDou
Copy link
Copy Markdown
Contributor

@CarmenDou CarmenDou commented May 27, 2026

Summary

  • reportMarketplaceDownload now POSTs {slug} to https://p8n7m7ci.us-east.insforge.app/functions/report-download (TemplateMarket project's new public proxy) instead of ${apiUrl}/templates/v1/<slug>/downloads on cloud-backend.
  • Drops the apiUrl parameter — marketplace is single-tenant so the URL is a constant, not derived from the per-user backend host.
  • Edge function internally calls increment_template_download RPC with the auto-injected API_KEY, so the CLI ships no anon key.
  • Sync workflow on insforge-templates is being repointed in Point sync workflow at TemplateMarket edge function insforge-templates#46. Cloud-backend's template.controller.ts + migration 073 will drop in a follow-up.

Test plan

  • npx vitest run src/commands/create.marketplace.test.ts — 9/9 pass
  • Manual smoke after merge: npx @insforge/cli create test-project --marketplace e-commerce and confirm template_downloads.count increments by 1 in the TemplateMarket project (verified out-of-band: POST /functions/report-download {"slug":"e-commerce"} returned {count:2} and DB row shows count=2).

Backend status (already live)

  • TemplateMarket project (c6366c10-…): tables, 4 RPCs, sync-templates + report-download edge functions all deployed.
  • report-download smoke-tested: existing slug 200/count, unknown slug 200/{count:0,unknown:true}, bad format 400, GET 405.

Summary by cubic

Report marketplace downloads via the TemplateMarket edge function. Removes apiUrl, sends {slug} to a fixed URL (overridable via INSFORGE_MARKETPLACE_REPORT_URL), and bounds the request to 5s to avoid hangs.

  • Refactors
    • POST {slug} as JSON to https://p8n7m7ci.us-east.insforge.app/functions/report-download; override via INSFORGE_MARKETPLACE_REPORT_URL.
    • Remove apiUrl from reportMarketplaceDownload and its call site.
    • Add 5s AbortSignal.timeout to prevent the CLI from hanging when the endpoint is unreachable.
    • Edge function handles auth/RPC; keep best-effort, non-blocking behavior.

Written for commit 3c2a947. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

  • Refactor

    • Simplified marketplace template download reporting by removing the extra parameter and consolidating reporting to a single fixed endpoint with a standardized JSON request format. No visible change to end-user behavior.
  • Tests

    • Updated marketplace download report tests to match the new endpoint and request contract; network errors and non-2xx responses remain swallowed.

Review Change Stack

Note

Report marketplace downloads via a fixed TemplateMarket edge function endpoint

  • Changes reportMarketplaceDownload in create.ts to POST the template slug as JSON to a fixed endpoint instead of deriving the URL from apiUrl.
  • Adds a MARKETPLACE_REPORT_URL constant defaulting to https://p8n7m7ci.us-east.insforge.app/functions/report-download, overridable via INSFORGE_MARKETPLACE_REPORT_URL.
  • Adds a 5-second AbortSignal.timeout to bound request duration; network errors and non-2xx responses continue to be swallowed.
  • Behavioral Change: callers no longer pass apiUrl to reportMarketplaceDownload; the endpoint is always resolved internally.

Macroscope summarized 3c2a947.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

Walkthrough

The marketplace download reporting function is refactored to use a fixed HTTP endpoint and simplified JSON body contract. The function signature is simplified by removing the apiUrl parameter, and call sites and tests are updated to POST { slug } to the fixed report URL.

Changes

Marketplace Download Reporting

Layer / File(s) Summary
Marketplace download reporting contract and tests
src/commands/create.ts, src/commands/create.marketplace.test.ts
reportMarketplaceDownload is simplified to accept only slug, introduces MARKETPLACE_REPORT_URL constant for the fixed endpoint, and posts { slug } as JSON instead of constructing a URL from apiUrl. Tests validate the new endpoint, JSON body format, slug verbatim handling, and error/non-2xx response swallowing behavior.
Marketplace install call site
src/commands/create.ts
The marketplace-success reporting call now invokes reportMarketplaceDownload with only opts.marketplace, removing the apiUrl argument and its fallback logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • InsForge/CLI#137: Both PRs touch the same marketplace download counter logic, reworking the contract from URL-path-based with URL-encoded slug to fixed endpoint with JSON body and simplified function signature.

Suggested reviewers

  • jwfing

Poem

🐇 I hopped to the endpoint bright,
I carried the slug, no encode in sight,
One JSON post, neat and small,
The marketplace hears my call,
A bunny's report, done just right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: migrating marketplace download reporting to use a TemplateMarket edge function instead of the previous cloud-backend endpoint.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/report-download-to-templatemarket

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

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

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 27, 2026

Greptile Summary

This PR refactors reportMarketplaceDownload to POST { slug } as a JSON body to a fixed TemplateMarket edge-function URL instead of constructing a path from the per-user apiUrl, removing the apiUrl parameter entirely.

  • reportMarketplaceDownload now targets https://p8n7m7ci.us-east.insforge.app/functions/report-download (overridable via INSFORGE_MARKETPLACE_REPORT_URL), with the edge function handling auth and the RPC call so no anon key is needed in the CLI.
  • AbortSignal.timeout(5000) is added to bound the fire-and-forget call and prevent the Node event loop from hanging up to OS socket timeout when the endpoint is unreachable.
  • Call site in the create command is simplified to void reportMarketplaceDownload(opts.marketplace as string).

Confidence Score: 5/5

Safe to merge — straightforward endpoint swap with no auth secrets in the CLI, a bounded timeout preventing process hangs, and all 9 existing tests passing.

The production code path is clean: the endpoint is a constant (env-overridable), the request body is well-formed, errors are swallowed as intended, and the new AbortSignal.timeout closes a real hang risk. The only gap is a test-environment fragility where the URL assertion breaks if INSFORGE_MARKETPLACE_REPORT_URL is set before the module is imported, and the env-override path has no test coverage — neither affects runtime behaviour.

No files require special attention for merging. The test file's hardcoded URL assumption is worth addressing in a follow-up.

Important Files Changed

Filename Overview
src/commands/create.ts Drops apiUrl parameter, fixes endpoint to a module-level constant (env-overridable), sends { slug } as JSON body, adds AbortSignal.timeout(5000) to prevent the CLI from hanging on an unreachable endpoint — all correct and well-documented.
src/commands/create.marketplace.test.ts Tests updated to match new signature and endpoint; hardcoded default URL is fragile if INSFORGE_MARKETPLACE_REPORT_URL is set in the environment at import time, and the override path is untested.

Sequence Diagram

sequenceDiagram
    participant CLI as CLI (create command)
    participant GH as GitHub (template repo)
    participant TM as TemplateMarket edge fn<br/>(report-download)
    participant DB as TemplateMarket DB<br/>(template_downloads)

    CLI->>GH: downloadGitHubTemplate(slug)
    GH-->>CLI: downloaded: true/false

    alt "downloaded === true"
        CLI-)TM: "void POST /functions/report-download<br/>{ slug } — fire-and-forget (5 s timeout)"
        TM->>DB: increment_template_download(slug)
        DB-->>TM: "{ count }"
        TM-->>CLI: "200 { count }"
    end
Loading

Reviews (3): Last reviewed commit: "fix(cli): bound marketplace download fet..." | Re-trigger Greptile

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@src/commands/create.ts`:
- Around line 750-756: The fetch in reportMarketplaceDownload currently has no
timeout and can hang; update the fetch call that posts to MARKETPLACE_REPORT_URL
inside reportMarketplaceDownload to include a bounded abort signal by adding
signal: AbortSignal.timeout(3000) to the options object passed to fetch so the
request is aborted after 3s; keep the existing try/catch so the abort error is
handled similarly to other failures.
🪄 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: b37d67db-61c9-45b4-8c45-6ffccf5f2dc2

📥 Commits

Reviewing files that changed from the base of the PR and between 40e9450 and aeb300d.

📒 Files selected for processing (2)
  • src/commands/create.marketplace.test.ts
  • src/commands/create.ts

Comment thread src/commands/create.ts
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/commands/create.ts
Copy link
Copy Markdown
Member

@jwfing jwfing left a comment

Choose a reason for hiding this comment

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

Code Review — #142

Summary: Small, focused PR that redirects marketplace download reporting from the per-user cloud-backend to a fixed TemplateMarket edge function, dropping apiUrl, sending {slug} in the JSON body, and adding a 5-second abort timeout to prevent CLI hangs.


Requirements context

No /docs/superpowers/ directory exists in this repo. Review is assessed against the PR description alone. The stated goals are: (1) POST {slug} to the fixed TemplateMarket edge function URL, (2) drop the apiUrl parameter, (3) add an env override, (4) add a 5 s timeout. All four are implemented.


Findings

Critical

(none)


Suggestion

[Software Engineering] No test coverage for AbortSignal.timeout being wired
src/commands/create.marketplace.test.ts:28-33

The happy-path test uses toMatchObject which does a partial match — it verifies method, headers, and body but not signal. The 5-second timeout is the PR's primary guard against the CLI hanging when the endpoint is unreachable (the PR description explicitly calls this out). It would be worth asserting something like:

expect(init.signal).toBeInstanceOf(AbortSignal);

in the existing happy-path case, so a future refactor that accidentally drops the signal doesn't go unnoticed.


Information

[Software Engineering] MARKETPLACE_REPORT_URL is read once at module load time
src/commands/create.ts:741-743

const MARKETPLACE_REPORT_URL =
  process.env.INSFORGE_MARKETPLACE_REPORT_URL ??
  'https://p8n7m7ci.us-east.insforge.app/functions/report-download';

process.env.INSFORGE_MARKETPLACE_REPORT_URL is snapshotted when the module is first imported. The existing tests correctly stub fetch rather than the env var, so this is fine today. Future test authors who try to override the env var after import will be surprised — a one-liner comment like // read once at import; stub fetch (not this const) in tests would save them the head-scratch.

[Security] No new concerns
The slug passes through SAFE_MARKETPLACE_SLUG (/^[a-z0-9][a-z0-9-]{0,99}$/) at action entry — before auth and well before this function is called. JSON.stringify gives natural escaping in the body. No anon key is shipped; auth is handled inside the edge function. No new dependencies.

[Performance] No new concerns
Fire-and-forget (void-awaited), single HTTP call, 5 s hard ceiling — no event-loop blocking, no N+1.


Verdict

approved (informational — human approval still required via the GitHub approve flow)

No Critical findings. The one Suggestion (testing the AbortSignal wiring) is worth a follow-up but does not block merge. The implementation is clean, well-scoped, and the test suite correctly captures the behavioral contract.

Copy link
Copy Markdown
Member

@jwfing jwfing left a comment

Choose a reason for hiding this comment

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

LGTM, approved.

@CarmenDou CarmenDou merged commit 3f8843d into main May 27, 2026
4 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.

2 participants