fix: resolve TS SDK review comments

[ChiragAgg5k]

Summary

Addresses review comments from the following SDK PRs:

• feat: Node.js SDK update for version 23.1.0 sdk-for-node#146
• feat: Web SDK update for version 24.1.0 sdk-for-web#164
• feat: React Native SDK update for version 0.27.0 sdk-for-react-native#95
• feat: Console SDK update for version 8.1.0 sdk-for-console#74
• feat: Command Line SDK update for version 17.2.1 sdk-for-cli#286

Changes

• Lockfile PLACEHOLDER values (P1): Converted lockfile templates from copy scope to Twig-rendered default scope. The name, version, and license fields now use proper Twig expressions ({{ language.params.npmPackage }}, {{ sdk.version }}, {{ sdk.license }}) instead of being stuck as "PLACEHOLDER". Updated scripts/update-lockfiles.sh to restore Twig expressions after generating lockfiles.
• Missing JSDoc on getHeaders() (P2): Added JSDoc documentation to getHeaders() in all four TS client templates (node, web, react-native, cli).
• .gitignore dist/ pattern (React Native + Web): Fixed the negation pattern so only dist/cjs/package.json and dist/esm/package.json are tracked — generated build outputs in those directories are now properly ignored.

Test plan

• Regenerated node, web, react-native, and cli SDKs — all succeed
• Verified lockfiles in generated output contain real package metadata instead of PLACEHOLDER
• Verified getHeaders() has JSDoc in all generated client files
• Verified .gitignore correctly ignores dist/ build outputs while keeping package.json stubs
• Twig linter passes (composer lint-twig — 0 errors)
• PHPUnit test results unchanged from master (pre-existing failures only)

Summary by CodeRabbit

• New Features

  • Added a public Client.getHeaders() method across SDKs to return a copy of current request headers (including auth) without mutating client state.

• Chores

  • Lockfile generation moved to templated outputs populated from SDK and language params; generated lockfiles are now validated to ensure placeholders are replaced.
  • Template/source entries updated to use templated lockfile rendering.
  • .gitignore patterns refined for distribution dirs; tooling now skips lockfiles when checking line lengths.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Lockfile generation moved from static copies to Twig templates with POSIX-portable helpers to restore Twig placeholders in npm/bun outputs. SDK file manifests updated to use .twig templates. Client classes gained getHeaders(). .gitignore patterns for dist dirs were refined.

Changes

Cohort / File(s)	Summary
Build Script Enhancement scripts/update-lockfiles.sh	Added replace_first, restore_twig_npm, restore_twig_bun. Changed update_npm(lang, extra_flags) → update_npm(lang, extra_flags, twig_name) and update_bun() → update_bun(twig_name). Write outputs to .twig, restore Twig expressions, and validate no PLACEHOLDER remains.
SDK Language Configuration src/SDK/Language/CLI.php, src/SDK/Language/Node.php, src/SDK/Language/ReactNative.php, src/SDK/Language/Web.php	Changed lockfile entries from scope: copy to scope: default and switched templates to .twig variants (e.g., package-lock.json → package-lock.json.twig, bun.lock → bun.lock.twig).
Lockfile Templates templates/cli/bun.lock.twig, templates/node/package-lock.json.twig, templates/react-native/package-lock.json.twig, templates/web/package-lock.json.twig	Replaced static "PLACEHOLDER" values with Twig expressions: name from language.params.npmPackage (with caseDash where used), version from sdk.version, and license from sdk.license.
Client Public API templates/cli/lib/client.ts, templates/node/src/client.ts.twig, templates/react-native/src/client.ts.twig, templates/web/src/client.ts.twig	Added Client.getHeaders(): Headers returning a shallow copy of the instance headers object (documented, non-mutating).
Distribution Directory Management templates/react-native/.gitignore, templates/web/.gitignore	Updated ignore patterns to explicitly exclude dist/cjs/* and dist/esm/*, preserving !dist/*/package.json exceptions.
CI Script Exclusions .github/scripts/max-line-length.sh	Excluded lockfiles from find-based scanning by adding negative -name filters for *.lock, *.lock.*, and *-lock.*.

Sequence Diagram(s)

sequenceDiagram
    participant Generator
    participant BuildScript as "scripts/update-lockfiles.sh"
    participant PackageTool as "npm / bun"
    participant Files as "Filesystem / Templates"

    Generator->>BuildScript: invoke update_npm/update_bun (lang, extra_flags, twig_name)
    BuildScript->>PackageTool: run package tool to generate lockfile
    PackageTool-->>BuildScript: emits lockfile (static values)
    BuildScript->>Files: copy lockfile to *.lock.twig
    BuildScript->>BuildScript: run restore_twig_* to replace first PLACEHOLDERs with Twig exprs
    BuildScript->>Files: write final *.lock.twig
    BuildScript-->>Generator: return status (fail if PLACEHOLDER remains)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰
Lockfiles once plain, now knitted with Twig,
Placeholders hop home—no more static gig.
Headers peek out, kept tidy and sweet,
Dist folders ignored, the tree stays neat.
A rabbit cheers: "Patch landed—nice and spry!" 🎋

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The PR title 'fix: resolve TS SDK review comments' is vague and overly broad, using a non-descriptive term that doesn't convey the specific nature of the changes (lockfile templating, JSDoc additions, .gitignore updates).	Consider a more specific title that highlights the primary changes, such as 'fix: convert lockfiles to Twig templates and add JSDoc for getHeaders()' to better reflect the main objectives.

✅ Passed checks (1 passed)

Check name	Status	Explanation
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/ts-sdk-review-comments

---

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

[greptile-apps]

Greptile Summary

