fix: plan-160 P1s — saffron contrast token, doc/token drift, coding-workflow harness batch - #888
cline-cloud[bot] wants to merge 16 commits into
Conversation
…#846) Chips called setInput(prompt) then handleSend(), whose closure still held the empty input — the empty-input guard silently aborted every chip click, and the covering test mocked the guard away. handleSend now accepts an explicit overrideInput used for the guard, user bubble, and request. Test rewritten against the real hook: failed pre-fix, passes post-fix.
effectiveModel (custom slug ?? dropdown) was computed after the useAiHarnessChat call and only fed display surfaces; the send pipeline received the dropdown model. Compute it before the hook and pass it as the request model so the UI and the network can never disagree. Regression test drives the full view: custom slug + api key + send, asserts sendChatStream receives the slug (failed pre-fix with the dropdown model).
…ntrast in dark mode (#845) Dark --saffron is a light orange (#e5944a) but five components painted pure white text on it (~2.4:1 vs the 4.5:1 AA floor): the drawer tabs and four count badges were illegible. New per-theme token (light #ffffff / dark #14110d, matching --sidebar-primary-foreground) applied at every bg-saffron text site, including the provider-setup icon tile.
…e shipped palette (#847) The token table documented a saffron family that no longer exists (#c77d3a/#8a4f1c/#b36a2e) while globals.css ships #9a5c2a/#6a4a1c/#8a5024 — and layout.tsx themeColor copied the stale doc value, tinting browser chrome with a ghost of the old palette. Table rows, the saffron button recipe (now text-saffron-foreground), and themeColor all match globals.css; layout comment cross-references the token so the next rebrand updates both.
AGENTS.md orders agents to load verify-before-asserting, but the skill appeared in zero catalogs — regeneration adds it (57 skills) and nothing failed the drift. generate-skills-docs.py gains --check (write-nothing diff mode, BATS-covered) and agent-surface.py validate now fails on stale catalogs, so the discovery layer cannot silently lie again.
…e name collision (#882) goap-agent named three surfaces: the skill, a Claude sub-agent, and an OpenCode sub-agent. The skill is now goap-planning (directory, SKILL.md name field, symlinks, regenerated catalogs, registry row); goap-agent remains exclusively the sub-agent name. The sub-agent evals stay in .claude/agents/goap-agent/evals (agent_type: claude); the skills own evals.json moved with the rename. agent-surface validation: no name mismatches, no broken symlinks, catalogs fresh.
…d SCRIPTS (#875, #877, #880) HARNESS.md taught a repealed ~150-line AGENTS.md ceiling (enforced constant is 250) and listed 4 of 7 manifest-managed agent surfaces (adds Cursor, Windsurf, Jules + manifest pointer). agents-docs/AGENTS.md claimed to be both auto-generated and manual; it is manual, misnamed create-agent as skill-creator, and omitted analysis-swarm/goap-agent OpenCode rows. SCRIPTS.md missed the build-critical generate-precache-manifest.mjs and generate-pwa-icons.py while documenting SKIP_CLIPPY, a Rust knob unreachable in a repo with no Cargo.toml.
…878) The verify-before-asserting rule fired on ".*" — every prompt — contradicting AGENTS.md's "load only what the stage needs". The pattern now derives from the skill's own trigger phrases, and a top-level _note marks the file advisory until an installed hook consumes it (.claude/hooks/ ships *.example only).
The repo shipped a Windows NTFS Zone.Identifier download marker, a .qwen settings backup, and mutable .jules runtime state. Untracked all four files and gitignored the patterns (.jules/, *.orig, *:Zone.Identifier) so they cannot return.
The rename commit staged the new goap-planning paths but left the old goap-agent deletions unstaged (broken by an intermediate git reset), so HEAD tracked both. Removes the old skill directory from the index; on-disk state and validation were already correct.
…her repo (#879) The config denied edits to packages/opencode/migration/* (a path that does not exist here), referenced foreign provider accounts (cline-pass/*) and a Xiaomi mimo schema, and was absent from .agents/manifest.json, so no validation covered it. Deleting per the issue's recommended option rather than adopting a surface nothing references.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Blocked merge diagnosis — blocked |
github-actions
Bot
added
documentation
GitNexus Review · PR #8882 issues found across 2 files. SummaryThis appears to combine design-system token and documentation changes with a coding-workflow harness batch. Its reach is broad, so review should focus on consistency across the design docs, agent tooling, and scripts. 🔴 CRITICAL blast radius. A design-system, documentation, and coding-workflow harness change spanning The graph reaches 11 dependents across direct and transitive callers or importers, and 54 traced flows pass through changed symbols. Impact lands in the The file risk level is LOW, with no HIGH or CRITICAL risk files identified. 🔀 Structural changes ·
|
🤖 Agent context for GitNexus Review · PR #888This comment carries deterministic graph detail for coding agents and reviewers who want the receipts — the main review comment carries the human summary.
What changedSymbol Changes (15)
Changed Files (44)
What it affectsArchitecture Impact
Blast Radius
Direct dependents (d1)
Indirect dependents (d2)
Transitive dependents (d3)
What to checkFile Risk (11)
Prompt for AI agents (2 issues) |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 12 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…subprocess Codacy flagged the subprocess.run call in validate_skills_catalog_freshness (B602/B603: non-static invocation) on PR #888. The generator's module level is side-effect free, so import it with importlib and call collect_skills/render_* directly — removes the subprocess import entirely and spares a process spawn. Behavior verified unchanged: fresh passes, stale fails with the actionable message, restored passes.
![nexus-check[bot]](https://avatars.githubusercontent.com/in/4300564?s=60&v=4){kind=small}
| await act(async () => { await hook.result.current.handleSend('Summarize the main entities.') }) | ||
| const userMessages = hook.result.current.messages.filter((m) => m.role === 'user') | ||
| expect(userMessages.at(-1)?.content).toBe('Summarize the main entities.') | ||
| expect(mockSendChatStream).toHaveBeenCalled() |
There was a problem hiding this comment.
🟡 Warning — Assert the override reaches the request builder, not only the transcript
This test checks the stored user bubble at line 172 and only that the stream mock ran here. mockBuildMessagesAsync is stubbed with a fixed message array in the existing beforeEach (line 51), so its output does not depend on the input argument; meanwhile, the real buildMessagesAsync puts its userMessage argument into the request messages (src/lib/ai/context.ts:161-181). Consequently, an implementation that records the override in the bubble but passes the empty composer input to buildMessagesAsync would still satisfy both assertions. The test therefore does not verify the advertised send-to-model behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/studio/views/use-ai-harness-chat.test.tsx, line 173:
<comment>This test checks the stored user bubble at line 172 and only that the stream mock ran here. 'mockBuildMessagesAsync' is stubbed with a fixed message array in the existing 'beforeEach' (line 51), so its output does not depend on the input argument; meanwhile, the real 'buildMessagesAsync' puts its 'userMessage' argument into the request messages ('src/lib/ai/context.ts:161-181'). Consequently, an implementation that records the override in the bubble but passes the empty composer input to 'buildMessagesAsync' would still satisfy both assertions. The test therefore does not verify the advertised</comment>
Why this matters: GitNexus flagged this from your code graph — a caller or contract relies on what changed here. · llm-review
🔄 What's new in this push (
|
| generator = _load_skills_docs_generator(repo_root) | ||
| if generator is None: | ||
| return [f"Skill docs generator not importable: {script}"] | ||
| skills = generator.collect_skills(repo_root / ".agents" / "skills") |
There was a problem hiding this comment.
🟡 Warning — Guard the skills directory before collecting catalogs
validate_canonical_skills() explicitly handles a missing canonical directory by adding an error and returning, but main() continues into this new check. At line 373, collect_skills() receives .agents/skills unconditionally; its implementation iterates skills_dir.iterdir() (scripts/generate-skills-docs.py:157), which raises FileNotFoundError when the directory is absent. Thus a missing skill root produces an uncaught traceback instead of the validator's normal collected error output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/agent-surface.py, line 373:
<comment>'validate_canonical_skills()' explicitly handles a missing canonical directory by adding an error and returning, but 'main()' continues into this new check. At line 373, 'collect_skills()' receives '.agents/skills' unconditionally; its implementation iterates 'skills_dir.iterdir()' ('scripts/generate-skills-docs.py:157'), which raises 'FileNotFoundError' when the directory is absent. Thus a missing skill root produces an uncaught traceback instead of the validator's normal collected error output.</comment>
Why this matters: GitNexus flagged this from your code graph — a caller or contract relies on what changed here. · llm-review
🔄 What's new in this push (
|
Summary
First implementation wave from the plan-160 roast (#844–#887): the four P1s plus the coding-workflow harness batch. Scope note: "harness" here means the coding-workflow harness (agents-docs, .agents, scripts) — in-app AI-harness issues beyond the two already-fixed P1 bugs remain open and out of scope.
Bug fixes (fail-first tested)
setInput+ stale-closurehandleSend()hit the empty-input guard.handleSend(overrideInput?)now carries the chip prompt through the guard, bubble, and request; test rewritten against the real hook (red pre-fix).effectiveModelis now computed beforeuseAiHarnessChatand passed as the request model. Full-view regression test (red:openrouter/free, green:openai/gpt-5).--saffron-foregroundtoken applied at all 6bg-saffrontext sites.layout.tsxthemeColornow match the shipped palette (#9a5c2a); layout comment cross-references the token.Coding-workflow harness
verify-before-assertingvisible, 57 skills);generate-skills-docs.py --check+agent-surface.py validatefreshness gate; BATS coverage.goap-agentskill renamedgoap-planning; sub-agents keep the name; symlinks + catalogs + registry updated.SKIP_CLIPPY.skill-rules.jsonno longer auto-activates on".*"; marked advisory..orig/.julesruntime state untracked + gitignored..mimocode/deleted (foreign repo's config).Verification
Closes #844, Closes #845, Closes #846, Closes #847, Closes #875, Closes #876, Closes #877, Closes #878, Closes #879, Closes #880, Closes #881, Closes #882
📝 Summary by GitNexus
Summary
This appears to combine design-system token and documentation changes with a coding-workflow harness batch. Its reach is broad, so review should focus on consistency across the design docs, agent tooling, and scripts.
🔴 CRITICAL blast radius. A design-system, documentation, and coding-workflow harness change spanning
DESIGN-SYSTEM.md, agent skill and configuration files, and scripts, with impact across the codebase.The graph reaches 11 dependents across direct and transitive callers or importers, and 54 traced flows pass through changed symbols. Impact lands in the
Views,Studio, andScriptsmodules. Start withDESIGN-SYSTEM.mdand the related agent documentation, then reviewscripts/agent-surface.pyandscripts/generate-skills-docs.pyalongside the skill and harness changes.The file risk level is LOW, with no HIGH or CRITICAL risk files identified.
Added by GitNexus for PR #888. Edit freely — this block is replaced on the next review, everything above it is left untouched.