feat: add business-flex profiles, budgets, and routing visibility#68
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Owner
Author
|
Docs update added on top of the business-flex implementation.\n\nAdded/updated:\n- README end-user setup guide for npm install + any framework\n- Mintlify Business Flex Profiles guide\n- Mintlify Builder & Validator Sandbox guide\n- Mintlify AI SDLC Trends & Loopholes roadmap\n- Mintlify navigation + Quickstart/Installation/CLI/Config/Business Hub updates\n- PPT moved into tracked backup: docs/mintlify/assets/backups/RStack-The-Future-of-Software-Development.backup.pptx\n\nVerification after docs update:\n- npm run lint\n- npm run validate\n- docs/mintlify: npm run check\n- git diff --check |
Contributor
📝 WalkthroughWalkthroughRStack now implements a profile-based configuration system enabling npm-installed packages to activate only business-team workflows, stages, domains, agents, and dashboard features. This PR adds built-in profiles, per-task budget envelopes, Contract v2 execution telemetry validation, and a Business Flex dashboard page that visualizes profile, budget, and agent routing from real run data. ChangesProfile and Budget Foundation with SDLC Integration
Contract v2 Telemetry Validation and Dashboard Visibility
Documentation and README Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
✅ Created PR with unit tests: #69 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
tests/harness-contracts.test.js (1)
56-75: ⚡ Quick winAdd negative v2 telemetry cases to lock validator behavior.
This new test is good for the PASS path, but it should also cover invalid telemetry (e.g.
tools_used: null,estimated_usd: '') so these checks fail when malformed payloads are provided.Suggested additions
+test('builder contract fails when execution.tools_used is present but not an array', () => { + const result = validateBuilderContract({ + task_id: '004-implementation', + status: 'PASS', + summary: 'bad telemetry', + files_modified: [], + tests_run: [], + risks: [], + next_steps: [], + execution: { tools_used: null }, + }, '004-implementation'); + + assert.equal(result.ok, false); + assert.ok(result.issues.some((i) => i.name === 'builder_v2_execution_tools_used_is_array')); +}); + +test('builder contract fails when cost fields are non-numeric types', () => { + const result = validateBuilderContract({ + task_id: '004-implementation', + status: 'PASS', + summary: 'bad telemetry', + files_modified: [], + tests_run: [], + risks: [], + next_steps: [], + cost: { estimated_usd: '', actual_usd: true }, + }, '004-implementation'); + + assert.equal(result.ok, false); + assert.ok(result.issues.some((i) => i.name === 'builder_v2_cost_values_are_numeric')); +});🤖 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 `@tests/harness-contracts.test.js` around lines 56 - 75, Add negative test cases to tests/harness-contracts.test.js that call validateBuilderContract with malformed v2 telemetry (one payload where execution.tools_used is null and another where cost.estimated_usd is an empty string) and assert the validator returns a failing result (result.ok === false) and that result.checks contains the relevant failing checks such as 'builder_v2_execution_tools_used_is_array' for the tools_used=null case and 'builder_v2_cost_values_are_numeric' for the estimated_usd='' case; place these alongside the existing PASS case and use the same validateBuilderContract invocation pattern and check lookups (result.checks.some(check => check.name === '...')) to ensure the validator rejects malformed telemetry.README.md (1)
227-234: ⚡ Quick winConsider making the verified state section more maintainable.
The hardcoded test results (111 pass, 196 agents validated) are snapshot-in-time data that will become stale as the codebase evolves. Future contributors may wonder why their local results differ from the README.
Consider either:
- Removing the specific numbers and keeping only the command examples
- Adding a timestamp or commit SHA to indicate when these results were captured
- Moving this to a CI badge or separate release notes
🤖 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 `@README.md` around lines 227 - 234, Update the "Latest verified branch state for this business-flex slice" section in README.md to avoid hardcoded snapshot numbers: either remove the specific numeric results (e.g., "111 pass, 0 fail" and "All 196 agents passed validation") and keep only the example commands block, or replace those numbers with a captured timestamp/commit SHA indicating when the results were taken, or swap them for a CI badge or link to release notes; modify the markdown around the commands block so the text reflects whichever option you choose and ensure the section header and the three command lines (npm test -- --runInBand, npm run lint, npm run validate, npm pack --dry-run) remain clear for contributors.src/core/profiles.js (2)
119-137: 💤 Low valueConsider distinguishing file-missing from parse-error cases.
The catch block at line 134 silently returns the default profile for both missing files and corrupt JSON. While safe, this could hide configuration issues (e.g., a typo in
.rstack/rstack.config.jsonwould silently fall back to the default instead of alerting the user).💡 Optional: Log warnings for parse failures
} catch { + // File exists but is unreadable/invalid — log to help debug config issues + console.warn(`[rstack] Failed to parse ${configPath}, using default profile`); return profileConfig('business-flex'); }🤖 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 `@src/core/profiles.js` around lines 119 - 137, The current loadProjectProfile silently falls back to profileConfig on any error; update loadProjectProfile to distinguish a missing file from a JSON parse error by checking existsSync(configPath) before/inside the catch and, on parse failures (e.g., SyntaxError while parsing the file at configPath), emit a warning including the error details (using your project logger or console.warn) referencing configPath and the error, then return the default profileConfig('business-flex') or propagate the error if you prefer stricter behavior; ensure the changes are applied around the JSON.parse/parsed usage in loadProjectProfile so only actual parse/read errors trigger the warning while the earlier existsSync(path) still handles the missing-file case.
156-172: 💤 Low valueDocument the hard-coded fallback divisor for budget envelope computation.
Line 159 uses
run_budget_usd / 8as a fallback when no stage budgets are configured. This magic number (8) is not explained and could be unclear to future maintainers or users adjusting budget policies.📝 Recommended: Add a comment explaining the fallback
const stageBudget = stageIds.reduce((sum, stageId) => sum + Number(budgetPolicy.stage_budgets?.[stageId] || 0), 0); - const fallback = Number(budgetPolicy.run_budget_usd || 0) / 8; + // Fallback: divide run budget by 8 (typical SDLC has ~8 lifecycle stages) + const fallback = Number(budgetPolicy.run_budget_usd || 0) / 8; return {🤖 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 `@src/core/profiles.js` around lines 156 - 172, The fallback divisor (8) used in budgetEnvelopeForTask to compute fallback = Number(budgetPolicy.run_budget_usd || 0) / 8 is a magic number and needs documentation (and preferably a named constant). Update budgetEnvelopeForTask to replace the hard-coded 8 with a clearly named constant (e.g., DEFAULT_STAGE_DIVISOR or RUN_BUDGET_STAGE_COUNT) and add a brief comment explaining why that divisor is used (what it represents and when it applies); ensure the constant is defined near the top of the module so reviewers can see its purpose and use that constant in the computed fallback and any related logic (refer to budgetEnvelopeForTask, fallback, and stage_budgets).tests/dashboard-business-hub.test.js (1)
14-109: ⚡ Quick winConsider adding test coverage for project-level config file loading.
The current test verifies the fallback path where
projectProfileis null (no.rstack/rstack.config.jsonfile), andrun.profileis constructed fromtasksRaw.profile. It would strengthen coverage to add a test case that creates.rstack/rstack.config.jsonwith a full profile object (includingenabled_domains,enabled_agents,enabled_plugins,dashboard_pages) and verifies thatprojectProfileis loaded and used asrun.profile.This would ensure the primary path (profile from config file) works correctly and that the full profile shape is handled properly by downstream aggregation in
buildBusinessFlexState.🧪 Suggested test case structure
test('Business Flex loads profile from .rstack/rstack.config.json', async () => { const projectRoot = mkdtempSync(join(tmpdir(), 'rstack-profile-config-')); try { const runId = '2026-06-01T12-00-00-profile'; const runDir = join(projectRoot, '.rstack', 'runs', runId); await mkdir(runDir, { recursive: true }); // Create project-level config with full profile await writeJson(join(projectRoot, '.rstack', 'rstack.config.json'), { profile: 'business-flex', name: 'Business Flex Delivery', workflow: 'production-business-sdlc', enabled_domains: ['product', 'backend', 'frontend'], enabled_agents: ['business-analyst', 'backend-architect'], enabled_plugins: ['business-analytics', 'backend-development'], dashboard_pages: ['command', 'business-flex', 'workflow'], }); await writeJson(join(runDir, 'manifest.json'), { run_id: runId, goal: 'Test profile loading' }); await writeJson(join(runDir, 'tasks.json'), { tasks: [] }); await writeFile(join(runDir, 'events.jsonl'), ''); const state = await buildFullState(projectRoot, { includeRegistry: false }); assert.equal(state.businessFlex.profiles[0].profile, 'business-flex'); assert.deepEqual(state.businessFlex.profiles[0].enabledDomains, ['product', 'backend', 'frontend']); assert.deepEqual(state.businessFlex.profiles[0].enabledAgents, ['business-analyst', 'backend-architect']); } finally { rmSync(projectRoot, { recursive: true, force: 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 `@tests/dashboard-business-hub.test.js` around lines 14 - 109, Add a new unit test that writes a project-level config file (.rstack/rstack.config.json) containing a full profile object and asserts that buildFullState (and downstream buildBusinessFlexState) loads that profile instead of falling back to tasksRaw.profile; specifically, create a temp project, write rstack.config.json with keys like profile, enabled_domains, enabled_agents, enabled_plugins, dashboard_pages, write minimal run manifest/tasks/events for a runId, call buildFullState(projectRoot, { includeRegistry: false }) and assert that state.businessFlex.profiles[0].profile equals the profile from the config and that enabledDomains/enabledAgents (or their normalized names in buildBusinessFlexState) match the config arrays.src/observability/dashboard/state/runs.js (1)
167-167: 💤 Low valueNested ternary for profile construction is hard to parse.
The fallback chain
projectProfile || (manifest?.profile ? {...} : (tasksRaw?.profile ? {...} : null))is functionally correct but dense. The logic prioritizes project-level config, then manifest, then tasks.json, finally null. Downstream code (business-flex.js, client.js) handles both the full profile object shape (withenabled_domains,enabled_agents, etc.) and the minimal fallback shape{profile, workflow}defensively.Consider adding an inline comment explaining the priority order and shape variation, or extract to a helper function for clarity.
🤖 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 `@src/observability/dashboard/state/runs.js` at line 167, The nested ternary that builds `profile` (projectProfile || (manifest?.profile ? { profile: manifest.profile, workflow: manifest.workflow } : (tasksRaw?.profile ? { profile: tasksRaw.profile, workflow: tasksRaw.workflow } : null))) is hard to read; replace it with a small helper (e.g. getProfileFromSources) or at least add an inline comment describing the priority: use projectProfile first, then manifest.profile, then tasksRaw.profile, otherwise null, and return either the full profile object or the minimal {profile, workflow} shape; update the code that currently references `profile` in runs.js to call this helper (or keep the single assignment but extracted) so the logic is explicit and easier to maintain.
🤖 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 `@docs/mintlify/reference/loopholes-roadmap.mdx`:
- Line 43: Replace the hard-coded string "Package version remains `1.8.0` on
this branch" in the roadmap table with a version-agnostic statement (e.g.,
"Package version unchanged on this branch — see package changelog for current
release") or a dynamic pointer to the package/changelog as the source of truth;
update the table cell text so it does not contain a specific semantic version
and instead references the changelog/package info.
In `@src/core/harness/contracts.js`:
- Around line 99-106: The current guard (builder?.execution?.tools_used) skips
validation for present-but-falsy values (e.g., null); change the presence test
to detect the property itself and always push a PASS/FAIL check: use a presence
check like Object.prototype.hasOwnProperty.call(builder.execution, 'tools_used')
or the `'tools_used' in builder.execution` form to decide whether to run the
Array.isArray check, and then keep the existing checks.push block (referencing
builder.execution.tools_used and the check name
builder_v2_execution_tools_used_is_array) so null/undefined/non-array values
produce a FAIL with evidence 'not an array'.
- Around line 108-116: The current check for cost fields uses Number(...) which
coerces values (e.g., empty strings, booleans) into numbers and accepts them;
update validation to avoid coercion by ensuring each builder.cost[field] is
already a JavaScript number and finite. Specifically, modify the logic around
builder?.cost and the computation for the 'builder_v2_cost_values_are_numeric'
check so the filtered fields and .every() test verify typeof builder.cost[field]
=== 'number' && Number.isFinite(builder.cost[field]) (rather than Number(...)
coercion), so only real numeric types pass.
In `@src/observability/dashboard/state/business-flex.js`:
- Line 32: runBudgetTotal is summing run.budgetPolicy?.run_budget_usd for every
run which double-counts shared project-level budgets when tasks.json lacks a
budget_policy; update the aggregation so you only add
run.budgetPolicy.run_budget_usd when the run actually declares a budget in its
tasks file (check run.tasksRaw?.budget_policy) or, if you must include
project-level budgets, dedupe by the project identifier (e.g., run.projectRoot
or run.projectId) using a Set before adding; modify the logic around
runBudgetTotal and references to run.budgetPolicy and
run.tasksRaw?.budget_policy accordingly.
---
Nitpick comments:
In `@README.md`:
- Around line 227-234: Update the "Latest verified branch state for this
business-flex slice" section in README.md to avoid hardcoded snapshot numbers:
either remove the specific numeric results (e.g., "111 pass, 0 fail" and "All
196 agents passed validation") and keep only the example commands block, or
replace those numbers with a captured timestamp/commit SHA indicating when the
results were taken, or swap them for a CI badge or link to release notes; modify
the markdown around the commands block so the text reflects whichever option you
choose and ensure the section header and the three command lines (npm test --
--runInBand, npm run lint, npm run validate, npm pack --dry-run) remain clear
for contributors.
In `@src/core/profiles.js`:
- Around line 119-137: The current loadProjectProfile silently falls back to
profileConfig on any error; update loadProjectProfile to distinguish a missing
file from a JSON parse error by checking existsSync(configPath) before/inside
the catch and, on parse failures (e.g., SyntaxError while parsing the file at
configPath), emit a warning including the error details (using your project
logger or console.warn) referencing configPath and the error, then return the
default profileConfig('business-flex') or propagate the error if you prefer
stricter behavior; ensure the changes are applied around the JSON.parse/parsed
usage in loadProjectProfile so only actual parse/read errors trigger the warning
while the earlier existsSync(path) still handles the missing-file case.
- Around line 156-172: The fallback divisor (8) used in budgetEnvelopeForTask to
compute fallback = Number(budgetPolicy.run_budget_usd || 0) / 8 is a magic
number and needs documentation (and preferably a named constant). Update
budgetEnvelopeForTask to replace the hard-coded 8 with a clearly named constant
(e.g., DEFAULT_STAGE_DIVISOR or RUN_BUDGET_STAGE_COUNT) and add a brief comment
explaining why that divisor is used (what it represents and when it applies);
ensure the constant is defined near the top of the module so reviewers can see
its purpose and use that constant in the computed fallback and any related logic
(refer to budgetEnvelopeForTask, fallback, and stage_budgets).
In `@src/observability/dashboard/state/runs.js`:
- Line 167: The nested ternary that builds `profile` (projectProfile ||
(manifest?.profile ? { profile: manifest.profile, workflow: manifest.workflow }
: (tasksRaw?.profile ? { profile: tasksRaw.profile, workflow: tasksRaw.workflow
} : null))) is hard to read; replace it with a small helper (e.g.
getProfileFromSources) or at least add an inline comment describing the
priority: use projectProfile first, then manifest.profile, then
tasksRaw.profile, otherwise null, and return either the full profile object or
the minimal {profile, workflow} shape; update the code that currently references
`profile` in runs.js to call this helper (or keep the single assignment but
extracted) so the logic is explicit and easier to maintain.
In `@tests/dashboard-business-hub.test.js`:
- Around line 14-109: Add a new unit test that writes a project-level config
file (.rstack/rstack.config.json) containing a full profile object and asserts
that buildFullState (and downstream buildBusinessFlexState) loads that profile
instead of falling back to tasksRaw.profile; specifically, create a temp
project, write rstack.config.json with keys like profile, enabled_domains,
enabled_agents, enabled_plugins, dashboard_pages, write minimal run
manifest/tasks/events for a runId, call buildFullState(projectRoot, {
includeRegistry: false }) and assert that state.businessFlex.profiles[0].profile
equals the profile from the config and that enabledDomains/enabledAgents (or
their normalized names in buildBusinessFlexState) match the config arrays.
In `@tests/harness-contracts.test.js`:
- Around line 56-75: Add negative test cases to tests/harness-contracts.test.js
that call validateBuilderContract with malformed v2 telemetry (one payload where
execution.tools_used is null and another where cost.estimated_usd is an empty
string) and assert the validator returns a failing result (result.ok === false)
and that result.checks contains the relevant failing checks such as
'builder_v2_execution_tools_used_is_array' for the tools_used=null case and
'builder_v2_cost_values_are_numeric' for the estimated_usd='' case; place these
alongside the existing PASS case and use the same validateBuilderContract
invocation pattern and check lookups (result.checks.some(check => check.name ===
'...')) to ensure the validator rejects malformed telemetry.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9df0bdd2-dd2b-44f5-955c-55947097857f
⛔ Files ignored due to path filters (1)
docs/mintlify/assets/backups/RStack-The-Future-of-Software-Development.backup.pptxis excluded by!**/*.pptx
📒 Files selected for processing (28)
README.mdbin/rstack-agents.jsdocs/mintlify/concepts/orchestrator-builder-validator.mdxdocs/mintlify/concepts/plugins-and-skills.mdxdocs/mintlify/docs.jsondocs/mintlify/getting-started/builder-validator-sandbox.mdxdocs/mintlify/getting-started/business-flex-profiles.mdxdocs/mintlify/getting-started/installation.mdxdocs/mintlify/introduction.mdxdocs/mintlify/quickstart.mdxdocs/mintlify/reference/business-hub.mdxdocs/mintlify/reference/cli.mdxdocs/mintlify/reference/configuration.mdxdocs/mintlify/reference/loopholes-roadmap.mdxsrc/core/harness/contracts.jssrc/core/profiles.jssrc/integrations/init.jssrc/integrations/pi/rstack-sdlc.tssrc/observability/dashboard/state/business-flex.jssrc/observability/dashboard/state/client-state.jssrc/observability/dashboard/state/index.jssrc/observability/dashboard/state/runs.jssrc/observability/dashboard/ui/client.jssrc/observability/dashboard/ui/pages/index.jstests/dashboard-business-hub.test.jstests/harness-contracts.test.jstests/harness.test.jstests/integrations-init.test.js
| | Open-source agent import is not automated | Copying random GitHub agents can break contracts or licensing. | `rstack-agents validate` checks local definitions. | Add `rstack-agents import agent --license-check --contract-check` for safe adaptation. | | ||
| | Dashboard cannot show missing data that agents never wrote | Empty Business Flex/Studio panels can look like product bugs. | Dashboard reads only real `.rstack` files. | Add diagnostics that explicitly say which contract/event is missing per panel. | | ||
| | MCP/A2A compatibility is conceptual, not native | External tool ecosystems cannot call RStack as a formal protocol server yet. | Universal adapter and filesystem state contract. | Add MCP server exposing `sdlc_start`, `sdlc_plan`, `sdlc_status`, artifact read APIs, and approval APIs. | | ||
| | Package version remains `1.8.0` on this branch | Users may not see this as a new business-flex release. | PR title and docs describe the slice. | Decide release number (`1.9.0` or `2.0.0`) and update changelog/package once accepted. | |
Contributor
There was a problem hiding this comment.
Avoid hard-coding the branch package version in roadmap docs.
Line 43 will drift after the next version bump and can misstate release status. Prefer a version-agnostic statement (or point to the package/changelog as source of truth).
Proposed wording tweak
-| Package version remains `1.8.0` on this branch | Users may not see this as a new business-flex release. | PR title and docs describe the slice. | Decide release number (`1.9.0` or `2.0.0`) and update changelog/package once accepted. |
+| Package release labeling may lag feature delivery | Users may not recognize this as a business-flex release. | PR title and docs describe the slice. | Finalize the release version and update changelog/package metadata before publish. |📝 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
| | Package version remains `1.8.0` on this branch | Users may not see this as a new business-flex release. | PR title and docs describe the slice. | Decide release number (`1.9.0` or `2.0.0`) and update changelog/package once accepted. | | |
| | Package release labeling may lag feature delivery | Users may not recognize this as a business-flex release. | PR title and docs describe the slice. | Finalize the release version and update changelog/package metadata before publish. | |
🤖 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 `@docs/mintlify/reference/loopholes-roadmap.mdx` at line 43, Replace the
hard-coded string "Package version remains `1.8.0` on this branch" in the
roadmap table with a version-agnostic statement (e.g., "Package version
unchanged on this branch — see package changelog for current release") or a
dynamic pointer to the package/changelog as the source of truth; update the
table cell text so it does not contain a specific semantic version and instead
references the changelog/package info.
Comment on lines
+99
to
+106
| if (builder?.execution?.tools_used) { | ||
| const isArray = Array.isArray(builder.execution.tools_used); | ||
| checks.push({ | ||
| name: 'builder_v2_execution_tools_used_is_array', | ||
| status: isArray ? 'PASS' : 'FAIL', | ||
| evidence: isArray ? `${builder.execution.tools_used.length} tool(s)` : 'not an array', | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Validate execution.tools_used based on presence, not truthiness.
Line 99 currently skips validation when tools_used is present but falsy (for example null), which lets malformed telemetry pass without a FAIL check.
Suggested fix
- if (builder?.execution?.tools_used) {
+ if (builder?.execution && hasOwn(builder.execution, 'tools_used')) {
const isArray = Array.isArray(builder.execution.tools_used);
checks.push({
name: 'builder_v2_execution_tools_used_is_array',
status: isArray ? 'PASS' : 'FAIL',
evidence: isArray ? `${builder.execution.tools_used.length} tool(s)` : 'not an array',
});
}🤖 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 `@src/core/harness/contracts.js` around lines 99 - 106, The current guard
(builder?.execution?.tools_used) skips validation for present-but-falsy values
(e.g., null); change the presence test to detect the property itself and always
push a PASS/FAIL check: use a presence check like
Object.prototype.hasOwnProperty.call(builder.execution, 'tools_used') or the
`'tools_used' in builder.execution` form to decide whether to run the
Array.isArray check, and then keep the existing checks.push block (referencing
builder.execution.tools_used and the check name
builder_v2_execution_tools_used_is_array) so null/undefined/non-array values
produce a FAIL with evidence 'not an array'.
Comment on lines
+108
to
+116
| if (builder?.cost?.estimated_usd != null || builder?.cost?.actual_usd != null) { | ||
| const numeric = ['estimated_usd', 'actual_usd'] | ||
| .filter((field) => builder.cost[field] != null) | ||
| .every((field) => Number.isFinite(Number(builder.cost[field]))); | ||
| checks.push({ | ||
| name: 'builder_v2_cost_values_are_numeric', | ||
| status: numeric ? 'PASS' : 'FAIL', | ||
| evidence: numeric ? 'numeric cost telemetry' : 'non-numeric cost telemetry', | ||
| }); |
Contributor
There was a problem hiding this comment.
Avoid coercing cost fields to numbers during validation.
Line 111 treats coercible values as valid numbers (e.g. empty string/boolean), which weakens Contract v2 type guarantees.
Suggested fix
if (builder?.cost?.estimated_usd != null || builder?.cost?.actual_usd != null) {
const numeric = ['estimated_usd', 'actual_usd']
.filter((field) => builder.cost[field] != null)
- .every((field) => Number.isFinite(Number(builder.cost[field])));
+ .every((field) => {
+ const value = builder.cost[field];
+ return typeof value === 'number' && Number.isFinite(value);
+ });
checks.push({
name: 'builder_v2_cost_values_are_numeric',
status: numeric ? 'PASS' : 'FAIL',
evidence: numeric ? 'numeric cost telemetry' : 'non-numeric cost telemetry',
});
}📝 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
| if (builder?.cost?.estimated_usd != null || builder?.cost?.actual_usd != null) { | |
| const numeric = ['estimated_usd', 'actual_usd'] | |
| .filter((field) => builder.cost[field] != null) | |
| .every((field) => Number.isFinite(Number(builder.cost[field]))); | |
| checks.push({ | |
| name: 'builder_v2_cost_values_are_numeric', | |
| status: numeric ? 'PASS' : 'FAIL', | |
| evidence: numeric ? 'numeric cost telemetry' : 'non-numeric cost telemetry', | |
| }); | |
| if (builder?.cost?.estimated_usd != null || builder?.cost?.actual_usd != null) { | |
| const numeric = ['estimated_usd', 'actual_usd'] | |
| .filter((field) => builder.cost[field] != null) | |
| .every((field) => { | |
| const value = builder.cost[field]; | |
| return typeof value === 'number' && Number.isFinite(value); | |
| }); | |
| checks.push({ | |
| name: 'builder_v2_cost_values_are_numeric', | |
| status: numeric ? 'PASS' : 'FAIL', | |
| evidence: numeric ? 'numeric cost telemetry' : 'non-numeric cost telemetry', | |
| }); |
🤖 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 `@src/core/harness/contracts.js` around lines 108 - 116, The current check for
cost fields uses Number(...) which coerces values (e.g., empty strings,
booleans) into numbers and accepts them; update validation to avoid coercion by
ensuring each builder.cost[field] is already a JavaScript number and finite.
Specifically, modify the logic around builder?.cost and the computation for the
'builder_v2_cost_values_are_numeric' check so the filtered fields and .every()
test verify typeof builder.cost[field] === 'number' &&
Number.isFinite(builder.cost[field]) (rather than Number(...) coercion), so only
real numeric types pass.
| for (const plugin of profile.enabled_plugins || []) entry.enabledPlugins.add(plugin); | ||
| for (const page of profile.dashboard_pages || []) entry.dashboardPages.add(page); | ||
|
|
||
| runBudgetTotal += Number(run.budgetPolicy?.run_budget_usd || 0); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Identify runs without budget_policy snapshots in tasks.json that would share project-level budgets.
# Find all tasks.json files and check for budget_policy field
fd -t f '^tasks\.json$' .rstack/runs -x sh -c '
if ! grep -q "\"budget_policy\"" "$1"; then
echo "Missing budget_policy snapshot: $1"
fi
' sh {}Repository: richard-devbot/SDLC-rstack
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Locate all tasks.json files and report which ones lack "budget_policy"
set -euo pipefail
# Find tasks.json anywhere under the repo (fd path is relative to repo root)
tasks=$(fd -t f '^tasks\.json$' .)
echo "$tasks" | wc -l | tr -d ' '
if [ -z "$tasks" ]; then
echo "No tasks.json files found."
exit 0
fi
echo "$tasks" | head -n 50
echo "---- Missing budget_policy snapshots (show up to 200 paths) ----"
# Check each tasks.json for presence of "budget_policy"
echo "$tasks" | while IFS= read -r f; do
if ! rg -q '"budget_policy"' "$f"; then
echo "$f"
fi
done | head -n 200Repository: richard-devbot/SDLC-rstack
Length of output: 99
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show the relevant code in business-flex.js around the referenced line
echo "== business-flex.js =="
ls -la src/observability/dashboard/state/business-flex.js || true
if [ -f src/observability/dashboard/state/business-flex.js ]; then
sed -n '1,120p' src/observability/dashboard/state/business-flex.js | nl -ba | sed -n '1,120p'
fi
# 2) Find where runBudgetTotal is defined/used
echo
echo "== usages of runBudgetTotal =="
rg -n "runBudgetTotal" -S .
# 3) Find runs.js (and inspect the referenced priority logic)
echo
echo "== locate runs.js and relevant budget_policy selection logic =="
fd -t f 'runs\.js$' src . || true
rg -n "tasksRaw\?\.\s*budget_policy|tasksRaw\.budget_policy|budgetPolicy.*run_budget_usd|run_budget_usd" -S src . || trueRepository: richard-devbot/SDLC-rstack
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/observability/dashboard/state/business-flex.js"
echo "== business-flex.js (lines 20-45) =="
awk 'NR>=20 && NR<=45 {printf "%d:%s\n", NR, $0}' "$file"
echo
echo "== usages of runBudgetTotal =="
rg -n "runBudgetTotal" -S src . || true
echo
echo "== find runs.js =="
fd -t f 'runs\.js$' src . || true
echo
echo "== search for budget_policy selection logic (tasksRaw?.budget_policy || budgetPolicy) =="
rg -n "tasksRaw\?\.\s*budget_policy|tasksRaw\.budget_policy|tasksRaw\?\.\s*budgetPolicy|budgetPolicy.*run_budget_usd|run_budget_usd|budget_policy" -S src . || trueRepository: richard-devbot/SDLC-rstack
Length of output: 10781
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect runs.js around the budgetPolicy selection.
echo "== runs.js (around line 150-210) =="
awk 'NR>=130 && NR<=210 {print NR ":" $0}' src/observability/dashboard/state/runs.js
echo
echo "== business-flex.js (around top / loop area) =="
awk 'NR>=1 && NR<=120 {print NR ":" $0}' src/observability/dashboard/state/business-flex.js
echo
echo "== dashboard-business-hub.test.js (around runBudgetTotal assertions) =="
awk 'NR>=1 && NR<=160 {print NR ":" $0}' tests/dashboard-business-hub.test.js
echo
echo "== rstack-sdlc.ts (around tasks.json write with budget_policy) =="
rg -n "writeFile\\(join\\(runDir, \"tasks\\.json\"" -S src/integrations/pi/rstack-sdlc.ts
awk 'NR>=1340 && NR<=1395 {print NR ":" $0}' src/integrations/pi/rstack-sdlc.tsRepository: richard-devbot/SDLC-rstack
Length of output: 18095
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== client.js (around business-flex budget computation) =="
awk 'NR>=420 && NR<=520 {print NR ":" $0}' src/observability/dashboard/ui/client.jsRepository: richard-devbot/SDLC-rstack
Length of output: 8304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find other places that write tasks.json
echo "== Find writes to tasks.json =="
rg -n "writeFile\\(join\\(runDir, \"tasks\\.json\"\\)|tasks\\.json\"\\), JSON\\.stringify\\(|tasks\\.json\\\"\\)\\s*,\\s*JSON\\.stringify" -S src tests || true
echo
echo "== Find other budget_policy references around tasks.json =="
rg -n "tasks\\.json|budget_policy" -S src tests | head -n 200Repository: richard-devbot/SDLC-rstack
Length of output: 3730
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for .rstack/budget.json usage =="
rg -n "\\.rstack/budget\\.json" -S src tests || true
echo
echo "== Inspect src/core/profiles.js around budget policy / run_budget_usd =="
awk 'NR>=1 && NR<=230 {print NR ":" $0}' src/core/profiles.js
echo
echo "== Inspect tests/harness-dashboard.test.js around tasks.json write =="
awk 'NR>=1 && NR<=120 {print NR ":" $0}' tests/harness-dashboard.test.js
echo
echo "== Inspect tests/harness.test.js around budget_policy assertion =="
awk 'NR>=1 && NR<=120 {print NR ":" $0}' tests/harness.test.js
echo
echo "== Inspect tests/people-layer-approvals.test.js around tasks.json read =="
awk 'NR>=80 && NR<=150 {print NR ":" $0}' tests/people-layer-approvals.test.jsRepository: richard-devbot/SDLC-rstack
Length of output: 19735
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "runBudgetTotal" -S tests src | head -n 200Repository: richard-devbot/SDLC-rstack
Length of output: 1352
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "buildBusinessFlexState" -S src testsRepository: richard-devbot/SDLC-rstack
Length of output: 376
Clarify runBudgetTotal double-counting when tasks.json lacks budget_policy.
src/observability/dashboard/state/business-flex.js sums run.budgetPolicy?.run_budget_usd for every run. In src/observability/dashboard/state/runs.js, run.budgetPolicy falls back to the project-level projectRoot/.rstack/budget.json when tasks.json has no budget_policy, so multiple runs can reuse the same shared run_budget_usd and inflate the total. The SDLC integration writes budget_policy into tasks.json, but legacy/other producers could still trigger this—so document the intended semantics or guard/dedupe when tasksRaw?.budget_policy is missing.
🤖 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 `@src/observability/dashboard/state/business-flex.js` at line 32,
runBudgetTotal is summing run.budgetPolicy?.run_budget_usd for every run which
double-counts shared project-level budgets when tasks.json lacks a
budget_policy; update the aggregation so you only add
run.budgetPolicy.run_budget_usd when the run actually declares a budget in its
tasks file (check run.tasksRaw?.budget_policy) or, if you must include
project-level budgets, dedupe by the project identifier (e.g., run.projectRoot
or run.projectId) using a Set before adding; modify the logic around
runBudgetTotal and references to run.budgetPolicy and
run.tasksRaw?.budget_policy accordingly.
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
2 participants
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
business-flex,enterprise-webapp,lean-mvp).rstack/rstack.config.jsonand.rstack/budget.jsongeneration throughrstack-agents init --profile.rstackprofile, budget, and routing dataTest Plan
npm test -- --runInBandnpm run lintnpm run validategit diff --checknpm pack --dry-runCloses #64
Closes #65
Closes #66
Closes #67
Summary by CodeRabbit
Release Notes
New Features
--profileoption for streamlined onboarding.Documentation