This PR addresses a set of SDK review comments across the TypeScript-based SDK targets (node, web, react-native, cli) by fixing PLACEHOLDER lock file values, adding getHeaders() JSDoc, and correcting .gitignore negation patterns. The changes are well-scoped and largely correct.

• Lockfile PLACEHOLDER fix — Templates renamed to .twig and scope changed from copy to default so Twig expressions ({{ language.params.npmPackage }}, {{ sdk.version }}, {{ sdk.license }}) are rendered during SDK generation rather than left as literal \"PLACEHOLDER\" strings. update-lockfiles.sh is updated with a POSIX-portable awk-based replace_first helper (addressing the previously noted macOS BSD sed incompatibility) that re-instates Twig expressions after the npm/bun lock-file generation step.
• .gitignore restructure — The new pattern (dist/* → !dist/cjs/ → !dist/esm/ → dist/cjs/* → dist/esm/* → !dist/cjs/package.json → !dist/esm/package.json) is the canonical three-stage gitignore technique for tracking a single file inside an otherwise-excluded subtree. The critical first line remains dist/* (not dist/), so git still recurses into dist/ and the negation chain operates correctly.
• JSDoc — getHeaders() now has consistent documentation across all four TS client templates.
• max-line-length.sh — Lock files are now excluded from the line-length lint, preventing false positives from auto-generated content.

One minor cleanup opportunity: when replace_first's awk call exits with code 2 (pattern not found), the intermediate ${file}.tmp file is created but not cleaned up because the && prevents mv from running.

Confidence Score: 5/5

Safe to merge — all previously flagged P1 issues are resolved and the one remaining finding is a minor P2 cleanup in a developer tooling script.

The two P1 concerns from prior review threads (PLACEHOLDER lockfile values and GNU sed incompatibility on macOS) are both properly addressed. The gitignore fix uses the correct dist/* glob and a valid three-stage negation chain. The only new finding is a stale .tmp file that can be left behind if replace_first fails to locate a pattern — this is P2 and does not affect the correctness of the generated SDKs.

scripts/update-lockfiles.sh — minor .tmp cleanup issue in the replace_first function.

Important Files Changed

Filename	Overview
scripts/update-lockfiles.sh	Replaces GNU-sed 0,/addr/ with a portable awk helper; adds restore_twig_npm/bun to rewrite PLACEHOLDER values back to Twig expressions after lock-file generation. Minor: the .tmp file created by the awk redirect is not cleaned up when the pattern is not found.
templates/web/.gitignore	Restructured dist/ negation pattern: adds dist/cjs/* and dist/esm/* before the final negations so only dist/cjs/package.json and dist/esm/package.json are tracked. Pattern is correct — dist/* (not dist/) keeps git recursing into dist/ for the negation chain to work.
templates/react-native/.gitignore	Identical gitignore restructure as web/.gitignore — correctly excludes generated build output while tracking dist/cjs/package.json and dist/esm/package.json.
src/SDK/Language/Node.php	Changed package-lock.json template from copy scope to default scope (with .twig extension) so Twig expressions are rendered during SDK generation.
src/SDK/Language/Web.php	Same copy→default scope change for web package-lock.json template.
src/SDK/Language/ReactNative.php	Same copy→default scope change for react-native package-lock.json template.
src/SDK/Language/CLI.php	Changed bun.lock from copy scope to default with .twig extension so the name field is rendered from Twig instead of left as PLACEHOLDER.
templates/node/package-lock.json.twig	Replaced PLACEHOLDER strings for name, version, and license with proper Twig expressions.
templates/web/package-lock.json.twig	Same PLACEHOLDER→Twig expression replacement as node package-lock.json.twig.
templates/react-native/package-lock.json.twig	Same PLACEHOLDER→Twig expression replacement as node package-lock.json.twig.
templates/cli/bun.lock.twig	Replaced PLACEHOLDER name with {{ language.params.npmPackage
templates/cli/lib/client.ts	Added JSDoc block to getHeaders() describing the return value and the security note about handling headers with care.
templates/node/src/client.ts.twig	Added JSDoc block to getHeaders().
templates/react-native/src/client.ts.twig	Added JSDoc block to getHeaders().
templates/web/src/client.ts.twig	Added JSDoc block to getHeaders().
.github/scripts/max-line-length.sh	Excludes *.lock and -lock. files from the max-line-length check, since lock files contain long auto-generated lines.

_{Reviews (4): Last reviewed commit: "fix: exclude lock files from max-line-le..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/update-lockfiles.sh`:
- Around line 40-45: The replace_first helper is allowing silent no-ops which
can corrupt lockfiles; update the awk invocation inside replace_first to exit
with code 2 when it doesn't find/replace the pattern, then ensure callers of
replace_first (the lines replacing "PLACEHOLDER" for name, version, license)
check that exit code and abort on non-zero; additionally add a final validation
step (e.g., grep or awk scan) after all replacements to detect any remaining
"PLACEHOLDER" tokens and fail the script if any are found so lockfiles are never
left partially replaced.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a4016a2b-ce13-4fa7-9ce1-ab49c0f111ed

📥 Commits

Reviewing files that changed from the base of the PR and between 840b8e6 and 47b4de5.

📒 Files selected for processing (15)

• scripts/update-lockfiles.sh
• src/SDK/Language/CLI.php
• src/SDK/Language/Node.php
• src/SDK/Language/ReactNative.php
• src/SDK/Language/Web.php
• templates/cli/bun.lock.twig
• templates/cli/lib/client.ts
• templates/node/package-lock.json.twig
• templates/node/src/client.ts.twig
• templates/react-native/.gitignore
• templates/react-native/package-lock.json.twig
• templates/react-native/src/client.ts.twig
• templates/web/.gitignore
• templates/web/package-lock.json.twig
• templates/web/src/client.ts.twig