Skip to content

fix(agy): support --model, --add-dir workspace, and pre-prompt extra args - #4481

Open
timoyan wants to merge 4 commits into
github:mainfrom
timoyan:fix/agy-model-and-workspace-support
Open

fix(agy): support --model, --add-dir workspace, and pre-prompt extra args#4481
timoyan wants to merge 4 commits into
github:mainfrom
timoyan:fix/agy-model-and-workspace-support

Conversation

@timoyan

@timoyan timoyan commented Sep 8, 2026

Copy link
Copy Markdown

Summary

Fixes three bugs in AgyIntegration that together prevented specify workflow run from working correctly with agy. Closes #4480.

Bugs Fixed

1. --model was silently ignored

build_exec_args() had a comment # agy does not support --model ... both params are ignored and hardcoded no --model flag. agy >=1.20 fully supports agy --model <name> --print <prompt>. Workflow YAML model: pins had no effect.

Fix: Pass --model <model> before --print when model is set.

2. EXTRA_ARGS were appended after --print (silently absorbed as prompt text)

agy treats every token after --print as part of the prompt string, not as CLI flags. Operator flags like --dangerously-skip-permissions or --print-timeout 30m set via SPECKIT_INTEGRATION_AGY_EXTRA_ARGS were appended after --print and silently became part of the prompt.

Fix: _apply_extra_args_env_var() is now called before args.extend(['--print', prompt]).

3. No workspace passed to agy -- 'no active workspace'

The base dispatch_command() sets cwd=project_root for the subprocess, but agy does not read cwd as its workspace root -- it requires an explicit --add-dir <path> flag. Without it, agy falls back to its own scratch directory, cannot locate .agents/skills/, and reports 'no active workspace' -- making all installed Spec Kit skills invisible.

Fix: build_exec_args() now accepts project_root: Path | None and injects --add-dir <project_root> before --print when project_root is given. dispatch_command() is overridden in AgyIntegration to thread project_root through to build_exec_args().

Flag Order Guarantee

All flags appear before --print so agy parses them correctly:

agy [--model <m>] [--add-dir <d>] [<EXTRA_ARGS>] --print <prompt>

Tests

  • Updated ignores_model -> honors_model (asserts --model is now included)
  • Updated extra_args_honors -> extra_args_before_print (asserts extra args come BEFORE --print)
  • Added: test_build_exec_args_add_dir_for_workspace
  • Added: test_build_exec_args_no_add_dir_when_project_root_is_none
  • Added: test_build_exec_args_no_model_flag_when_model_is_none
  • Added: test_build_exec_args_combined_flag_order

All 47 tests pass.

AI Assistance Disclosure

This PR was drafted with substantial AI assistance (Google Antigravity / agy). The bugs were discovered while personally running specify workflow run against a real project and observing the failures firsthand. The root cause analysis, fix design, and test verification were reviewed and validated by me. AI was used to write the code, tests, commit message, and PR body.

@timoyan
timoyan requested a review from mnriem as a code owner September 8, 2026 21:31
@mnriem
mnriem requested a balanced review from Copilot September 9, 2026 14:30
@mnriem mnriem added author-awaiting Waiting on author response triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review labels Sep 9, 2026
@mnriem

