[gentlementlegen]

Resolves #

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR updates configurations, dependencies, and source code across the project. Knip analysis is narrowed to TypeScript files under src. GitHub Actions workflows are updated: Jest testing uses a newer checkout action and modified test command; deployment configuration treats demo branches as production-like environments. Jest switches from ts-jest to @swc/jest with explicit Node test environment. Dependencies are bumped and test scripts simplified. Source code adds early-exit logic for bad-credentials scenarios via a new shouldIgnoreEvent helper and refactored user-fetching logic. The isLabelEvent type guard is removed from typeguards, and test setup no longer loads environment variables directly.

Possibly related PRs

• command-demo#22: Modifies the same update-configuration.yml workflow file with environment selection logic and branch-to-environment mapping changes.
• command-demo#17: Implements identical code-level changes to src/index.ts (BAD_CREDENTIALS_MESSAGE, shouldIgnoreEvent, getGitHubUser, runPlugin refactoring) and removal of isLabelEvent in src/types/typeguards.ts.

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is incomplete. It contains only the template placeholder with no issue link filled in and no explanatory text about the actual changes.	Fill in the issue number (e.g., 'Resolves #123') and add a brief summary of the changes made across configurations, workflows, and test infrastructure.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.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 title 'Demo' is vague and does not meaningfully describe the changeset. It doesn't convey what was actually changed across the multiple files and systems.	Use a descriptive title summarizing the main change, e.g., 'Migrate to @swc/jest and update GitHub workflows' or 'Fix credentials handling and update test infrastructure'.

_{✏️ 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 demo

---

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]

Actionable comments posted: 1

🧹 Nitpick comments (4)

tests/main.test.ts (1)

50-50: Consider why this test needs 2.5x the default timeout.

Default testTimeout in jest.config.ts is 20s. A 50s override suggests potential slow operations or flaky behavior. If consistently needed, consider increasing the global timeout instead.

package.json (1)

26-27: Redundant scripts: jest:test and test are identical.

Both run jest --coverage. Consider keeping only test or documenting why both exist.

src/index.ts (1)

26-36: Consider adding type annotation for better error context.

The function implicitly returns Promise<any> from the Octokit response. Adding explicit return type improves readability.

♻️ Add explicit return type

-async function getGitHubUser(context: Context) {
+async function getGitHubUser(context: Context): Promise<{ login: string }> {

jest.config.ts (1)

16-16: Empty transformIgnorePatterns transforms all node_modules, impacting test performance.

Excluding node_modules (except specific ESM packages) is a standard Jest optimization. Found 2 ESM packages in node_modules: knip and eslint-plugin-check-file.

Suggested pattern to transform only `@ubiquity-os` ESM packages

-  transformIgnorePatterns: [],
+  transformIgnorePatterns: [
+    "node_modules/(?!(`@ubiquity-os`)/)"
+  ],

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 83c3cef8-23bb-47ad-9738-0fd3937a234b

📥 Commits

Reviewing files that changed from the base of the PR and between 515aba8 and 2b1650a.

⛔ Files ignored due to path filters (1)

• bun.lock is excluded by !**/*.lock

📒 Files selected for processing (8)

• .github/knip.ts
• .github/workflows/jest-testing.yml
• .github/workflows/update-configuration.yml
• jest.config.ts
• package.json
• src/index.ts
• src/types/typeguards.ts
• tests/main.test.ts

💤 Files with no reviewable changes (1)

• src/types/typeguards.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check what logger.error returns in `@ubiquity-os/ubiquity-os-logger`
ast-grep --pattern 'error($$$) {
  $$$
  return $_;
}'

Repository: ubiquity-os/command-demo

Length of output: 50

---

🌐 Web query:

Does @ubiquity-os/ubiquity-os-logger error method return an Error object?

💡 Result:

No, the @ubiquity-os/ubiquity-os-logger error method does not return an Error object. Evidence from Ubiquity OS plugin SDK documentation and usage shows the logger provides methods like logger.ok("Done"), which are used for logging messages, not throwing or returning Error instances. In error contexts from GitHub issues (e.g., command-start-stop #136), errors are standard JavaScript Error objects ("name": "Error") passed to logging, indicating the logger.error likely logs the error rather than returning one. No sources explicitly state or demonstrate the error method returning an Error object; it aligns with standard logger patterns where log methods return void or log metadata.

Citations:

• 1: Broken Errors Posting on Wrong Threads ubiquity-os-marketplace/command-start-stop#136
• 2: https://github.com/ubiquity-os/plugin-sdk

---

🏁 Script executed:

# Find references to context.logger.error to understand how it's used
rg "logger\.error" -A 2 -B 2 --max-count 5

Repository: ubiquity-os/command-demo

Length of output: 715

---

🏁 Script executed:

# Check if there's a logger configuration or type definitions in the repo
fd -t f "logger" --max-one-result | head -20

Repository: ubiquity-os/command-demo

Length of output: 301

---

🏁 Script executed:

# Check package.json for the `@ubiquity-os/ubiquity-os-logger` dependency
cat package.json | grep -A 2 -B 2 "ubiquity-os-logger"

Repository: ubiquity-os/command-demo

Length of output: 220

---

Fix throwing logger.error() calls — logger.error() returns void, not Error.

The code at lines 33-35 throws context.logger.error(), but this method returns void (confirmed via package documentation). Throwing undefined breaks exception handling and loses stack traces. This pattern also appears in src/handlers/run-demo.ts.

Instead, log first then throw a proper Error:

Example fix

} catch (err) {
  context.logger.error(BAD_CREDENTIALS_MESSAGE, { err });
  throw new Error(BAD_CREDENTIALS_MESSAGE);
}