chore(xtest): Enables pqc on platform checkouts#484
Conversation
📝 WalkthroughWalkthroughPins CI startup actions to PQC-enabled revisions and passes ChangesPQC and Hybrid TDF Test Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables the hybrid_tdf_enabled preview setting in the local KAS service configuration when key management is active. It also adds a draft specification document (spec/DSPX-3499.md) for handling pqc and hybrid pq/t tests. However, the specification document currently contains placeholder text for several key sections that should be fully documented before merging.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
X-Test Failure Report |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
otdf-local/src/otdf_local/services/kas.py (1)
1-229:⚠️ Potential issue | 🟠 MajorRun Python quality gates for
otdf-local(ruff + pyright) before merge: the attempted checks didn’t run becauseuvwasn’t available (uv: command not found). Fromotdf-local/, run:
uv run ruff check .uv run ruff format .(and re-stage any changes)uv run pyright🤖 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 `@otdf-local/src/otdf_local/services/kas.py` around lines 1 - 229, The CI-quality checks for this module failed because the repo-level task runner "uv" was not available; from the otdf-local root run the requested linters and type checks (uv run ruff check ., uv run ruff format . then re-stage, and uv run pyright); if "uv" is not installed on your environment, run ruff and pyright directly (ruff check ., ruff format . then git add any formatted files, and pyright) and fix any reported issues in this file (look for KASService, KASManager, and get_kas_manager) before re-running the checks.
🤖 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 @.github/workflows/vulnerability.yml:
- Around line 39-42: The workflow startup step that invokes the action
"opentdf/platform/test/start-up-with-containers@626ce47dd662cb8ff16898e3b6727001a4753d92"
currently doesn't pass the pqc-enabled input; update that action invocation (the
step using that "uses" string) to include the input kv pair pqc-enabled: true
under its with: block so the startup runs with PQC explicitly enabled.
---
Outside diff comments:
In `@otdf-local/src/otdf_local/services/kas.py`:
- Around line 1-229: The CI-quality checks for this module failed because the
repo-level task runner "uv" was not available; from the otdf-local root run the
requested linters and type checks (uv run ruff check ., uv run ruff format .
then re-stage, and uv run pyright); if "uv" is not installed on your
environment, run ruff and pyright directly (ruff check ., ruff format . then git
add any formatted files, and pyright) and fix any reported issues in this file
(look for KASService, KASManager, and get_kas_manager) before re-running the
checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c822ec94-7fc8-4e60-8cbb-13d7313d26eb
📒 Files selected for processing (4)
.github/workflows/vulnerability.yml.github/workflows/xtest.ymlotdf-local/src/otdf_local/services/kas.pyspec/DSPX-3499.md
dmihalcik-virtru
changed the title
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…al-kas Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dmihalcik-virtru
force-pushed
the
DSPX-3499-pqcrun
branch
from
5019885 to
6def870
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
spec/DSPX-3499.md (2)
9-9: ⚡ Quick winLink the implementing PR in
prsmetadata.
prs: []drops traceability to this change set. Please add the current PR reference (for example,opentdf/tests#484) so the draft spec is connected to implementation history.🤖 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 `@spec/DSPX-3499.md` at line 9, The prs metadata currently is empty (`prs: []`), breaking traceability; update the `prs` entry in spec/DSPX-3499.md by replacing the empty list with the implementing PR reference (e.g., `prs: ["opentdf/tests#484"]`) so the draft spec links to the corresponding implementation PR.
36-39: ⚡ Quick winReplace placeholder acceptance criteria with explicit skip/run conditions.
The current checklist is not verifiable. Please define concrete outcomes (e.g., when PQC/hybrid support is detected tests must run; when features are absent tests must skip with expected reason) so this draft can serve as an executable contract for CI behavior.
🤖 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 `@spec/DSPX-3499.md` around lines 36 - 39, Replace the placeholder checklist under the "## Acceptance Criteria" header with explicit, testable skip/run conditions: for each bullet (currently "_Clear, testable condition_" and "_…_") state the exact detection condition (e.g., "If PQC or hybrid cryptography support is detected via feature flag X or environment variable Y, run tests A, B, C"), and the explicit skip condition and reason (e.g., "If feature X is absent, skip tests A, B, C with skip-reason 'PQC support missing'"), include expected pass/fail outcomes and the CI signal (e.g., exit code or artifact) so the criteria act as an executable contract for CI behavior; update any referenced test names or feature identifiers to the real symbols used in the codebase (e.g., the feature flag name or test suite names) so automation can map these acceptance criteria directly to CI logic.
🤖 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.
Nitpick comments:
In `@spec/DSPX-3499.md`:
- Line 9: The prs metadata currently is empty (`prs: []`), breaking
traceability; update the `prs` entry in spec/DSPX-3499.md by replacing the empty
list with the implementing PR reference (e.g., `prs: ["opentdf/tests#484"]`) so
the draft spec links to the corresponding implementation PR.
- Around line 36-39: Replace the placeholder checklist under the "## Acceptance
Criteria" header with explicit, testable skip/run conditions: for each bullet
(currently "_Clear, testable condition_" and "_…_") state the exact detection
condition (e.g., "If PQC or hybrid cryptography support is detected via feature
flag X or environment variable Y, run tests A, B, C"), and the explicit skip
condition and reason (e.g., "If feature X is absent, skip tests A, B, C with
skip-reason 'PQC support missing'"), include expected pass/fail outcomes and the
CI signal (e.g., exit code or artifact) so the criteria act as an executable
contract for CI behavior; update any referenced test names or feature
identifiers to the real symbols used in the codebase (e.g., the feature flag
name or test suite names) so automation can map these acceptance criteria
directly to CI logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b0a7653-570f-46e5-8212-4ff3a6f9c050
📒 Files selected for processing (4)
.github/workflows/vulnerability.yml.github/workflows/xtest.ymlotdf-local/src/otdf_local/services/kas.pyspec/DSPX-3499.md
🚧 Files skipped from review as they are similar to previous changes (3)
- otdf-local/src/otdf_local/services/kas.py
- .github/workflows/xtest.yml
- .github/workflows/vulnerability.yml
_algs_from_km1_log was checking for "kas initialized" and "kas config", but the platform emits "kas trust mechanisms initialized" and "kas config loaded". Neither branch matched, so the function always returned an empty set and every test_pqc.* skipped with "platform service ... doesn't yet support [mechanism-xwing|secpmlkem|mlkem]". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
2 participants
With this fix (run from this branch):
Without this fix (last night's scheduled run):
Summary by CodeRabbit
New Features
Chores
Bug Fixes / Tests