Skip to content

Test: design review on README.md - #10

Open
ItzikEzra-rh wants to merge 113 commits into
mainfrom
test/readme-design-review
Open

ItzikEzra-rh wants to merge 113 commits into
mainfrom
test/readme-design-review

Conversation

@ItzikEzra-rh

Copy link
Copy Markdown
Owner

Testing OSAC-2136 — README.md design doc detection

tzvatot and others added 30 commits June 2, 2026 15:49
Replace user stories to match the defined OSAC personas (Cloud Provider
Admin, Tenant Admin, Tenant User). The previous stories focused on
implementation details (pre-define fields, prevent overrides) rather
than the persona-driven workflows (create, publish, browse, provision).

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Elad Tabak <etabak@redhat.com>
* OSAC-46: Add VM instance types enhancement proposal

- Phase 1 MVP: Provider-controlled global instance types
- Strict mode: instance_type field required, cores/memory_gib removed
- Enables/disables control visibility to org users
- VMaaS only (ComputeInstance), CaaS uses HostType for bare metal
- GPU support deferred to Phase 2

* Replace boolean enabled field with InstanceTypeState enum

Changes based on PR discussion with mhrivnak and ygalblum:

State management:
- Replace `enabled` boolean with `InstanceTypeState` enum (ACTIVE, DEPRECATED, OBSOLETE)
- Follow GCP deprecation model for lifecycle management
- ACTIVE: Fully available for new VM creation
- DEPRECATED: Available with warnings, includes optional replacement field
- OBSOLETE: Cannot create new VMs (409 Conflict), still visible via GetInstanceType

API behavior:
- ListInstanceTypes: Returns ACTIVE and DEPRECATED by default, supports filter for OBSOLETE
- GetInstanceType: Returns any state (for viewing details of existing VMs)
- CreateComputeInstance: ACTIVE succeeds, DEPRECATED succeeds with warning, OBSOLETE rejected with 409

Summary updates:
- Replace long "In the initial implementation..." with "All instance types are global"
- Highlight that instance types exist only at gRPC API layer, not CRD layer

Other changes:
- Use osac-admin CLI for instance type management operations
- Simplify Proposal section (just mention deprecation mechanism)
- Remove parenthetical visibility qualifications
- Add filter parameter support to ListInstanceTypes
- Update all examples, validation rules, and test cases

Addresses all review feedback from mhrivnak and ygalblum.

Signed-off-by: Avishay Traeger <atraeger@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>

* clarify name as primary identifier and uniqueness model

Address review feedback from ygalblum about ID vs name usage:

- Standardize on 'name' as the primary identifier for InstanceTypes
- Clarify that instance type names are globally unique (Phase 1)
- Update database schema to include UNIQUE constraint on name column
- Update proto definitions to use 'name' field instead of 'id'
- Update API paths to use {name} instead of {id}
- Add new section explaining naming and uniqueness guarantees
- Document future Phase 2 approach for tenant-specific types (prefixed names)
- Update all validation rules to reference "name" consistently
- Clarify that id field is internal database primary key (UUID)

The name field serves as both the user-visible identifier and the
reference used in ComputeInstance.spec.instance_type (e.g., "standard-4-16").

For future tenant-specific types, the proposal recommends a prefixed
naming convention (e.g., "org-name/type-name") to maintain global
uniqueness while supporting organizational customization.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* docs(instance-types): fix troubleshooting terminology to use OBSOLETE and 409

Update troubleshooting section to use consistent lifecycle terminology:
- Change "instance type disabled" to "instance type is obsolete"
- Update HTTP status from 400 Bad Request to 409 Conflict
- Use OBSOLETE/DEPRECATED/ACTIVE terminology consistently
- Update resolution guidance to reference state transitions
- Clarify deletion protection references instance type by name

Addresses CodeRabbit review feedback.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* docs(instance-types): add GCP-style deprecation metadata with timestamps

Add comprehensive deprecation metadata following GCP's machine type
deprecation model to support transparent migration timelines:

Proto Changes:
- Add InstanceTypeDeprecation message with state, replacement, deprecated,
  and obsolete timestamps
- Replace standalone replacement field with deprecation sub-message
- Auto-populate timestamps on state transitions

API Changes:
- Support --obsolete-at flag when deprecating instance types
- Include deprecation metadata in List/Get responses
- Warning messages include obsolete date when creating VMs with DEPRECATED types
- Validate obsolete timestamp is in the future when transitioning to DEPRECATED

Database Changes:
- Store full deprecation metadata in JSONB data field
- Add examples for both ACTIVE and DEPRECATED instance types

Workflow Changes:
- Admin can specify future obsolete date when deprecating
- Users see migration timeline in warnings and list responses
- Deprecation timestamps provide audit trail for lifecycle changes

