[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 1 commit into
Conversation
📝 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)
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.
1 participant
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