boost(docs): 2.1 github staging plan, frontend requirements, RHDHPLAN-1513 deferment details - #4069
Conversation
|
🤖 Finished Review · ✅ Success · Started 8:34 PM UTC · Completed 8:51 PM UTC |
PR Summary by QodoDocument Boost 2.1 staging, frontend scope, and 1513 deferral
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
37 rules✅ Cross-repo context Not relevant to this PR:
redhat-developer/rhdh Not relevant to this PR:
redhat-developer/rhdh-chart Not relevant to this PR:
redhat-developer/rhdh-operator Not relevant to this PR:
redhat-developer/rhdh-local 1.
|
rhdh-qodo-merge
Bot
added
the
documentation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4069 +/- ##
=======================================
Coverage 58.06% 58.06%
=======================================
Files 2411 2411
Lines 96368 96368
Branches 26805 26795 -10
=======================================
Hits 55955 55955
Misses 40224 40224
Partials 189 189
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
ReviewFindingsLow
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsLow
Previous run (3)ReviewFindingsMedium
Low
Previous run (4)ReviewFindingsMedium
Low
Previous run (5)ReviewFindingsMedium
Low
Labels: PR adds planning/specification documents under workspaces/boost/ |
fullsend-ai-review
Bot
added
requires-manual-review
mareklibra
left a comment
mareklibra
left a comment
There was a problem hiding this comment.
Some stale skeletons from not-perfect specifications are emerging. Better now than later.
Let's block implementation of the 4039-4045 issues until resolving similar gotchas. It would be very painful to fix them later in the expected ton of generated code.
|
Thanks for the review findings fullsend review agent. Here's what we're addressing and what we're leaving as-is: Addressed (commit 90285e5)
Not addressing
|
|
in addition to the changes in this PR's files @mareklibra I had claude/opus edit the github issues along these lines: |
|
🤖 Review · ❌ Terminated · Started 7:45 PM UTC · Ended 8:01 PM UTC |
|
OK @mareklibra I'm still arguing with claude/opus about I've told claude/opus in the past I wanted the next push hopefully has resolved claude's confusion, and it should be |
|
🤖 Finished Review · ✅ Success · Started 7:45 PM UTC · Completed 8:01 PM UTC |
|
Also am needing to re-litigate the use of "@boost" as a package name with claude/opus today @mareklibra hopefully with the next push the "@boost" NPM package refs are gone |
|
WRT from fullsend review, I'm leaving things as is for now i.e. leveraging intentional scheduling grouping vs. strict dependency-driven classification |
|
WRT from fullsend review, we are going with |
|
🤖 Finished Review · ✅ Success · Started 8:28 PM UTC · Completed 8:49 PM UTC |
|
for fullsend review simply disregarding .... staged-issues.md does not need to be kept consistent, nor does rhdhplan1505-1-stated-issues.md "supercede" stage-issues.md was the initial drop, rhdhplan1505.... is the next drop will address |
|
🤖 Finished Review · ✅ Success · Started 9:46 PM UTC · Completed 10:02 PM UTC |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ready-for-merge
|
ok don't consider any of the latest fullsend review findings actionable |
mareklibra
left a comment
mareklibra
left a comment
There was a problem hiding this comment.
A few non-critical notes.
I suggest addressing them. Anyway, still LGTM even without it.
… deps, deferral - Fix RHDIP→RHIDP typo in staged-issues.md - Defer Issue 1 group 7 (connector integration) to Tier 1 connector issues - Align config namespace: ai-catalog.providers→catalog.providers in Issue 6 - Clarify Issue 6 Zod schemas target existing Backstage connectors (Jira/GitHub/GitLab) - Add Issue 5 dependency on Issue 3 (ai-catalog.admin permission) - Fix package names: @boost/*→@red-hat-developer-hub/backstage-plugin-* in Issues 2, 4, 10, 13 - Add deferral split note to Issue 4 for RHDHPLAN-1513 contingency - Fix Issue 29 group 5 header: RHIDP-15280/former RHIDP-15344 - Fix RHDHPLAN-1510-frontend-coverage.md RHOAI scope annotation Corresponding GitHub issues updated: redhat-developer#4039, redhat-developer#4040, redhat-developer#4042, redhat-developer#4043, redhat-developer#4044, redhat-developer#4048, redhat-developer#4051, redhat-developer#4067 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: gabemontero <gmontero@redhat.com>
…oss openspecs, staged issues, and feasibility reports Replaces all 68 bare catalog.providers references with ai-catalog.providers across 21 files: - 4 openspec connector dirs (connector-shared-infrastructure, mcp-registry-connector, oci-skill-connector, rhoai-connector) — proposals, designs, tasks, specs - JIRA-analysis staged-issues.md (Issues 1, 6, 7, 13, 16) - JIRA-analysis feasibility reports (RHDHPLAN-1507, RHDHPLAN-1510) connector-config-hot-reload and ingestion-health-dashboard openspecs already used ai-catalog.providers — no changes needed. GitHub issues redhat-developer#4039, redhat-developer#4044, redhat-developer#4045, redhat-developer#4051, redhat-developer#4054 updated separately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: gabemontero <gmontero@redhat.com>
…b/backstage-plugin-* npm names Align openspec package references with Marek's review feedback: - @boost/toolscope → @red-hat-developer-hub/backstage-plugin-boost-toolscope (9 refs) - @boost/entity-provider-sdk → @red-hat-developer-hub/backstage-plugin-boost-entity-provider-sdk (8 refs) - @boost/responses-api-toolkit → @red-hat-developer-hub/backstage-plugin-boost-responses-api-toolkit (2 refs) - @boost/plugin-boost-backend-module-kagenti → @red-hat-developer-hub/backstage-plugin-boost-backend-module-kagenti (1 ref) 20 replacements across 12 openspec files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: gabemontero <gmontero@redhat.com>
Resolve internal-consistency finding: Issue 5 was in Tier 0 (no dependencies) but declared a dependency on Issue 3 for ai-catalog.admin permission used in RBAC gating. Fix: defer RBAC gating task 2.5 from Issue 5 to Issue 26, removing Issue 5's dependency on Issue 3. Issue 26 now depends on both Issue 3 and Issue 5. Dependency graph updated accordingly. GitHub issues redhat-developer#4043 and redhat-developer#4064 updated to match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: gabemontero <gmontero@redhat.com>
…onfig blocks Replace bare `catalog:` top-level key with `ai-catalog:` in 20 YAML config example blocks across 10 openspec files. Code references (`catalog: catalogProcessingExtensionPoint`) are untouched. Addresses fullsend review finding: stale-reference on YAML config blocks that still used the pre-rename `catalog:` namespace instead of `ai-catalog:`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: gabemontero <gmontero@redhat.com>
…plit, §7 deferral - Add two-enabled-flags config note to Issues 1 and 6 in staged-issues.md matching Marek's edits to GitHub redhat-developer#4039 and redhat-developer#4044 - Reword Issue 5 task 2.6: structured LoggerService logging now, full audit emitters deferred to Issue 21 (GitHub redhat-developer#4043 updated in lockstep) - Defer §7 (7.1–7.6) in connector-shared-infrastructure/tasks.md with strikethrough + deferral note matching staged-issues.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: gabemontero <gmontero@redhat.com>
84503d4 to
bbee83f
Compare
|
🤖 Review · ❌ Terminated · Started 8:52 PM UTC · Ended 9:10 PM UTC |
fullsend-ai-review
Bot
added
requires-manual-review
|
🤖 Finished Review · ✅ Success · Started 8:52 PM UTC · Completed 9:10 PM UTC |
…HPLAN-404 scope Apply strikethrough + RHDHPLAN-404 annotation to RHOAI entity mention in the Summary section, matching the pattern used in the table above (lines 53, 55). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: gabemontero <gmontero@redhat.com>
all of marek's reviews have been processed, and he gave an LGTM earlier today
I think this is another case of a req for change reivew followed by a comment review
Fullsend review findings — addressedThe fullsend review run (actions/runs/30852135198) produced 3 actionable findings and 1 informational note. All 3 actionable items are now fixed: Fixed
Not actionable
|
… reclassification, stale package names - Fix tls.ca → tls.caFile typo in Issue 13 task 2.2 - Move Issues 23, 24, 26 from Tier 2 to Tier 1 (they depend only on Tier 0 issues, not Tier 1). Updates header counts, parallelism note, and dependency graph. Tier 1: 15→18, Tier 2: 7→4. - Update 5 stale @boost/* package names in staged-issues.md to @red-hat-developer-hub/backstage-plugin-* convention Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
So fullsend's last review is leading to moving some issues up a tier to better align with dependencies @mareklibra ... it has been raising this consistently ... decided to go with it in the next push |
|
|
🤖 Finished Review · ✅ Success · Started 9:30 PM UTC · Completed 9:45 PM UTC |
fullsend-ai-review
Bot
added
ready-for-merge
|
ok the latest fullsend review findings are repeats we 've previously dismissed: |
|
ok @mareklibra if you are still "LGTM" after updating from you last set of minor comments, plus my latest responses to fullsend review, please do a github review approval so we can merge this thanks |
- Remove unreachable FetchError branch in classifyConnectorError (already covered by generic retryable-code check) - Add rejectUnauthorized: true to createHttpsAgent to guard against NODE_TLS_REJECT_UNAUTHORIZED=0 leaking from dev environments - Switch ca-bundle.ts from namespace imports to named destructured imports to match existing boost workspace conventions - Update connector-shared-infrastructure design doc, tasks, proposal, and spec files to use ai-catalog.providers config root per #4069 - Fix loadCaBundle signature in docs to include logger parameter - Fix createProviderWrapper signature in docs to use ConnectorEntityProvider type and optional ctx parameter - Update exports list in design doc to include classifyConnectorError, validateConnectorStartupConfig, ValidateConnectorStartupConfigOptions, and ConnectorEntityProvider Addresses review feedback on #4144
mareklibra
left a comment
mareklibra
left a comment
There was a problem hiding this comment.
Approving to unblock the whole flow, I will push a small fix in a follow-up
Create @red-hat-developer-hub/backstage-plugin-boost-connector-utils providing shared utilities for AI catalog connector entity providers: - loadCaBundle(config, logger): resolve CA bundles from tls.caFile (file path) or tls.caSecret (resolved $env), with graceful failure (WARN for missing file, ERROR for invalid PEM, no expire check) - createHttpsAgent(caBundle?): factory for https.Agent with custom CA - createProviderWrapper(provider, logger, ctx?): wrap EntityProvider connect() in try/catch to prevent unhandled rejections crashing the catalog backend process - createSafeRefresh(fn, connectorId, logger, ctx?): wrap scheduled refresh callbacks with the same fault isolation pattern - classifyConnectorError(error): classify errors as retryable (ECONNREFUSED, ETIMEDOUT, HTTP 5xx/429) or non-retryable (HTTP 4xx, TypeError, TLS cert errors) - isConnectorEnabled(config): check enabled boolean with default true - validateConnectorStartupConfig(config, opts): reject empty credentials and invalid/non-HTTPS endpoint URLs at startup Includes 47 unit tests covering CA loading (file, env, missing, invalid PEM, chains, isolation), fault isolation (connect crash, refresh crash, no rethrow, multi-provider independence, structured error logging), config guards (enabled/disabled/default), and startup validation (HTTPS URLs, credential fields, descriptive errors). Reference app-config YAML at workspaces/boost/examples/ demonstrates MCP Registry, RHOAI, and OCI Skill connector configuration with an air-gapped deployment variant. Config root: ai-catalog.providers.<id> per #4069 clarification. No Helm charts or Operator CRs (deferred per issue clarification). Closes #4039
- Remove unreachable FetchError branch in classifyConnectorError (already covered by generic retryable-code check) - Add rejectUnauthorized: true to createHttpsAgent to guard against NODE_TLS_REJECT_UNAUTHORIZED=0 leaking from dev environments - Switch ca-bundle.ts from namespace imports to named destructured imports to match existing boost workspace conventions - Update connector-shared-infrastructure design doc, tasks, proposal, and spec files to use ai-catalog.providers config root per #4069 - Fix loadCaBundle signature in docs to include logger parameter - Fix createProviderWrapper signature in docs to use ConnectorEntityProvider type and optional ctx parameter - Update exports list in design doc to include classifyConnectorError, validateConnectorStartupConfig, ValidateConnectorStartupConfigOptions, and ConnectorEntityProvider Addresses review feedback on #4144
2 participants
Hey, I just made a Pull Request!
This processed our RHDHPLAN-1505 related 2.1 stories (RHDHPLAN-1507,1508,1510,1513) against the openspec proposal/design/tasks/specs and build a set of 29 github issues to use with fullsend to code up each feature
the github issues are already opened .... we'll adjust the github issue title and content if any review feedback on their content arises
Also provide details on the frontend reqs for 1507,1508,1510,1513 outside of RHDHPLAN-1509
Lastly posts a proposal on what changes if we defer RHDHPLAN-1513 to 2.2
✔️ Checklist