Skip to content

[AI-3386] Write the Kiro Crew test fixtures culture-invariantly - #1256

Merged
realtonyyoung merged 2 commits into
mainfrom
tonyyoung/ai-3386-kiro-crew-invariant-fixtures
Sep 30, 2026
Merged

realtonyyoung merged 2 commits into
mainfrom
tonyyoung/ai-3386-kiro-crew-invariant-fixtures

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

Closes #1250 — AI-3386

What & why

KiroCrewParentResolverTests interpolated a double straight into the JSON it seeds, so under a comma-decimal locale (nb_NO, for one) it wrote 1790266768,153635, which is invalid JSON. The reader then saw no sub-agent record and six tests failed. That fixture now formats the number with CultureInfo.InvariantCulture. KiroCrewSessionEndWatchTests had the same exposure in a custom date format, whose : is the culture's time separator, and now formats invariantly too. Production parsing was already culture-invariant.

Verification

Under LC_ALL=nb_NO.UTF-8, six of the fourteen resolver tests failed before the change. After it, every Kiro Crew test in the core unit suite passes under both nb_NO and en_US.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 30, 2026

Copy link
Copy Markdown

AI-3386

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Make Kiro Crew test fixtures culture-invariant

🐞 Bug fix 🧪 Tests 🕐 Less than 10 minutes

Grey Divider

AI Description

• Format sub-agent start times as valid JSON numbers regardless of the machine's locale.
• Format session timestamps consistently so watcher tests work under comma-decimal locales.
Diagram

graph TD
  ParentTests["Parent resolver tests"] --> Json["Fixture JSON"] --> Reader["Kiro Crew records"] --> Resolver["Parent resolver"]
  WatchTests["Session watch tests"] --> Json
  Reader --> Watch["Session end watch"]
Loading
High-Level Assessment

Keep the targeted fixture-formatting changes: the defect is in test data generation, while production parsing is already culture-invariant. Introducing a JSON serializer or changing production parsing would add scope without addressing a broader need in this PR.

Files changed (2) +5 / -3

Bug fix (2) +5 / -3
KiroCrewParentResolverTests.csWrite sub-agent start times with invariant decimal formatting +2/-1

Write sub-agent start times with invariant decimal formatting

• Formats the epoch-seconds value in seeded sub-agent JSON with CultureInfo.InvariantCulture. This preserves a valid JSON number when tests run under a comma-decimal locale.

test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewParentResolverTests.cs

KiroCrewSessionEndWatchTests.csWrite session metadata timestamps with invariant formatting +3/-2

Write session metadata timestamps with invariant formatting

• Uses invariant interpolation for session creation timestamps in two watcher tests. This prevents locale-specific time separators from changing their seeded metadata.

test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewSessionEndWatchTests.cs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Informational

1. The PR title breaks the commit naming rule 📘 Rule violation ⚙ Maintainability
Description
The PR title and first commit subject are `[AI-3386] Write the Kiro Crew test fixtures
culture-invariantly`, using a bracketed Linear identifier and omitting the required trailing GitHub
reference in (#number) form. Because merge-time commit validation checks this subject before
implementation review, the change is rejected even though the description contains `Closes #1250 —
AI-3386`.
Code

test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewParentResolverTests.cs[1]

+using System.Globalization;
Relevance

● Weak

PR #659 accepted guidance allowing [AI-123] title prefixes while requiring references in the
description, contradicting this title-format finding.

PR-#659

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The supplied PR title and first commit subject are identical and begin with [AI-3386], confirming
that the Linear-style issue token is present in both user-facing and commit metadata. The title does
not end with the required (#digits) reference, while Closes #1250 — AI-3386 appears only in the
description; therefore the title and commit-subject requirements remain independently non-compliant.

Rule 2897961: Enforce single-clause imperative commit subject with GitHub issue reference and 80-char limit
Rule 2897991: PR description must contain both GitHub and Linear issue references; PR title must not contain issue IDs
Rule 2897978: Format PR titles as imperative commit subjects without explicit issue references

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR title and primary commit subject use `[AI-3386]` and do not satisfy the required concise imperative format ending with a GitHub issue reference such as `(#1250)`. Remove the bracketed Linear identifier from the title and commit subject while retaining the description reference line `Closes #1250 — AI-3386`.

## Fix Focus Areas
- test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewParentResolverTests.cs[1-1]
- test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewSessionEndWatchTests.cs[1-1]
- test/Capacitor.Cli.Core.Tests.Unit.Harness/Kiro/KiroCrewParentResolverTests.cs[1-1]

## Recommended Fix
Rename the PR title and merge commit subject to one imperative clause of no more than 80 characters, remove `[AI-3386]`, and end the subject with the explicit GitHub issue reference `(#1250)`. Keep `Closes #1250 — AI-3386` together on the description's reference line.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
Review mode: 🚀 Fast: This is a small, localized test-fixture culture-invariance change with no production, security, API, or data-path impact.

Grey Divider

Tip of the day
💡 Did you know, you can route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

Declining the title-format finding: this repo's merged history uses the [AI-N] Imperative subject form (e.g. #1232, #1216–#1219), GitHub appends (#N) at squash time, and the issue reference is in the description (Closes #1250 — AI-3386). Qodo's own relevance note cites PR #659 accepting this convention.

@realtonyyoung
realtonyyoung merged commit a060a26 into main Sep 30, 2026
8 checks passed
@realtonyyoung
realtonyyoung deleted the tonyyoung/ai-3386-kiro-crew-invariant-fixtures branch September 30, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KiroCrewParentResolverTests fail under a comma-decimal locale

1 participant