Fix Factory usage metrics#561
Open
abyssbugg wants to merge 1 commit into
Open
Conversation
Co-Authored-By: Oz <oz-agent@warp.dev>
📝 WalkthroughWalkthroughThe Factory/Droid plugin gains support for extra usage tracking, Droid Core status, managed computers metrics via supplemental billing and compute-usage endpoints, and enhanced plan inference. Documentation, configuration, request handling, parsing utilities, and data integration work together to surface extended usage information in the UI. ChangesFactory Plugin Extended Usage & Droid Core Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Author
|
@robinebers this surgical Factory-only PR is ready for review. Validation completed:
Screenshots are not attached because they require authenticated Factory account data. No UI layout/components changed; this only updates the provider data lines rendered by the existing UI. |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Extends the Factory/Droid provider to surface additional quota/usage signals (rate-limit windows, extra usage balance, Droid Core, managed compute) and adds fallback API calls when the primary subscription usage response is legacy-only.
Changes:
- Add supplemental fetch/normalization logic (billing limits + compute usage) and enrich displayed lines/plan labeling.
- Update plugin manifest to display new lines (Extra Usage, 5-hour/weekly/monthly usage, Droid Core, Managed Computers).
- Add/expand tests and documentation to cover the new endpoints and behaviors.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/factory/plugin.js | Adds header/payload helpers, supplemental endpoint fetch/merge, new line builders, and updated plan inference. |
| plugins/factory/plugin.json | Registers new UI lines for extra usage, rate windows, Droid Core, and managed compute. |
| plugins/factory/plugin.test.js | Adds tests for new formatting behavior, legacy-only fallback fetching, and request headers/userId. |
| docs/providers/factory.md | Documents the additional endpoints and updated plan detection notes. |
| README.md | Updates provider capability summary to reflect the new metrics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+11
to
+16
| function makeJwt(expSecondsOrClaims) { | ||
| const header = btoa(JSON.stringify({ alg: "HS256", typ: "JWT" })) | ||
| const payload = btoa(JSON.stringify({ exp: expSeconds, org_id: "org_123", email: "test@example.com" })) | ||
| const claims = typeof expSecondsOrClaims === "object" | ||
| ? { org_id: "org_123", email: "test@example.com", ...expSecondsOrClaims } | ||
| : { exp: expSecondsOrClaims, org_id: "org_123", email: "test@example.com" } | ||
| const payload = btoa(JSON.stringify(claims)) |
Comment on lines
+261
to
+268
| function getUserIdFromAccessToken(ctx, accessToken) { | ||
| const payload = ctx.jwt.decodePayload(accessToken) | ||
| if (!payload || typeof payload !== "object") return null | ||
| const rawUserId = payload.sub || payload.user_id || payload.userId | ||
| if (typeof rawUserId !== "string") return null | ||
| const userId = rawUserId.trim() | ||
| return userId || null | ||
| } |
Comment on lines
+537
to
+545
| const compute = requestGetJson(ctx, accessToken, COMPUTE_USAGE_URL, "compute usage") | ||
| if (isObject(compute) && !firstObject( | ||
| merged.managedComputers, | ||
| merged.managedComputerUsage, | ||
| merged.computers, | ||
| merged.compute, | ||
| merged.managedCompute, | ||
| )) { | ||
| const limitMs = firstNumber(compute, ["limitMs", "includedMs", "allowanceMs", "totalMs", "limit"]) |
Comment on lines
+679
to
+687
| const managedUsed = firstNumber(managed, ["usedHours", "usageHours", "hoursUsed", "used", "current"]) | ||
| const used = managedUsed === null ? 0 : managedUsed | ||
| const limit = firstNumber(managed, ["includedHours", "limitHours", "allowanceHours", "totalHours", "limit"]) | ||
| if (limit === null || limit <= 0) return false | ||
| lines.push(ctx.line.progress({ | ||
| label: "Managed Computers", | ||
| used, | ||
| limit, | ||
| format: { kind: "count", suffix: "h" }, |
Comment on lines
+85
to
+87
| "fiveHour": { "usedPercent": 0.12, "windowEnd": 1770626926000, "secondsRemaining": 1200 }, | ||
| "weekly": { "usedPercent": 0.34, "windowEnd": 1771228126000, "secondsRemaining": 604800 }, | ||
| "monthly": { "usedPercent": 0.56, "windowEnd": 1772956800000, "secondsRemaining": 2333474 } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugins/factory/plugin.js (1)
261-711: 🏗️ Heavy liftSplit this plugin module to stay within the repository size guideline.
This file is now far beyond the ~400 LOC target; the newly added parsing/fetch/line-formatting logic is a strong extraction seam into smaller modules.
As per coding guidelines:
**/*.{js,ts,tsx,jsx,rs,py,java,go,cpp,c,rb}should keep files under ~400 lines of code and split/refactor as needed.🤖 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 `@plugins/factory/plugin.js` around lines 261 - 711, This file exceeds the repo size guideline; split the large plugin.js into smaller modules by extracting cohesive seams: move HTTP/usage helpers (buildUsageHeaders, buildUsagePayload, buildUsageGetUrl, fetchUsage, refreshToken, requestGetJson) into a usage-api module, move generic utilities (isObject, asNumber, firstValue, firstNumber, firstObject, normalizePercent, metricStartValue, metricEndValue, metricResetsAt, metricPeriodDurationMs, percentFromMetric) into a utils module, and move presentation/line-formatting logic (windowMetricWithDuration, mergeSupplementalUsage, addPercentUsageLine, addExtraUsageLine, addDroidCoreLine, addManagedComputersLine, inferPlan, droidCoreConfig, hasExtendedUsageFields, isDroidCoreEnabled) into a usage-ui module; export/import these functions where needed and update plugin.js to orchestrate them (retain getUserIdFromAccessToken and top-level exports), ensuring tests/imports are updated and no logic changes are made during the refactor.plugins/factory/plugin.test.js (1)
508-754: 🏗️ Heavy liftSplit this test suite; it’s well beyond the size guideline.
These additions are good, but this file is already very large. Consider splitting by behavior area (auth loading, refresh flow, usage formatting) to keep tests maintainable.
As per coding guidelines:
**/*.{js,ts,tsx,jsx,rs,py,java,go,cpp,c,rb}should keep files under ~400 lines of code and split/refactor as needed.🤖 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 `@plugins/factory/plugin.test.js` around lines 508 - 754, This file exceeds the size guideline; split the large test suite into smaller focused files (e.g. auth tests, refresh-flow tests, usage-formatting tests) by moving related it() blocks into new files and keeping shared helpers (makeCtx, loadPlugin, makeJwt) imported from the existing test helpers so you don't duplicate setup; specifically extract the tests that exercise authentication/loading (those using makeCtx + auth.json writes and makeJwt), the refresh flow cases, and the usage/formatting assertions (the tests asserting result.plan and result.lines such as the ones checking "Extra Usage", "5-hour usage", "Droid Core", "Managed Computers", and "Standard"), ensure each new file references loadPlugin and plugin.probe the same way, update imports to use the shared helpers, and keep each test file under ~400 lines.
🤖 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 `@plugins/factory/plugin.js`:
- Around line 531-534: The merge currently forces merged.droidCore.enabled =
true whenever a coreLimits object exists; change it to respect explicit flags
from coreLimits: if coreLimits exists and coreLimits.enabled === false, set
merged.droidCore = { enabled: false }; otherwise if coreLimits exists and merged
has no droid core config (use droidCoreConfig(merged)), set merged.droidCore = {
enabled: true }. Reference the coreLimits variable, firstObject-derived input,
droidCoreConfig(merged) check, and merged.droidCore assignment when making the
change.
---
Nitpick comments:
In `@plugins/factory/plugin.js`:
- Around line 261-711: This file exceeds the repo size guideline; split the
large plugin.js into smaller modules by extracting cohesive seams: move
HTTP/usage helpers (buildUsageHeaders, buildUsagePayload, buildUsageGetUrl,
fetchUsage, refreshToken, requestGetJson) into a usage-api module, move generic
utilities (isObject, asNumber, firstValue, firstNumber, firstObject,
normalizePercent, metricStartValue, metricEndValue, metricResetsAt,
metricPeriodDurationMs, percentFromMetric) into a utils module, and move
presentation/line-formatting logic (windowMetricWithDuration,
mergeSupplementalUsage, addPercentUsageLine, addExtraUsageLine,
addDroidCoreLine, addManagedComputersLine, inferPlan, droidCoreConfig,
hasExtendedUsageFields, isDroidCoreEnabled) into a usage-ui module;
export/import these functions where needed and update plugin.js to orchestrate
them (retain getUserIdFromAccessToken and top-level exports), ensuring
tests/imports are updated and no logic changes are made during the refactor.
In `@plugins/factory/plugin.test.js`:
- Around line 508-754: This file exceeds the size guideline; split the large
test suite into smaller focused files (e.g. auth tests, refresh-flow tests,
usage-formatting tests) by moving related it() blocks into new files and keeping
shared helpers (makeCtx, loadPlugin, makeJwt) imported from the existing test
helpers so you don't duplicate setup; specifically extract the tests that
exercise authentication/loading (those using makeCtx + auth.json writes and
makeJwt), the refresh flow cases, and the usage/formatting assertions (the tests
asserting result.plan and result.lines such as the ones checking "Extra Usage",
"5-hour usage", "Droid Core", "Managed Computers", and "Standard"), ensure each
new file references loadPlugin and plugin.probe the same way, update imports to
use the shared helpers, and keep each test file under ~400 lines.
🪄 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 Plus
Run ID: 86c2557a-c49d-4d27-bbfe-ba37baa88734
📒 Files selected for processing (5)
README.mddocs/providers/factory.mdplugins/factory/plugin.jsplugins/factory/plugin.jsonplugins/factory/plugin.test.js
Comment on lines
+531
to
+534
| const coreLimits = limits && firstObject(limits.core, limits.droidCore, limits.droid_core) | ||
| if (coreLimits && !droidCoreConfig(merged)) { | ||
| merged.droidCore = { enabled: true } | ||
| } |
There was a problem hiding this comment.
Respect explicit Droid Core disable flags when merging supplemental limits.
Line 533 forces enabled: true whenever a core object exists. If the API returns enabled: false, UI/plan output will still show Droid Core as enabled.
Suggested fix
- const coreLimits = limits && firstObject(limits.core, limits.droidCore, limits.droid_core)
- if (coreLimits && !droidCoreConfig(merged)) {
- merged.droidCore = { enabled: true }
- }
+ const coreLimits = limits && firstObject(limits.core, limits.droidCore, limits.droid_core)
+ if (coreLimits && !droidCoreConfig(merged)) {
+ const hasExplicitFlag =
+ typeof coreLimits.enabled === "boolean" ||
+ typeof coreLimits.available === "boolean" ||
+ typeof coreLimits.included === "boolean"
+ const coreEnabled =
+ coreLimits.enabled === true ||
+ coreLimits.available === true ||
+ coreLimits.included === true ||
+ (!hasExplicitFlag && Object.keys(coreLimits).length > 0)
+ if (coreEnabled) merged.droidCore = { enabled: true }
+ }📝 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.
Suggested change
| const coreLimits = limits && firstObject(limits.core, limits.droidCore, limits.droid_core) | |
| if (coreLimits && !droidCoreConfig(merged)) { | |
| merged.droidCore = { enabled: true } | |
| } | |
| const coreLimits = limits && firstObject(limits.core, limits.droidCore, limits.droid_core) | |
| if (coreLimits && !droidCoreConfig(merged)) { | |
| const hasExplicitFlag = | |
| typeof coreLimits.enabled === "boolean" || | |
| typeof coreLimits.available === "boolean" || | |
| typeof coreLimits.included === "boolean" | |
| const coreEnabled = | |
| coreLimits.enabled === true || | |
| coreLimits.available === true || | |
| coreLimits.included === true || | |
| (!hasExplicitFlag && Object.keys(coreLimits).length > 0) | |
| if (coreEnabled) merged.droidCore = { enabled: true } | |
| } |
🤖 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 `@plugins/factory/plugin.js` around lines 531 - 534, The merge currently forces
merged.droidCore.enabled = true whenever a coreLimits object exists; change it
to respect explicit flags from coreLimits: if coreLimits exists and
coreLimits.enabled === false, set merged.droidCore = { enabled: false };
otherwise if coreLimits exists and merged has no droid core config (use
droidCoreConfig(merged)), set merged.droidCore = { enabled: true }. Reference
the coreLimits variable, firstObject-derived input, droidCoreConfig(merged)
check, and merged.droidCore assignment when making the change.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plugins/factory/plugin.js">
<violation number="1" location="plugins/factory/plugin.js:533">
P2: This unconditionally sets `enabled: true` whenever a `coreLimits` object exists in the API response. If the API returns `{ enabled: false }` (e.g., a disabled or suspended Droid Core add-on), the UI will still show Droid Core as active and append `+ Droid Core` to the plan label. Check the explicit flags (`enabled`, `available`, `included`) from `coreLimits` before setting this.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
|
||
| const coreLimits = limits && firstObject(limits.core, limits.droidCore, limits.droid_core) | ||
| if (coreLimits && !droidCoreConfig(merged)) { | ||
| merged.droidCore = { enabled: true } |
Contributor
There was a problem hiding this comment.
P2: This unconditionally sets enabled: true whenever a coreLimits object exists in the API response. If the API returns { enabled: false } (e.g., a disabled or suspended Droid Core add-on), the UI will still show Droid Core as active and append + Droid Core to the plan label. Check the explicit flags (enabled, available, included) from coreLimits before setting this.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plugins/factory/plugin.js, line 533:
<comment>This unconditionally sets `enabled: true` whenever a `coreLimits` object exists in the API response. If the API returns `{ enabled: false }` (e.g., a disabled or suspended Droid Core add-on), the UI will still show Droid Core as active and append `+ Droid Core` to the plan label. Check the explicit flags (`enabled`, `available`, `included`) from `coreLimits` before setting this.</comment>
<file context>
@@ -314,34 +353,363 @@
+
+ const coreLimits = limits && firstObject(limits.core, limits.droidCore, limits.droid_core)
+ if (coreLimits && !droidCoreConfig(merged)) {
+ merged.droidCore = { enabled: true }
+ }
+ }
</file context>
Suggested change
| merged.droidCore = { enabled: true } | |
| const hasExplicitFlag = | |
| typeof coreLimits.enabled === "boolean" || | |
| typeof coreLimits.available === "boolean" || | |
| typeof coreLimits.included === "boolean" | |
| const coreEnabled = | |
| coreLimits.enabled === true || | |
| coreLimits.available === true || | |
| coreLimits.included === true || | |
| (!hasExplicitFlag && Object.keys(coreLimits).length > 0) | |
| if (coreEnabled) merged.droidCore = { enabled: true } |
robinebers
requested changes
Jun 6, 2026
robinebers
left a comment
robinebers
left a comment
Owner
There was a problem hiding this comment.
Hey! This is Rob's AI reviewer. Thanks for the contribution.
The Factory expansion is useful and the coverage is strong. One thing needs fixing before merge:
- Droid Core can be shown as enabled when it is disabled - when
/api/billing/limitsreturns alimits.coreobject, the current merge setsdroidCore.enabled = truejust because that object exists. Please preserve an explicit disabled value, for example whencoreLimits.enabled === false, and add a regression test so a disabled Droid Core response does not show the Droid Core badge or append+ Droid Coreto the plan.
Everything else I checked looks aligned with the plugin shape, docs, and tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Scope
Surgical Factory-only change based on latest upstream/main. No version bump, build workflow changes, or unrelated provider/app changes.
Validation
bun run test --run plugins/factory/plugin.test.js— 44 tests passedbun run test --run— 64 files / 1093 tests passedbun run build— passedScreenshots
Draft PR: screenshots are pending because they require authenticated Factory account data. No UI layout/components changed; this updates provider data lines displayed by the existing UI.
Conversation: https://app.warp.dev/conversation/0133ae31-1784-4e10-87f5-baf8d6c69bca
Co-Authored-By: Oz oz-agent@warp.dev
Summary by cubic
Show full Factory/Droid usage: 5-hour, weekly, monthly, extra usage balance, Droid Core, and managed computer hours. Adds supplemental API calls and smarter parsing so data matches the Factory app.
GET /api/billing/limitsandGET /api/organization/compute-usagewhen subscription usage lacks these fields; sends browser-like headers anduserId; falls back to GET if POST returns 405.Written for commit cfc2041. Summary will update on new commits.
Summary by CodeRabbit
Documentation
New Features