fix(issue): auto-seed TypeIssuePriority when enum is empty (HULY-7) - #42
Conversation
resolvePriority now seeds the 5 platform tracker:class:TypeIssuePriority
records (Urgent/High/Medium/Low/NoPriority) into core:space:Model when
the workspace has none, before resolving --priority. Self-heals
workspaces created with INIT_REPO_DIR=/no-init-scripts on a self-hosted
install, where the priority enum is left empty by the skipped default
init scripts.
Gated by the existing --minimal / HULY_OPINIONATED=0 master switch —
no new flag or env var. CLI-13 explicit-priority guard still throws
when opinionated defaults are OFF.
Aliases added for backward compatibility with the old --priority help
text: 'Normal' -> Medium, 'None' -> NoPriority. Help text updated to
show the platform-canonical labels.
Verified on the main server (huly.aaravlabs.com, workspace Life):
- 5 priorities seeded into core:space:Model
- --priority High resolves correctly
- --priority Normal resolves to Medium (alias)
- issue update --priority Urgent updates correctly
- --minimal still throws CLI-13 on bogus names
- lint, typecheck, 14/14 tests, build all clean
Docs updated: CHANGELOG, docs/reference/cli-behavior.md,
docs/reference/environment.md, docs/usage.md, skills/huly/SKILL.md,
skills/huly/references/{issues-and-todos,escape-hatches-and-internals}.md.
Refs HULY-7.
|
Warning Review limit reached
Next review available in: 26 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe CLI now supports canonical issue priorities and backward-compatible aliases. In opinionated mode, it seeds five missing default priorities before resolving issue creation or update priorities. Minimal mode and ChangesIssue priority seeding
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant IssueCommand
participant IssueResource
participant ModelSpace
IssueCommand->>IssueResource: create or update issue with priority
IssueResource->>ModelSpace: read TypeIssuePriority records
IssueResource->>ModelSpace: seed five defaults when enabled and empty
IssueResource-->>IssueCommand: return resolved priority
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
|
|
||
| /** | ||
| * Seed the 5 classic IssuePriority records into DOMAIN_MODEL. Idempotent — | ||
| * createDoc on an existing _id returns the existing ref, and any per-doc |
There was a problem hiding this comment.
WARNING: JSDoc claim about createDoc behavior is incorrect.
createDoc on PlatformClient does NOT perform get-or-create by _id — it generates a fresh _id and creates a new document every call. There is no platform-side dedup based on _id or on the body fields. The only thing keeping this loop idempotent in practice is the outer all.length === 0 gate at the call site (lines 327/348). If queryAll ever returns an empty list while the server actually has records (stale local model, race during a model upgrade, or a custom tracker:class:TypeIssuePriority set the local model doesn't know about), seedDefaultPriorities will silently create 5 duplicates alongside the existing records, and subsequent --priority X calls will resolve to whichever copy findPriorityHit returns first — likely the wrong rank. Recommend either (a) reading the doc string as "seeded only when the caller's gate sees an empty enum; createDoc errors are swallowed to avoid breaking self-heal" or (b) actually performing a deduped upsert (e.g. findOne per label first, then createDoc only on miss).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const normalized = normalizePriorityInput(name) | ||
| let all = await queryAll() | ||
| let hit = findPriorityHit(all, normalized) | ||
| if (!hit && all.length === 0 && !opts?.minimal && isOpinionated()) { |
There was a problem hiding this comment.
WARNING: Race condition — concurrent CLI invocations on the same empty-enum workspace will create duplicate TypeIssuePriority records.
Both calls see all.length === 0, both call seedDefaultPriorities, and each createDoc succeeds with a fresh _id (see comment on line 275 — there is no dedup). The workspace ends up with 10 (or more) priority records sharing the same labels but different _ids. Downstream findPriorityHit then returns an arbitrary one, so --priority High may resolve to the seed with rank 1|a0000: or a phantom duplicate with no rank ordering, producing inconsistent ordering in the UI. Consider one of: (a) gating the seed on a workspace-level marker (HULY_SEEDED_V1 doc in core:space:Model) that is set transactionally with the seed batch; (b) wrapping the 5-record seed in client.apply() so it's a single tx and the marker write is atomic with the seeds; (c) at minimum, deduplicating in the catch path by re-querying and pruning duplicates when a duplicate/already exists error is observed.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| * here is the self-healing path. Gated by `--minimal` / `HULY_OPINIONATED=0` | ||
| * like every other opinionated default in this CLI. | ||
| */ | ||
| async function seedDefaultPriorities(client: PlatformClient): Promise<void> { |
There was a problem hiding this comment.
SUGGESTION: No tests added for the new self-healing logic.
The PR description claims 14/14 tests pass, but the only test file in the package (packages/cli/src/raw/ws.test.ts) does not cover seedDefaultPriorities, normalizePriorityInput, findPriorityHit, or the new opts.minimal plumbing through resolvePriority. This is exactly the kind of self-healing code that breaks silently: if a future refactor changes the catch {} to a catch (e) and re-throws, or changes core:space:Model to a project space, no CI signal will fire. Recommend at minimum a unit test that mocks PlatformClient.createDoc and findAll and verifies: (1) Normal and None alias to Medium / NoPriority; (2) seed fires only when enum is empty AND opinionated AND not minimal; (3) after seed, the canonical label resolves; (4) on minimal, CLI-13 still throws.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| .option('--priority <p>', 'Urgent | High | Normal | Low | None') | ||
| .option( | ||
| '--priority <p>', | ||
| 'Urgent | High | Medium | Low | NoPriority (aliases: Normal→Medium, None→NoPriority)', |
There was a problem hiding this comment.
SUGGESTION: issue update help text is now out of sync with issue create.
The issue create long help (lines 925-932, edited in this PR) now advertises the auto-seed behavior under "Defaults & auto-creation" — good. But the issue update long help immediately below this option (lines 967-984, NOT edited) is completely silent on the fact that issue update --priority X will, in an empty-enum workspace, also seed the 5 platform priorities before resolving. A user reading huly issue update --help will not learn this. Either mirror the note here, or add a "Defaults & side effects" block for the update command listing the same auto-seed trigger.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The previously-flagged No new inline comments. Files Reviewed (1 file)
Previous Review Summaries (2 snapshots, latest commit 9875657)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 9875657)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Previously Reported (now resolved)
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit ac27aaf)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (10 files)
Reviewed by minimax-m3 · Input: 29.5K · Output: 5.5K · Cached: 196.2K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@packages/cli/src/resources/issue.ts`:
- Around line 298-301: Update resolvePriority to accept a dryRun flag and skip
seedDefaultPriorities when dryRun is true. Pass the command’s dryRun value from
both the create flow near line 758 and update flow near line 1117 before their
dry-run returns, while preserving normal priority resolution and seeding when
dryRun is false.
- Around line 341-353: Update the implicit default priority resolution around
queryAll to reuse findPriorityHit for case-insensitive matching against both
label and name, including after seedDefaultPriorities. Preserve the existing
preference for Medium/Normal, then the first available priority, and omit the
field when no priority exists.
- Around line 283-295: Update seedDefaultPriorities to be atomic and repeatable
by using deterministic priority IDs or an atomic upsert, handling only duplicate
conflicts while propagating other failures and ensuring every missing default is
retried; update its callers to seed only when opts.dryRun is false so
resolvePriority cannot persist data during dry runs.
In `@skills/huly/references/issues-and-todos.md`:
- Line 79: Update the priority behavior statement in the opinionated defaults
documentation to say that priority resolves to Medium or the first available
value when available, and is omitted when the enum is empty with defaults
disabled. Keep the existing distinction that priority is not gated by
HULY_OPINIONATED generally, while accurately reflecting resolvePriority’s
empty-enum behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 192a5060-2cbb-48cb-b4b1-93f1004b6fad
📒 Files selected for processing (10)
CHANGELOG.mddocs/reference/cli-behavior.mddocs/reference/environment.mddocs/usage.mdpackages/cli/src/auth/env.tspackages/cli/src/cli.tspackages/cli/src/resources/issue.tsskills/huly/SKILL.mdskills/huly/references/escape-hatches-and-internals.mdskills/huly/references/issues-and-todos.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Kilo Code Review
🧰 Additional context used
🪛 LanguageTool
docs/reference/cli-behavior.md
[style] ~64-~64: ‘first priority’ might be wordy. Consider a shorter alternative.
Context: ...umif it exists in the workspace; else first priority; else omitted. Aliases:--priority Nor...
(EN_WORDINESS_PREMIUM_FIRST_PRIORITY)
🪛 SkillSpector (2.5.1)
skills/huly/SKILL.md
[warning] 74: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
[warning] 277: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
[warning] 216: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 218: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 219: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 222: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 224: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 226: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 227: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 229: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[error] 58: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 134: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 135: [YR1] YARA rule 'agent_skill_destructive_autonomous_actions': Autonomous destructive filesystem, shell history, or repository actions in AI agent skills [agent_skills]: YARA rule matched a known malware signature (reverse shell, backdoor, ransomware, C2 framework, or info stealer).
Remediation: Remove the malware payload or compromised file entirely. Investigate how it entered the skill and audit all other artifacts for additional indicators of compromise.
(YARA Match (YR1))
🔇 Additional comments (8)
docs/reference/cli-behavior.md (1)
39-39: LGTM!Also applies to: 56-82
docs/reference/environment.md (1)
28-44: LGTM!CHANGELOG.md (1)
12-25: LGTM!skills/huly/SKILL.md (1)
216-230: LGTM!skills/huly/references/escape-hatches-and-internals.md (1)
225-238: LGTM!skills/huly/references/issues-and-todos.md (1)
67-78: LGTM!Also applies to: 406-406
docs/usage.md (1)
31-41: LGTM!packages/cli/src/cli.ts (1)
894-897: LGTM!Also applies to: 915-917, 959-962
Reviewers (kilo-code-bot, coderabbitai) flagged the original
auto-seed implementation in HULY-7 with three classes of concern:
silent duplicate seeding on race, --dry-run persisting data, and
missing test coverage. All addressed here.
seedDefaultPriorities — deterministic _ids:
Each seeded priority now carries a stable _id
(tracker:priority:{Urgent|High|Medium|Low|NoPriority}) passed as
the 4th arg to createDoc. The platform rejects duplicate _id, so
re-seeding is a no-op (model-upgrade tx beat us to it, concurrent
CLI invocation, etc.). JSDoc rewritten to describe the actual
dedup mechanism instead of the prior 'createDoc returns the
existing ref' claim, which was false.
resolvePriority — dry-run safe:
Accepts dryRun in opts. When set, seedDefaultPriorities is never
called, so 'huly issue create --dry-run --priority High' against
an empty enum prints the would-create tx without persisting the 5
platform priorities. --minimal keeps its existing behavior.
Both call sites (create + update) now pass opts.dryRun through.
Implicit Medium lookup — case-insensitive:
Replaced 'all.find(p => p.label === "Medium")' with
findPriorityHit(all, 'Medium') ?? findPriorityHit(all, 'Normal')
so a record whose only field is name: 'Medium' (or a custom
case-variant label) is still preferred over Urgent. Added unit
test covering the name-only case.
issue update --help — symmetric auto-seed note:
The update long-help now mirrors the create help's
'Defaults & auto-creation' block, so 'huly issue update --help'
documents the auto-seed trigger that fires on
'issue update --priority' against an empty enum.
Tests:
New packages/cli/src/resources/issue.test.ts (25 tests) covering
normalizePriorityInput, findPriorityHit, resolvePriority
(explicit + implicit, opinionated vs --minimal, --dry-run,
partial-enum case), and seedDefaultPriorities (deterministic ids,
idempotent re-seed, core:space:Model target). Total: 39/39
tests pass.
Docs:
skills/huly/references/issues-and-todos.md §'Opinionated defaults
master switch' updated to spell out the empty-enum behavior:
when --minimal / HULY_OPINIONATED=0 / --dry-run suppress the
seed and the enum is empty, the priority field is omitted from
the issue rather than auto-seeded. cli-behavior.md swapped the
LanguageTool-flagged 'else first priority' for 'else the
top-ranked priority'.
Refs PR #42.
kilo-code-bot re-review flagged that the previous test suite always mocked isOpinionated() to return true, leaving the canSeed = false branch of resolvePriority uncovered. Replace the constant mock with a vi.hoisted flag (opinionatedState.on) flipped per describe block, then add four new cases under 'resolvePriority — opinionated defaults OFF': - explicit --priority High against empty enum throws CLI-13 (no seed) - implicit resolution against empty enum returns undefined (no seed) - explicit --priority still resolves when the enum is populated - implicit resolution still picks Medium when the enum is populated Total: 43/43 tests pass; pnpm verify clean.
1 participant
Summary
Fixes HULY-7.
When the workspace has zero
tracker:class:TypeIssuePriorityrecords (e.g. self-hosted installs created withINIT_REPO_DIR=/no-init-scriptsin theworkspaceservice — see~/apps/huly-selfhost/compose.yml:322),huly issue create --priority XthrowsCLI-13 priority "X" not found in this workspace — available priorities: (none — workspace may not have tracker migration applied).resolvePrioritynow seeds the 5 platform records (Urgent / High / Medium / Low / NoPriority) intocore:space:Modelbefore resolving the priority, gated by the existing--minimal/HULY_OPINIONATED=0master switch. Self-healing; no new flag or env var.Behavior
--minimal/HULY_OPINIONATED=0)--priority High, enum emptyHigh--priority Normal(alias)MediumMediumif present, else CLI-13--priority, enum emptyMediumChanges
packages/cli/src/resources/issue.ts—resolvePriority+ newseedDefaultPrioritieshelper; aliasesNormal→Medium,None→NoPriority; both create and update paths routed through the same resolver.packages/cli/src/cli.ts—--priorityoption help and theissue createlong help updated to show platform-canonical labels and auto-seed behavior.packages/cli/src/auth/env.ts—isOpinionateddoc comment lists the new auto-seed.CHANGELOG.md,docs/reference/cli-behavior.md,docs/reference/environment.md,docs/usage.md,skills/huly/SKILL.md,skills/huly/references/issues-and-todos.md,skills/huly/references/escape-hatches-and-internals.md.Verification
Tested against the main server (
huly.aaravlabs.com, workspaceLife):TypeIssuePriorityrecords incore:space:Model.huly issue create --project HULY --title "Auto-seed verify" --priority High→ seeded 5 priorities, created HULY-13 with theHighref.Urgent 0|a0000:,High 1|a0000:,Medium 2|a0000:,Low 3|a0000:,NoPriority 4|a0000:.huly issue create --priority Normal→ resolved toMediumref.huly issue update HULY-13 --priority Urgent→ updated priority toUrgentref.huly issue create --priority Bogus --minimal→ CLI-13 with full available list.pnpm verify(format + lint + typecheck + 14/14 tests + build) all clean.