Testing:
- Validate deprecation metadata in unit and integration tests
- Test warning messages include obsolete dates
- Verify auto-population of timestamps on state transitions

Non-Goals (Phase 1):
- Automatic state transitions based on timestamps (scheduled job deferred)

Reference: https://cloud.google.com/compute/docs/reference/rest/v1/machineTypes

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* docs(instance-types): clarify name uniqueness semantics (never reusable)

Fix contradictory soft-delete uniqueness policy in database schema:
- Remove redundant partial unique index
- Clarify that PRIMARY KEY on name prevents all duplicates
- Document that instance type names are never reusable, even after soft deletion

The PRIMARY KEY constraint already enforces global uniqueness across all rows,
making the conditional unique index redundant and confusing.

Addresses CodeRabbit review feedback.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* docs(instance-types): clarify deprecation timestamp validation rules

Address ygalblum's question about timestamp enforcement:

Deprecation Timestamp Validation:
- deprecated timestamp: Can be past or future (records when deprecation was/will be announced)
- obsolete timestamp: Can be past or future for OBSOLETE state, must be future for DEPRECATED state
- API behavior: Based on state field, not timestamps (future timestamps don't affect current behavior)

Auto-population:
- deprecated: Set to current time when transitioning to DEPRECATED (if not provided)
- obsolete: Set to current time when transitioning to OBSOLETE (if not provided)

Key Design Decision:
State transitions are manual (admin-initiated), not automatic based on timestamps.
Timestamps are for communication and planning, not enforcement.

This allows admins to:
- Backdate deprecated timestamp when recording historical decisions
- Set future obsolete dates for migration planning
- Manually control all state transitions

Addresses review comment on line 288.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* docs(instance-types): clarify manual state transitions in Phase 1

Fix workflow contradiction - remove misleading "(Future)" note that suggested
automatic transitions might happen:

- Change workflow step 6 from "(Future) Scheduled job automatically transitions"
  to explicit note that Phase 1 requires manual transitions
- Keep Non-Goals bullet consistent: automatic transitions are out of scope
- Clarify that Cloud Provider Admin must manually transition to OBSOLETE

Phase 1 Behavior:
- Timestamps communicate migration timeline to users
- Admins manually trigger DEPRECATED → OBSOLETE transitions
- No scheduled jobs or automatic state changes

Future Phase:
- Scheduled job could auto-transition based on obsolete timestamp
- Remains explicitly out of scope for Phase 1

Addresses CodeRabbit review feedback about contradictory statements.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* docs(instance-types): add warnings field to CreateComputeInstanceResponse

Address CodeRabbit review feedback - specify API contract for DEPRECATED warnings:

Proto Changes:
- Add CreateComputeInstanceResponse message with warnings field
- warnings: repeated string field containing deprecation messages
- Example: "Instance type 'standard-2-4' is deprecated and will become obsolete on 2026-12-31. Consider migrating to 'standard-2-8'."

Updated Documentation:
- Workflow: Show CreateComputeInstanceResponse with warnings array
- Validation: Reference CreateComputeInstanceResponse.warnings field explicitly
- API behavior: Specify DEPRECATED returns warning in response.warnings field

Warning Format:
- Empty array for ACTIVE instance types
- Contains deprecation message with replacement and obsolete timestamp for DEPRECATED types

This provides a clear API contract for returning warnings consistently across
all CreateComputeInstance operations.

Addresses review feedback on lines 202-203.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* docs(instance-types): update last-updated date to 2026-05-17

Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* docs(instance-types): address reviewer feedback

- Add update_timestamp field for auditing (ygalblum)
- Clarify soft-delete pattern used by DeleteInstanceType (ygalblum)
- Change deletion error to not leak VM details, just indicate in-use (ygalblum, mhrivnak)
- Add Private API List/Get operations for completeness (ygalblum)
- Document labels/annotations reserved for future extensibility (ygalblum)
- Explain JSONB usage follows OSAC standard pattern (ygalblum)
- Add note about bidirectional state transitions (ygalblum)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* docs(instance-types): document bidirectional state transitions

Update lifecycle descriptions to show ACTIVE ↔ DEPRECATED ↔ OBSOLETE
with bidirectional arrows and add Reactivating workflow section.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

---------

Signed-off-by: Avishay Traeger <atraeger@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## What
Add a ComputeImage API resource for managing VM images as first-class entities in OSAC.

## Why
Today, ComputeInstance creation accepts arbitrary image URLs with no access control, discoverability, or governance. Every major cloud provider treats VM images as managed resources — OSAC needs the same to be a credible VM-as-a-Service platform.

## Changes
- Introduce ComputeImage resource with CRUD API for registering, listing, and managing VM image metadata
- Support two-tier visibility: provider-global images (all tenants) and tenant-scoped images
- Define image lifecycle states: AVAILABLE, DEPRECATED (with warnings), and OBSOLETE (blocks new VMs)
- Update ComputeInstance to reference a registered ComputeImage by ID instead of accepting arbitrary URLs
- Include deletion protection preventing removal of images referenced by active ComputeInstances
- Define authorization model for provider admins, tenant admins, and tenant users
- Specify database schema, list filtering, deprecation metadata, and migration strategy

## Testing
Comprehensive test plan covering unit tests for CRUD and validation, integration tests for multi-tenant visibility and lifecycle transitions, and end-to-end tests for the full image registration through VM provisioning workflow.

Assisted-by: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
This update provides a more detailed architecture that more closely
matches k8s patterns.
* OSAC-1118: Add BareMetal Instance API enhancement proposal

Signed-off-by: Adrien Gentil <agentil@redhat.com>

* Address review: refine user stories and workflow descriptions

Signed-off-by: Adrien Gentil <agentil@redhat.com>

* Address CodeRabbit review comments

Signed-off-by: Adrien Gentil <agentil@redhat.com>

* Address review: immutable ssh_key/user_data, quota as non-goal

Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: update initial backend from OpenStack to BCM

Scope change: BCM (NVIDIA Base Command Manager) is the first
supported backend, replacing OpenStack/MOC references throughout
the EP.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: keep API backend-agnostic, BCM only in admin/impl context

Public API (proto field comments, tenant user stories, non-goals)
must not leak BCM as the backend. BCM is referenced only where
relevant: Cloud Infrastructure Admin user story, workflow actors,
risks, support procedures, and infrastructure section.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: simplify proto field comments in BaremetalInstanceTemplateSpecDefaults

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: address CodeRabbit review comments

- Non-goals: document interim workarounds for deferred networking
  (user_data/cloud-init) and custom profile deferral
- Pluggable provider: add HostLease → BaremetalInstanceStatus mapping table
- Test plan: remove quota enforcement from unit tests (quota is non-goal)
- Credential risk: defer to fulfillment-service cross-cutting policy
  instead of prescribing storage mechanisms

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: defer credential management to a dedicated security enhancement

Credential storage is undefined and not owned by fulfillment-service.
Only constraint stated: no credentials in CRDs or API responses.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: remove HostLease→BaremetalInstance mapping table (HostLease WIP)

Mapping will be defined once the HostLease CRD schema is finalized.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: fix networking non-goal — network config owned by admin, not tenant

Tenants have no mechanism to configure networking; it is defined by
the Cloud Provider Admin in the BaremetalInstanceTemplate.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: remove Pluggable Provider Architecture section

Pluggable backend interface definition is owned by OSAC-1032.
This EP covers BCM integration against that interface, not the
interface itself.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: clarify networking non-goal is phase-scoped, not permanent

Network config via VirtualNetwork/Subnet/SecurityGroup is deferred to
a dedicated networking enhancement, not removed from scope entirely.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: close open question on BaremetalInstance vs HostLease coupling

HostLease remains distinct from BaremetalInstance because it serves
cases outside fulfillment-service scope (e.g. cluster BM nodes).

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: empty BaremetalInstanceTemplateSpecDefaults, no fields yet

spec_defaults follows template convention but no tenant-overridable
fields are defined in this initial version.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: clarify BaremetalPool+HostLease may both be used initially

Making pool optional is not a prerequisite; both CRs may exist in the
initial implementation as internal details of the BMF component.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: rearchitect provisioning flow — FS creates HostLease directly

- fulfillment-service creates HostLease CR directly (no BareMetalPool,
  no osac-operator CR creation step)
- baremetal-fulfillment-operator owns inventory assignment and AAP
  provisioning trigger
- osac-operator watches HostLease for status and signals back via
  Signal RPC
- Add provisioning sequence diagram
- Update actors, workflow steps, drawbacks, alternatives, version skew,
  and support procedures to match

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: address carbonin review comments

- Remove pluggable provider abstraction from Goals (implementation
  concern of baremetal fulfillment component, not API scope)
- Remove "no intermediate CRD" callout per carbonin's suggestion
- Fix step numbering gap in provisioning sequence (9→8)
- Note that delete+recreate retry may assign a different host
- Remove ip_address from BaremetalInstanceStatus and mermaid diagram;
  deferred until networking integration is scoped
- Simplify Drawbacks: drop consolidation language (BaremetalInstance
  and HostLease would never be the same object)
- Simplify Alternatives: same reasoning, remove "future consolidation
  breaking change" framing

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: clarify run_strategy field comment

Explain that run_strategy controls power state and note why an enum
is used over a boolean, per tzumainn's review feedback.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: describe BMF/osac-operator division of responsibility

Per carbonin's feedback, add a note near the non-goals clarifying the
intended pattern: baremetal-fulfillment-operator handles host assignment
and provisioning; osac-operator handles status feedback via Signal RPC.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: fix non-goals formatting, inline BMF/operator note

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: remove osac-operator non-goal bullet and inline note

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: clear drawbacks section

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1118: remove HostLease-as-API alternative

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

---------

Signed-off-by: Adrien Gentil <agentil@redhat.com>
Introduce a Snapshot API for OSAC ComputeInstance resources, enabling
tenants to create, list, get, delete, and restore from point-in-time
snapshots of their VMs through the fulfillment-service API.

Key design decisions:
- New Snapshot resource (proto + CRD + dual controller)
- Both online and offline snapshot modes via KubeVirt
- In-place restore via declarative signal on ComputeInstance
- Cascade deletion when parent ComputeInstance is deleted

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
* OSAC-1317: rename BaremetalInstance → BareMetalInstance (case fix)

BaremetalInstance → BareMetalInstance
BaremetalInstanceTemplate → BareMetalInstanceTemplate
BAREMETAL_INSTANCE_* → BARE_METAL_INSTANCE_* (enum values)

Mechanical rename only; no API or semantic changes.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1317: adopt BareMetalInstanceCatalogItem API pattern

Replace BareMetalInstanceTemplate with BareMetalInstanceCatalogItem as the
tenant-facing resource, following the ComputeInstanceCatalogItem pattern.

- BareMetalInstanceCatalogItem gains: template (ref to private resource),
  published, tenant, field_definitions fields
- BareMetalInstanceSpec.catalog_item replaces spec.template
- Private admin CRUD service added (BareMetalInstanceCatalogItems)
- Public API restricted to List/Get on published catalog items
- Private REST routes added under /api/private/v1/
- Workflow, proto definitions, mermaid diagram, test plan, non-goals,
  support procedures, and alignment section updated throughout
- see-also: /enhancements/catalog-items added to frontmatter

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1317: add log hygiene note to Support Procedures

Address CodeRabbit No-Sensitive-Data-In-Logs check: operators must redact
ssh_key, user_data, and backend credentials before inspecting or sharing
logs. Treat any such exposure as a security incident.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1317: remove stale non-goal about tenant scoping

The tenant field is fully defined in the BareMetalInstanceCatalogItem proto
(empty = global, set = tenant-scoped) and relied upon in the proposal, test
plan, and support procedures. The non-goal claiming it was deferred
contradicted the rest of the document.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1317: add BareMetalInstanceTemplate proto and private service

Both BareMetalInstanceTemplate and BareMetalInstanceCatalogItem are now
fully specified:

- Add BareMetalInstanceTemplate proto (id, metadata, title, description,
  spec with host_type and os_image)
- Add BareMetalInstanceTemplates private gRPC service (full CRUD)
- Add private REST routes for baremetal_instance_templates

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1317: fix summary and consolidate non-goals

- Correct template/catalog-item relationship: each catalog item is
  backed by a BareMetalInstanceTemplate (not the reverse)
- Clarify that templates are created via osac-aap; catalog items are
  managed through the OSAC private API
- Consolidate four "covered in companion work" non-goals into one line

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1317: align BareMetalInstance API with ComputeInstance pattern

- Add Tenant Admin user story for tenant-scoped catalog items
- Make BareMetalInstanceTemplate and BareMetalInstanceCatalogItem fully
  public (full CRUD), consistent with ComputeInstanceTemplate and
  ComputeInstanceCatalogItem
- Public BareMetalInstanceCatalogItem omits tenant field (server-managed)
- Show public/private proto variants for BareMetalInstanceCatalogItem
- Add explicit consistency goal instead of scattered pattern references
- Update API Extensions, REST routes, actors, and test plan accordingly

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1317: show catalog item resolution in provisioning sequence

The fulfillment service resolves catalog_item to templateID and derives
templateParameters from field_definitions internally before creating the
HostLease CR. Update sequence diagram and provisioning step 4 to reflect
this — HostLease carries templateID + templateParameters, not catalogItemID.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1317: fix mermaid syntax in provisioning sequence diagram

Replace <br/> with a single-line note — GitHub's mermaid renderer
does not support HTML self-closing tags in Note blocks.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

* OSAC-1317: restrict public BareMetalInstanceTemplates to List/Get only

Templates are managed through the private API by Cloud Provider Admins;
tenants can discover available templates via public List/Get only.
This matches the ComputeInstanceTemplates authconfig pattern.

Update Goals, Proposal, Actors, Provisioning step 1, API Extensions,
REST routes, and Alignment section accordingly.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Adrien Gentil <agentil@redhat.com>

---------

Signed-off-by: Adrien Gentil <agentil@redhat.com>
Introduces the split PRD + design doc format for the TenantStorage CRD
and OSAC Storage Controller extraction from the Tenant controller.

Includes the AAP playbook split (OSAC-1145) as part of the delivery scope.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
PRD should be created, reviewed, and merged before the design doc.
Design will come in a follow-up PR.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
- Storage Controller watches Tenant CRs and creates TenantStorage (not Tenant controller)
- Controller-driven deletion (no owner reference GC)
- ClusterOrder watch included in v0.1 scope
- Tenant CRD cleanup (remove storage fields) in scope
- Phase 1/Phase 2 terminology defined explicitly
- Remove internal workflow markers ([Clarify: D1] etc.)

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
Storage Controller sets StorageReady condition on the Tenant CR (and
future ClusterOrder CR) so storage readiness is visible through the
primary resources users interact with. The Tenant controller still
runs no storage logic.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
Reorganize requirements into Controller/API Foundation and Storage
Onboarding Workflow sections to make the dependency explicit: storage
must be extracted from the Tenant controller before onboarding workflows
can run independently.

Introduce a two-stage model for tenant storage provisioning. Stage 1
(backend setup) creates tenant credentials and sets StorageBackendReady
on the Tenant. Stage 2 (cluster-side setup) discovers StorageClasses on
target clusters. For VMaaS, both stages run at tenant onboarding. For
CaaS, Stage 2 runs after ClusterOrder reaches Ready.

Add per-cluster entries array to TenantStorage status so each cluster
tracks its own storage readiness and delivery model. Add migration risk
for existing tenants and open questions for delivery model detection and
migration procedure.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Remove verbose phrasing, clarify the storage controller scope
(clusters, compute instances, PVCs), and tighten wording throughout
for readability.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
The OSAC Storage Controller manages storage conditions and status
fields directly on the Tenant CR instead of introducing a separate
TenantStorage CRD. This follows the condition ownership pattern
where multiple controllers manage distinct conditions on a shared
resource.

Key changes from the previous PRD revision:
- StorageBackendReady and StorageClassReady conditions on Tenant CR
- status.storageClasses and status.jobs ownership moves to storage controller
- ComputeInstance controller unchanged (reads from Tenant as before)
- Finalizer on Tenant CR for deletion ordering
- AAP playbooks renamed: osac-create-tenant-storage-backend,
  osac-create-tenant-storage-class, osac-delete-tenant-storage-class,
  osac-delete-tenant-storage-backend
- StorageClasses created by AAP (not manually), controller discovers
  them via osac.openshift.io/tenant label
- VMaaS target cluster is remote or hub (statically configured)
- Removed Risk 7.2 (migration): pre-GA, no existing tenants
- Resolved all open questions
- Added NFR for credential-safe logging
- Added E2E testing acceptance criterion

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Clarify that status.storageClasses uses the same tier resolution
algorithm (tenant-specific with Default fallback) previously owned
by the Tenant controller. Tighten FR-5 to require the hub Secret
exists and is valid before marking StorageBackendReady=True.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
- Remove Risk 7.1 (pre-GA, breaking changes are expected)
- Move API field names from FR-1 to design doc scope
- Fix NFR-1: admin credentials are in a pre-configured Secret,
  not "never persisted to Kubernetes"

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Zoltan Szabo <zszabo@redhat.com>
FR-1 now requires per-backend detail (name, provider, status, error)
and per-cluster detail (cluster name, readiness, reason) in the
Tenant status. Clarifies that status.jobs is a shared field used by
multiple controllers, not owned by the storage controller.

For this PRD, cluster storage readiness covers the VMaaS target
cluster only. CaaS clusters are covered by a separate PRD.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Rename osac-create-tenant-storage-class to
osac-create-tenant-cluster-storage and osac-delete-tenant-storage-class
to osac-delete-tenant-cluster-storage. "cluster-storage" better
describes the scope (CSI operator, CSI Secrets, StorageClasses) than
"storage-class" which only captures one artifact.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Add "Tier addition and removal workflows" to non-goals per Avishay's
feedback. Scope the management-state Unmanaged check to steady-state
reconciliation only so that finalizer and deletion handling still run,
preventing Tenants from being stuck in Terminating.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
* OSAC-1111: Add StorageBackend enhancement proposal

Signed-off-by: Roy Golan <rgolan@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>

* OSAC-1111: Address EP review findings

- Add README.md index with YAML frontmatter and stub section headers
- Clarify PENDING→READY state transition: manual via Signal RPC (Phase 1),
  not READY-on-create (NetworkClass) or full reconciler (PublicIPPool)
- Explain Signal RPC divergence from NetworkClass (status payload needed
  because fulfillment-service has no direct knowledge of array state)
- Fix description field: mark as `optional string` in proto appendix
- Document soft-delete name reuse as intentional (tiers reference by ID)
- Fix copyright year: 2025 → 2026
- Resolve Open Question 1: hub = management cluster in OSAC HCP model
- Standardize "management cluster" terminology throughout design.md
- Fix tracking-link to issues.redhat.com

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>

* remove design.md, it will come later

Signed-off-by: Roy Golan <rgolan@redhat.com>

* OSAC-1111: Rework PRD to structured requirement format

Restructure the StorageBackend PRD from narrative format to the numbered
section template with FR-N/NFR-N requirement IDs, acceptance criteria
checkboxes, and structured risks/assumptions/dependencies sections.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>

* OSAC-1111: Remove implementation details from PRD

Reframe requirements in terms of product capabilities rather than
implementation choices (PostgreSQL, FieldMask, buf lint, SQL indexes).
Addresses review feedback to keep the PRD scoped to product requirements.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>

* OSAC-1111: Address review feedback round 2

- Remove Risk 7.1 (proto appendix divergence) — coding concern, not
  product requirement
- Clarify CSI drivers and StorageClasses install on target clusters
  (VMaaS/CaaS), not management cluster
- Renumber remaining risks

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>

* OSAC-1111: Clarify FR-4 and FR-8 per review feedback

- FR-4: Note that pagination/filtering/ordering follows the established
  OSAC List API pattern used by all existing entities
- FR-8: Define each state meaning (UNSPECIFIED, PENDING, READY, FAILED)
  and explain why Phase 1 transitions are manual (no reconciler)

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>

* OSAC-1111: Simplify state lifecycle to UNSPECIFIED and READY only

Remove PENDING and FAILED states — they will be introduced when
reconciliation or health probing is added in a future phase. Backends
are created with initial state READY.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>

* OSAC-1111: Remove UNSPECIFIED state, keep READY only

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>

---------

Signed-off-by: Roy Golan <rgolan@redhat.com>
…permissions (#38)

* Update organizations & authentication enhancement

Key updates:
- Clarify multi-cluster scope (infrastructure clusters, not CaaS)
- Add project-scoped resource access pattern (project switcher)
- Defer cross-project views to future enhancement
- Answer deletion lifecycle semantics (block by default, optional cascade)
- Define hierarchical project permissions policy
- Add Keycloak Authorization Services integration details (local RPT validation)
- Clarify project permission model and resource listing
- Align scope examples with OSAC resources (CREATE_COMPUTE_INSTANCE)
- Note that specific scopes are implementation details
- Update last-updated date to 2026-05-06

Addresses feedback from Crystal Chun and Juan Antonio Hernandez Fernandez
on multi-cluster architecture, project permissions, and resource visibility.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* Address CodeRabbit review comments

- Split long summary paragraph into 5 focused paragraphs for readability
- Clarify that local RPT validation is the recommended approach (not token introspection)
- Add reference link to detailed RPT validation section

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* Fix CodeRabbit review findings

- Fix HTTP code block formatting (add Content-Type header, use http syntax)
- Clarify hierarchical permission propagation is consistent across document
- Expand RPT validation to include full JWT validation details (JWKS/JWS, claims, time checks)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* Address CrystalChun's review comments

- Clarify hierarchical permissions: MANAGE_PROJECT and VIEW_PROJECT cascade from parent to child projects, but resource-specific permissions do not (line 248)
- Clarify that the 'default' project does not grant automatic access to all org users - permissions must be explicitly granted (line 165)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

* Address mhrivnak's scalability concerns

- Add implementation note about on-demand OpenShift Project creation to avoid N×M scaling issues
- Note that database queries need efficient indexing on project_id and organization_id
- Clarify that OpenShift Projects can be created on-demand or upfront as implementation choice

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Avishay Traeger <atraeger@redhat.com>

---------

Signed-off-by: Avishay Traeger <atraeger@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* OSAC-23: add design document for tenant storage onboarding

Introduces a dedicated OSAC Storage Controller that extracts all storage
provisioning logic from the Tenant controller. The controller manages a
two-stage onboarding workflow (backend setup, then cluster-side
StorageClass installation) and a two-step ordered teardown, using
condition ownership on the Tenant CR.

Key design decisions:
- Two conditions on Tenant CR: StorageBackendReady, ClusterStorageReady
- Two ProvisioningProvider instances (backend + cluster-storage)
- Tenant Phase=Ready decoupled from storage readiness
- Four AAP playbook lifecycle actions
- Per-backend and per-cluster status detail structs

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>

* OSAC-23: use annotation-based ClusterOrder watch mapping

ClusterOrder and Tenant live in different namespaces, so
ownerReferences cannot be used. Replace EnqueueRequestForOwner with
EnqueueRequestsFromMapFunc that reads the osac.openshift.io/tenant
annotation from ClusterOrder to enqueue the correct Tenant.

This is consistent with how ComputeInstance and all other OSAC
resources associate with Tenants via the same annotation, set by
the fulfillment-service at CR creation time.

Also incorporates review feedback: management state scoping (FR-4
steady-state only), label-based hub Secret lookup, no-auto-retry
on failure, operator-first deployment order, priority=1 print
columns, per-tenant management-state in support procedures, and
tier addition workflows as non-goal.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>

* OSAC-23: add extension points for multi-backend and tier source

Frame the reconciliation loop around two pluggable sources (backend
list and tier source) so the controller logic can evolve from single
VAST backend to multiple backends (OSAC-1111) and from STORAGE_TIERS
env var to StorageTier CRs (OSAC-1110) without rewriting the core
loop or changing the status schema.

Clarify that v0.1 storageBackends and clusterStorage lists each have
a single entry, with the list structures anticipating multi-backend
and multi-cluster support.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>

* OSAC-23: update last-updated date to 2026-06-15

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>

* OSAC-23: clarify implicit stage derivation in reconciliation steps

Add parenthetical notes to make the stage sequencing explicit for
code reviewers: Secret absence indicates Stage 1 has not run,
StorageBackendReady=True indicates Stage 1 is complete.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>

* OSAC-23: tighten design document prose

Remove source markers, cut redundant sentences, shorten failure
table, simplify alternatives and boilerplate sections, remove
inline Go comments from self-documenting structs. Preserves all
technical content in a more concise form.

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>

* OSAC-23: rename design document from README.md to design.md

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Cursor/Claude
Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>

---------

Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
- Replace credentials_ref (K8s Secret reference) with inline
  credentials (username/password) stored in JSONB, consistent
  with all existing OSAC entities
- Remove Signal RPC — no reconciler, no consumer
- Update PRD to match (FR-3, FR-10, open questions)

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
Tenants don't need direct access to StorageBackend — they
interact with storage through StorageTier (OSAC-1110). Removed
public API proto, server, tests, and all references.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
…dated

Add name validation constraints to StorageBackend design:
- metadata.name is required on Create (must be non-empty)
- metadata.name is immutable on Update (cannot be changed after creation)
- DNS label format enforced by generic server (RFC 1035)
- Uniqueness already covered by the partial index (FR-9)

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
Replace StorageBackendEndpoint message (host + port) with a plain string
field that accepts a URL or host:port. Simpler and more flexible.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
…rd-delete)

The generic DAO uses soft-delete by default. Switching to hard-delete
requires DAO extension or custom server logic. Mark as OQ-1 for review.

Also adds referential integrity check: delete is rejected if any
StorageTier references the backend.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
…ategy

OQ-1 resolution: soft-delete by default (standard DAO), hard-delete
when purge=true on the request. Both modes enforce referential
integrity (reject if StorageTiers reference the backend).

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
Replace soft-delete/archive as user-facing decommission with state-based
lifecycle:
- READY: operational (phase 1)
- MAINTENANCE: temporary unavailability, reversible (phase 0.2)
- DECOMMISSIONED: terminal, backend retired but visible (phase 0.2)

Delete RPC permanently removes the record. In phase 0.2, only allowed on
DECOMMISSIONED backends with no StorageTier references. The archived table
remains as DAO infrastructure but is not user-facing.

Resolves OQ-1.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
danmanor and others added 25 commits July 2, 2026 11:37
Signed-off-by: Dan Manor <dmanor@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Dan Manor <dmanor@redhat.com>
Remove stale region reference in ComputeInstanceSpec comment, fix
CaaS prerequisite flow to say "resolved" instead of "resolved/created".

Signed-off-by: Dan Manor <dmanor@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Dan Manor <dmanor@redhat.com>
Signed-off-by: Dan Manor <dmanor@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Dan Manor <dmanor@redhat.com>
Adds a GitHub Action that triggers on PRs containing prd.md or design.md.
Uses agentic-ci with Podman backend to run Claude Code review against
the prd-review or ep-review skill from osac-workspace.

Files:
- .github/workflows/ep-review.yml — workflow triggered on PR events
- .github/scripts/ep_review.py — entry point: detects skill, runs review
- .github/scripts/ep_hooks.py — agentic-ci hooks (context, prompt, verdict, comment)
- .github/scripts/ep_skill_config.py — SkillConfig builder

Starts in shadow mode (EP_REVIEW_SHADOW=true) — reviews run but
no comments are posted until verified.

Requires secrets: GCP_SA_KEY, GCP_PROJECT, GCP_REGION

Part of OSAC-1773

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. SHA in comment body — include head SHA for dedup check
2. _gh error handling — raise on failure for write ops (check=True)
3. detect_skill returns all matches — PRs with both prd+design get both reviews
4. Stale run guard — abort if live headRefOid differs from trigger SHA
5. ImportError fails in CI — only dry-run locally
6. Concurrency group — cancel older runs on same PR
7. Prompt injection boundary — context files treated as data only
8. Pin action versions to commit SHAs
9. Pin dependencies via requirements.txt
10. validate_scores checks expected rubric keys
11. Remove unused skill_path param from build_skill_config

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…working PRD

- Replace "AWS VPC or Azure VNet" with "cloud VPC or VNet"
- Add region-scoped networking to non-goals (not yet defined in OSAC)

Signed-off-by: Dan Manor <dmanor@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Dan Manor <dmanor@redhat.com>
Signed-off-by: Dan Manor <dmanor@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Dan Manor <dmanor@redhat.com>
Region is not yet a defined concept in OSAC. Remove all references
from terminology, problem statement, user stories, and requirements.

Signed-off-by: Dan Manor <dmanor@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Dan Manor <dmanor@redhat.com>
Rename creators→creator and tenants→tenant in storage_tiers table
definition to match migrations 40 and 41.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
OSAC-1110: Design: StorageTier API
…g-auto-externalip

NO-ISSUE: Fix terminology and add region to non-goals
Add BareMetalInstanceImage message and image field to BareMetalInstanceSpec,
template spec defaults, and FieldDefinition documentation. Move OS image
selection from Non-Goals to Goals. Update provisioning workflow and sequence
diagram to show image propagation through the pipeline.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: MENNY ABOUSH <maboush@maboush-thinkpadt14gen5.raanaii.csb>
…rageTier PRD

Align StorageTier delete semantics with StorageBackend soft-delete pattern,
add missing acceptance criteria for name uniqueness and no-cascade delete,
clarify OSAC-23 integration via gRPC API (not CRs), fix incorrect FR-10
cross-reference, and justify ACTIVE vs READY state naming.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
OSAC-1110: PRD: StorageTier API
OSAC-1971: add OS base image field to BareMetalInstance EP
…h-action

OSAC-1774: Add GitHub Action for automated EP review via agentic-ci
Add workflow_dispatch (PR number input) and issue_comment (/review-ep)
triggers so the EP review agent can run on existing PRs, not just new
ones. Extracts PR resolution into a dedicated job that all triggers
feed into.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Itzik Ezra <iezra@redhat.com>
…anual-ep-review-trigger

OSAC-2001: add /review-ep comment trigger for EP review workflow
The resolve-pr job had pull-requests: read, but adding reactions on PR
comments requires pull-requests: write since GitHub routes PR comments
through the pulls API. This caused the Acknowledge comment step to fail
with HTTP 403, skipping the entire review pipeline.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Eran Cohen <eranco@redhat.com>
…iew-permissions

OSAC-2050: fix resolve-pr job permissions for PR comment reactions
The validate_scores post-gate always validated against PRD keys
because the ticket argument wasn't passed by the agentic-ci
framework. Auto-detect review type from actual score keys instead,
matching what apply_labels already does.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix design review post-gate rejecting valid design scores
…lenames

The enhancement template tells authors to name design docs README.md,
but the EP Review Action only triggered on lowercase design.md. Now
handles Design.md, DESIGN.md, and README.md inside enhancements/.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…detection

OSAC-2136: detect design docs in README.md and case-insensitive filenames
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

AI Design Review: EP-10

Score: 0/8 | Verdict: FAIL

Criterion Score Notes
Feasibility 0/2 The PR diff adds only a trailing newline to an existing file. No implementation details, proto schemas, lifecycle operations, error handling, risks, or mitigations are introduced or modified. There is
Testability 0/2 No test plan, test strategy, or graduation criteria are introduced or modified in this diff. The change is a single trailing newline with no functional impact.
Scope 0/2 No summary, motivation, user stories, goals, non-goals, or alternatives are introduced or modified. The diff contains no scope-defining content whatsoever.
Architecture 0/2 No architectural decisions, resource definitions, API extensions, controller patterns, dependency analysis, or integration descriptions are introduced or modified. The diff is a whitespace-only change

Verdict: The PR adds only a trailing newline to an existing EP file with no substantive design content — it is a test PR (OSAC-2136) for design doc detection and contains nothing to review against the EP rubric.

Feedback: This PR is explicitly a test for design review tooling detection (per the PR title and body) and contains no design content — only a trailing newline addition. If the intent is to test the review pipeline, consider submitting a PR with actual EP content (even a minimal draft) so the reviewer can exercise the full rubric. As-is, every scoring dimension receives a zero because there are no design decisions, implementation details, test plans, or scope definitions to evaluate.

Critical (2)

  1. No design content present: The entire diff is a single trailing newline addition (+1 empty line). All required EP sections (Summary, Motivation, Proposal, Test Plan, etc.) are untouched. There is no enhancement proposal to review.
  2. PR appears to be a tooling test (title: 'Test: design review on README.md', body: 'Testing OSAC-2136') rather than a genuine design submission.

Important (0)

None.

Suggestions (1)

  1. If this PR is meant to validate the design review pipeline, re-submit with at least a minimal draft EP containing substantive content in the required template sections so the review skill can produce meaningful scores and feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.