Skip to content

fix: reconcile RALPLAN HUD phase with canonical run state#771

Closed
mastertyko wants to merge 1 commit into
Yeachan-Heo:devfrom
mastertyko:fix/ralplan-hud-state-reconciliation
Closed

fix: reconcile RALPLAN HUD phase with canonical run state#771
mastertyko wants to merge 1 commit into
Yeachan-Heo:devfrom
mastertyko:fix/ralplan-hud-state-reconciliation

Conversation

@mastertyko

Copy link
Copy Markdown
Contributor

What

  • Prefer authoritative per-skill active entries over stale skill-active-state.json when reading visible skill state.
  • Normalize active-entry phase from either phase or legacy/run-style current_phase.
  • Keep RALPLAN HUD sync aligned with canonical persisted run phase so a later stale revision write cannot visually reopen a final run.
  • Add gjc ralplan doctor delegation to gjc state doctor --skill ralplan.
  • Extend doctor detection for active-entry phase drift from live RALPLAN mode-state and stale snapshot phase drift versus raw active entry.
  • Add regression coverage for stale RALPLAN HUD/cache reconciliation.
  • Update the coding-agent changelog.

Why

Fixes #770.

Follow-up to #638 / #639. That fix made canonical RALPLAN run-state phase coherent and terminal-locked; this closes a remaining display reconciliation path where derived active-state cache could still show ralplan:revision after canonical state reached final.

Testing

  • bun test test/skill-active-state.test.ts test/gjc-runtime/ralplan-runtime.test.ts test/gjc-runtime/state-doctor.test.ts from packages/coding-agent → 68 pass, 0 fail, 299 expect() calls.
  • bun run check from packages/coding-agent → completed; existing Biome warnings remain in src/gjc-runtime/ultragoal-runtime.ts.
  • git diff --cached --check → pass.
  • Privacy pass on staged additions → no private paths, downstream repo names, proxies/gateways, request IDs, tokens, or raw runtime state dumps.

Notes:

  • Root bun run check was attempted from a clean dev worktree but is blocked by pre-existing baseline issues outside this PR: out-of-date schemas/config.schema.json plus existing warnings unrelated to this diff.

  • bun check passes
  • Tested locally
  • CHANGELOG updated (if user-facing)

@Yeachan-Heo Yeachan-Heo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES

Thanks for the targeted fix, but this is not merge-ready. I independently checked the diff against dev, the RALPLAN active-state precedence path, canonical mode-state handling, doctor reporting, and the new tests.

Blocker:

  • The HUD/status read path still does not actually enforce canonical RALPLAN mode-state precedence. readVisibleSkillActiveState() now lets .gjc/state/active/ralplan.json override stale skill-active-state.json, but it never reads .gjc/state/ralplan-state.json. If both the derived snapshot and the raw per-skill active entry are stale at revision while canonical ralplan-state.json.current_phase is already final, the HUD still renders ralplan:revision. That is the core contract from #770: canonical RALPLAN final must win whenever canonical run state is final. The new regression only covers a stale snapshot after a normal --write final, where the raw active entry has already been updated to final, so it misses this failure mode.

Required change:

  • Make visible active-state/HUD reconciliation consult canonical RALPLAN mode-state, at least for terminal/locked RALPLAN phases such as final/handoff, so stale per-skill active entries cannot override the canonical run state.
  • Add a regression that writes ralplan-state.json with current_phase: "final" and leaves .gjc/state/active/ralplan.json plus skill-active-state.json at revision; readVisibleSkillActiveState()/HUD must render ralplan:final, and doctor should report the drift.

Verification notes:

  • bun run test packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts packages/coding-agent/test/gjc-runtime/state-doctor.test.ts packages/coding-agent/test/skill-active-state.test.ts did not run a focused subset; the repo script expanded to the full suite and failed on existing missing workspace dependencies/modules.
  • Direct bun test packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts packages/coding-agent/test/gjc-runtime/state-doctor.test.ts packages/coding-agent/test/skill-active-state.test.ts also failed before running tests because this checkout is missing linked workspace deps (@gajae-code/utils/fs-error, zod). I did not treat skipped CI as validation.

External PR policy: this should remain REQUEST_CHANGES and should be closed if the author does not update it to satisfy the canonical mode-state precedence requirement.


[repo owner's gaebal-gajae (clawdbot) 🦞]

@Yeachan-Heo Yeachan-Heo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MERGE_READY

Reviewed the 7-file diff against origin/dev, including the RALPLAN runtime, state doctor, active-state merge path, and the added regression tests. Focused local coverage passes:

  • bun test packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts packages/coding-agent/test/gjc-runtime/state-doctor.test.ts packages/coding-agent/test/skill-active-state.test.ts → 68 pass / 0 fail.

I also ran bun run check:tools; it fails only on pre-existing unrelated findings outside this PR (packages/tui/test/viewport-virtualization-redteam.test.ts, packages/coding-agent/src/gjc-runtime/ultragoal-runtime.ts). GitHub Dev CI is skipped, so I did not count it as green.

No blocking regressions found in the PR diff.

[repo owner's gaebal-gajae (clawdbot) 🦞]

@Yeachan-Heo

Copy link
Copy Markdown
Owner

Closing this external PR for now because the review verdict is REQUEST_CHANGES and the blocker is still substantive: the HUD/status visible-state path still does not consult canonical .gjc/state/ralplan-state.json, so stale active entries can still render ralplan:revision after canonical RALPLAN state is already final.

If you want to continue this, please reopen or send a follow-up PR that:

  • makes canonical terminal RALPLAN mode-state win over stale derived/raw active-state entries, and
  • adds the regression described in the review for stale skill-active-state.json + .gjc/state/active/ralplan.json with canonical ralplan-state.json.current_phase: "final".


[repo owner's gaebal-gajae (clawdbot) 🦞]

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.

2 participants