mnriem commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks — and this is a model report→fix loop (#4480 + this), with genuinely exemplary disclosure and the bugs verified on a real project. The build_exec_args fix is right: --model/--add-dir/extra-args before --print, prompt last. One design question before merge: this overrides dispatch_command to thread project_root through as --add-dir, which duplicates the base class's subprocess/shutil.which/streaming/timeout logic in the agy integration — that's a maintenance surface that can drift from the base. Is there a lighter path (e.g. threading project_root through the base dispatch_command so all integrations can use --add-dir-style hints), rather than a full override here? I'll trigger the automated review in parallel. Also relates to the systemic build_exec_args() audit in #2416.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The production dispatch path lacks a regression test confirming that it forwards the workspace argument.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes agy workflow dispatch by correctly ordering CLI flags and supplying workspace context.

Changes:

  • Adds --model and --add-dir support.
  • Places extra arguments before --print.
  • Expands argument-building tests.
File summaries
File Description
src/specify_cli/integrations/agy/__init__.py Updates argument construction and dispatch.
tests/integrations/test_integration_agy.py Tests flag inclusion and ordering.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/specify_cli/integrations/agy/__init__.py Outdated
timoyan added a commit to timoyan/spec-kit that referenced this pull request Sep 9, 2026
As suggested in PR github#4481, this removes the dispatch_command override in AgyIntegration to avoid duplicating subprocess and timeout logic. Instead, project_root is now threaded through IntegrationBase.dispatch_command directly into build_exec_args. All other integration subclasses have been updated to accept **kwargs to maintain signature compatibility.
@timoyan

timoyan commented Sep 9, 2026

Copy link
Copy Markdown
Author

Thanks — and this is a model report→fix loop (#4480 + this), with genuinely exemplary disclosure and the bugs verified on a real project. The build_exec_args fix is right: --model/--add-dir/extra-args before --print, prompt last. One design question before merge: this overrides dispatch_command to thread project_root through as --add-dir, which duplicates the base class's subprocess/shutil.which/streaming/timeout logic in the agy integration — that's a maintenance surface that can drift from the base. Is there a lighter path (e.g. threading project_root through the base dispatch_command so all integrations can use --add-dir-style hints), rather than a full override here? I'll trigger the automated review in parallel. Also relates to the systemic build_exec_args() audit in #2416.

@mnriem I think you're right, we could have project_root from base. Please check the latest commmit. :)

@mnriem mnriem added the author-needs-rebase Branch conflicts with main — rebase/resolve before merge label Sep 9, 2026
@mnriem

mnriem commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

This is a great generalization — threading project_root through the base dispatch_command is the right call, and it likely fixes the same class for other integrations (relates to #2416, and to the bob dispatch bug in #4491/#4492). One consequence to name: this grew from an agy fix into a base-layer change touching ~15 integrations, so it needs (a) a rebase — it's currently conflicting with main — and (b) green CI across the full matrix, since every integration's dispatch is now affected. Given the blast radius I'll give it a thorough review rather than merging on the automated pass alone. Could you rebase and confirm the per-integration dispatch tests still pass? Then re-request review.

@timoyan
timoyan force-pushed the fix/agy-model-and-workspace-support branch from d32a534 to 441254e Compare September 9, 2026 18:31
@timoyan

timoyan commented Sep 9, 2026

Copy link
Copy Markdown
Author

@mnriem Rebased onto the latest main, threaded project_root across base classes and all integrations, and added a regression test for dispatching project_root as --add-dir. All integration and base test suites are green locally!

@mnriem mnriem removed the author-needs-rebase Branch conflicts with main — rebase/resolve before merge label Sep 9, 2026
@mnriem

mnriem commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks — the base-class threading is the right call and it's actually leaner than the per-integration override. Rebased and green-locally is great, but given this touches ~15 integrations I want a full CI run across the matrix (macOS/Windows have surprised us elsewhere today) plus a proper review of the base change before merge — I'll trigger CI now. Nothing needed from you meanwhile.

@mnriem
mnriem requested a balanced review from Copilot September 9, 2026 18:57
@timoyan
timoyan force-pushed the fix/agy-model-and-workspace-support branch from 441254e to 12a3fd5 Compare September 9, 2026 19:02
@mnriem mnriem removed the author-awaiting Waiting on author response label Sep 9, 2026
@timoyan

timoyan commented Sep 9, 2026

Copy link
Copy Markdown
Author

@mnriem sorry there is Ruff error in opencode integration. I just submit the fix.

@mnriem
mnriem requested a balanced review from Copilot and removed request for Copilot September 9, 2026 19:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Workflow prompt steps still omit project_root, so agy can retain the workspace failure.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 18/18 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/specify_cli/integrations/agy/__init__.py Outdated
@mnriem

mnriem commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Good — the base-class threading and rebase look right, and CI's now running. One real gap the re-review found before merge: workflow prompt steps still don't get project_root. PromptStep._try_dispatch() calls build_exec_args(prompt, model=..., output_json=False) without threading it through, so agy's "no active workspace" failure (the original #4480 bug) can still recur on prompt steps. Please thread project_root on the prompt path too, with a test covering it. Re-request once addressed (and let's confirm the full CI matrix goes green given this touches ~15 integrations).

@timoyan
timoyan force-pushed the fix/agy-model-and-workspace-support branch from 12a3fd5 to 10adae1 Compare September 9, 2026 19:19
@timoyan

timoyan commented Sep 9, 2026

Copy link
Copy Markdown
Author

@mnriem Changes pushed, please verify it in latest commit :)

…ation env vars in tests

- scripts/powershell/common.ps1: Enforce [Console]::OutputEncoding and $OutputEncoding as UTF-8. On Windows hosts with non-UTF-8/OEM code pages (such as CP950/Big5), PowerShell stdout would otherwise output non-ASCII characters (e.g. arrows in templates) using the system code page, causing UnicodeDecodeError when captured by external runners.

- tests/conftest.py: Drop ambient SPECKIT_INTEGRATION_*_EXTRA_ARGS and SPECKIT_INTEGRATION_*_EXECUTABLE environment variables in the autouse _strip_specify_env fixture to prevent host configuration leakage into integration argument-building tests.

Assisted-by: Antigravity (model: Gemini 3.8 Flash, supervised)
@timoyan
timoyan force-pushed the fix/agy-model-and-workspace-support branch from 10adae1 to e49845b Compare September 10, 2026 03:30
@timoyan

timoyan commented Sep 10, 2026

Copy link
Copy Markdown
Author

@mnriem I also found unit test edge case about encoding and execution environment variables pollution. I have submitted these fixes in commit e49845b so they are clearly isolated for review. All test suites pass cleanly.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Relative workspace roots resolve incorrectly for agy, and the unrelated PowerShell encoding change should be removed or separated.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 23/23 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread scripts/powershell/common.ps1 Outdated
Comment thread src/specify_cli/integrations/agy/__init__.py Outdated
timoyan and others added 2 commits September 10, 2026 22:47
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@timoyan

timoyan commented Sep 10, 2026

Copy link
Copy Markdown
Author

@mnriem Following Copilot’s suggestions, I submitted two changes: one for encoding in testing and another for the project_root path. Sorry for the back and forth.

@timoyan

timoyan commented Sep 10, 2026

Copy link
Copy Markdown
Author

I discussed this with my AI coding assistant (Antigravity), and it correctly identified that when project_root is a relative path, passing it directly to --add-dir
causes agy to resolve it incorrectly within the subprocess's updated cwd. We fixed this by calling .resolve() to ensure it always passes an absolute path.

I've pushed the fix and added a regression test for this specific relative-path edge case. I also ran a full end-to-end test on a local dummy project to verify the workflow 

engine dispatch works as expected.

*(Note: The code changes, tests, and this explanation were generated in collaboration with the Antigravity AI assistant).*                                                   
                                                                                                                                                                             
## Manual test results                                                                                                                                                       
                                                                                                                                                                             
**Agent**: Antigravity (agy) | **OS/Shell**: Windows / PowerShell                                                                                                            
                                                                                                                                                                             
| Command tested | Notes |                                                                                                                                                   
| --- | --- |                                                                                                                                                                
| `/speckit.specify` | Verified via `specify workflow run speckit`. The workflow engine now correctly passes the absolute path to `--add-dir`. `agy` successfully located the

.agents/skills directory without throwing the no active workspace error. |
All integration test suites and the agent config consistency checks (test_agent_config_consistency.py) are green locally! Ready for another CI run whenever you are.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-nice-to-have Verdict: evidence-backed fix or greenlit feature — land after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(agy): --model ignored, EXTRA_ARGS appended after --print, and --add-dir workspace not passed

3 participants