[WRONG BRANCH] fix(kiro): preserve property names when flattening root composition - #277
[WRONG BRANCH] fix(kiro): preserve property names when flattening root composition#277luvs01 wants to merge 2 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe adapter now preserves schema property names that match rejected keywords while recursively sanitizing each property schema. Tests cover root and ChangesSchema sanitization
Estimated code review effort: 1 (Trivial) | ~5 minutes Mergeability Score: 🔵 Low · up to The sanitizer can still drop a schema property literally named proto, altering affected schemas before they reach Kiro. This is a bounded edge-case correctness risk, so the PR is mergeable with explicit owner awareness or follow-up. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
![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
🤖 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 `@src/adapters/kiro-tools.ts`:
- Line 106: Preserve a literal __proto__ schema property by using null-prototype
objects for the output of sanitizeSchemaMap and the props objects receiving its
entries, or define assignments as own properties with Object.defineProperty.
Update both affected Object.assign paths and add a regression case alongside
format and pattern confirming __proto__ reaches Kiro unchanged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c3e8299c-ffde-4991-a323-1895134e9c15
📒 Files selected for processing (2)
src/adapters/kiro-tools.tstests/kiro-adapter.test.ts
| // { type:"object", properties:{path}, required:["path"], oneOf:[...] } keeps them. | ||
| if (obj.properties && typeof obj.properties === "object") { | ||
| Object.assign(props, sanitizeKiroSchema(obj.properties) as Record<string, unknown>); | ||
| Object.assign(props, sanitizeSchemaMap(obj.properties) as Record<string, unknown>); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
bun - <<'BUN'
const source = JSON.parse('{"__proto__":{"type":"string"}}');
const out = {};
for (const [name, child] of Object.entries(source)) {
out[name] = child;
}
if (Object.hasOwn(out, "__proto__")) {
throw new Error("The reproduction did not expose the key-loss behavior");
}
console.log("Reproduced: __proto__ is not an own property.");
BUNRepository: luvs01/opencodex
Length of output: 195
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- src/adapters/kiro-tools.ts ---'
sed -n '45,135p' src/adapters/kiro-tools.ts
printf '%s\n' '--- JavaScript special-key reproduction ---'
node - <<'JS'
const source = JSON.parse('{"__proto__":{"type":"string"}}');
const out = {};
for (const [name, child] of Object.entries(source)) out[name] = child;
console.log({
sourceOwnProperty: Object.hasOwn(source, "__proto__"),
outputOwnProperty: Object.hasOwn(out, "__proto__"),
outputPrototype: Object.getPrototypeOf(out),
outputPrototypeType: Object.getPrototypeOf(out)?.type,
});
JSRepository: luvs01/opencodex
Length of output: 4853
Preserve __proto__ as a schema property name.
At src/adapters/kiro-tools.ts:106 and :121, Object.assign writes sanitized entries into normal props. sanitizeSchemaMap also writes entries into normal out at line 64. If the input contains properties.__proto__, the assignment invokes the prototype setter. The schema property is lost before Kiro receives the schema.
Use Object.create(null) for both out and props, or define entries with Object.defineProperty. Add a regression case for __proto__ beside format and pattern.
🤖 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/adapters/kiro-tools.ts` at line 106, Preserve a literal __proto__ schema
property by using null-prototype objects for the output of sanitizeSchemaMap and
the props objects receiving its entries, or define assignments as own properties
with Object.defineProperty. Update both affected Object.assign paths and add a
regression case alongside format and pattern confirming __proto__ reaches Kiro
unchanged.
@Wibias stepped down from developing opencodex, and repository permission was reduced to read access. Move him out of the current-maintainers table into a new Former maintainers section, drop him from the CODEOWNERS default-reviewer line and the four high-impact runtime paths, and record the change with the 2026-07-27 addition entry it closes. Nothing he authored is unwound: commits, merged pull requests, release-note attributions, and the code comments citing his reviews stay as they are.
f60a04d to
75d2b6e
Compare
2 participants
Motivation
formatandpatternto be dropped by the Kiro sanitizer.Description
sanitizeSchemaMap(...)(schema-map-aware recursion) instead ofsanitizeKiroSchema(...)when mergingpropertiesfrom the root and from root composition variants insideensureRootObjectType, so property names are preserved while their child schemas are still sanitized.tests/kiro-adapter.test.tsthat verifies property names that collide with rejected validation keywords survive rootoneOfflattening and that nested validation-only keywords are still stripped.Testing
bun test tests/kiro-adapter.test.tsand the updatedkirotests passed (54 tests across that file).bun x tsc --noEmitand it succeeded.bun run test), which exercised the broader suite but encountered unrelated environment-sensitive timeouts and integration failures; these are not caused by this small sanitizer change and do not affect the focused regression coverage added here.Codex Task
Summary by CodeRabbit
Bug Fixes
formatorpattern, ensuring they remain available while conflicting validation metadata is handled correctly.Tests