Skip to content

Conversation

@cgwalters
Copy link
Contributor

  • Avoids redundant code
  • Doesn't ignore parameters on type mismatches

Motivated specifically by me having a typo in my optional parameter name when developing.

Note: I only changed a random selection of goose-mcp, but if we choose to do this it'd likely be trivial to ask goose to finish it. I didn't do so yet to not make this a conflict-fest.

@jamadeo
Copy link
Collaborator

jamadeo commented Jul 3, 2025

Thank you @cgwalters, sorry this took a bit to get to. If you merge/rebase the build should pass and we can merge.

@cgwalters
Copy link
Contributor Author

Thank you @cgwalters, sorry this took a bit to get to. If you merge/rebase the build should pass and we can merge.

I understand, it's a very active repository! I am interested in continuing to contribute to Goose, so it's helpful to have small PRs like this merge.

@DOsinga DOsinga self-assigned this Jul 16, 2025
@DOsinga
Copy link
Collaborator

DOsinga commented Jul 29, 2025

we're still interested in getting this in - can you see if you can resolve the conflicts and fix the CI?

@cgwalters
Copy link
Contributor Author

we're still interested in getting this in - can you see if you can resolve the conflicts and fix the CI?

Sure, done!

There's lots of other possible places to port to using the new helpers, I might do so after this lands, but I think to avoid a continual conflict fest we should get this smaller scope one in first.

@DOsinga
Copy link
Collaborator

DOsinga commented Aug 1, 2025

not sure why this is is happening, but can you merge in main & then regenerate the api? must be some versioning

@cgwalters
Copy link
Contributor Author

I think it's the same thing as #3750 (comment)

My guess here without digging is a classic semantic merge conflict where two PRs merged that passed independently but broke together. (The general solution to this to have an evergreen-main is a merge queue)

Anyways yes rebased 🏄

@DOsinga
Copy link
Collaborator

DOsinga commented Aug 2, 2025

ok, I think my manual merge screwed this up. can you have another look?

- Avoids redundant code
- Doesn't ignore parameters on type mismatches

Motivated specifically by me having a typo in my optional parameter name
when developing.

Note: I only changed a random selection of goose-mcp, but if
we choose to do this it'd likely be trivial to ask goose to
finish it. I didn't do so yet to not make this a conflict-fest.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters
Copy link
Contributor Author

Thanks for looking at this again. So this is your project and not mine but personally I try to avoid merges (i.e. keeping history linear) unless it's truly necessary, and this is a pretty tiny PR.

I've rebased again and the tests pass locally for me. If the GHA CI isn't passing I'm pretty sure there must be some environmental issue. (Though I could be wrong)

@DOsinga
Copy link
Collaborator

DOsinga commented Aug 3, 2025

finally ...

@DOsinga DOsinga merged commit 412ceb8 into block:main Aug 3, 2025
12 of 13 checks passed
@cgwalters
Copy link
Contributor Author

I've rebased again and the tests pass locally for me. If the GHA CI isn't passing I'm pretty sure there must be some environmental issue. (Though I could be wrong)

OK we probably shouldn't have merged over red CI, my bad for suggesting it. I was lazy in not even trying to look at the CI failure, but I put up a PR in #3816

katzdave added a commit that referenced this pull request Aug 4, 2025
…ng-quickfix

* 'main' of github.com:block/goose: (26 commits)
  docs: Add YouTube link to Git MCP Tutorial (#3831)
  feat: more robust client initialization for the app (#3830)
  Build app bundles on release branches always (#3789)
  fix param order of debug_conversation_fixer (#3796)
  Fix directory switcher not working in active chat sessions and file browser not defaulting to current session directory path (#3791)
  File completion in CLI (#3822)
  docs: Dynamic linux install buttons (#3810)
  tests: Add missing `#[serial]` to two tests (#3816)
  Chore: apply more clippy rules to prevent from code complexity (#3813)
  chore(mcp): Add helpers to parse parameters (#2821)
  feat: enable docusaurus respectPrefersColorScheme (#3746)
  fix session resume in new window (#3800)
  Add settings field documentation to recipe guides (#3809)
  chore(deps): bump on-headers and compression in /documentation (#3532)
  fix(ui): refresh provider related issues (#3385)
  feat: Add comprehensive Linux build support (#3673)
  developer: Optimize text_editor_view a bit (#3781)
  Override session name generator for ollama provider (#3710)
  docs: fix markdown for cognee tutorial (#3801)
  chore: Upgrade node (#3756)
  ...
michaelneale added a commit that referenced this pull request Aug 4, 2025
* main: (34 commits)
  Token counting in Auto-compact uses provider metadata (#3788)
  docs: Add YouTube link to Git MCP Tutorial (#3831)
  feat: more robust client initialization for the app (#3830)
  Build app bundles on release branches always (#3789)
  fix param order of debug_conversation_fixer (#3796)
  Fix directory switcher not working in active chat sessions and file browser not defaulting to current session directory path (#3791)
  File completion in CLI (#3822)
  docs: Dynamic linux install buttons (#3810)
  tests: Add missing `#[serial]` to two tests (#3816)
  Chore: apply more clippy rules to prevent from code complexity (#3813)
  chore(mcp): Add helpers to parse parameters (#2821)
  feat: enable docusaurus respectPrefersColorScheme (#3746)
  fix session resume in new window (#3800)
  Add settings field documentation to recipe guides (#3809)
  chore(deps): bump on-headers and compression in /documentation (#3532)
  fix(ui): refresh provider related issues (#3385)
  feat: Add comprehensive Linux build support (#3673)
  developer: Optimize text_editor_view a bit (#3781)
  Override session name generator for ollama provider (#3710)
  docs: fix markdown for cognee tutorial (#3801)
  ...
michaelneale added a commit that referenced this pull request Aug 5, 2025
* main: (56 commits)
  Token counting in Auto-compact uses provider metadata (#3788)
  docs: Add YouTube link to Git MCP Tutorial (#3831)
  feat: more robust client initialization for the app (#3830)
  Build app bundles on release branches always (#3789)
  fix param order of debug_conversation_fixer (#3796)
  Fix directory switcher not working in active chat sessions and file browser not defaulting to current session directory path (#3791)
  File completion in CLI (#3822)
  docs: Dynamic linux install buttons (#3810)
  tests: Add missing `#[serial]` to two tests (#3816)
  Chore: apply more clippy rules to prevent from code complexity (#3813)
  chore(mcp): Add helpers to parse parameters (#2821)
  feat: enable docusaurus respectPrefersColorScheme (#3746)
  fix session resume in new window (#3800)
  Add settings field documentation to recipe guides (#3809)
  chore(deps): bump on-headers and compression in /documentation (#3532)
  fix(ui): refresh provider related issues (#3385)
  feat: Add comprehensive Linux build support (#3673)
  developer: Optimize text_editor_view a bit (#3781)
  Override session name generator for ollama provider (#3710)
  docs: fix markdown for cognee tutorial (#3801)
  ...
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.

3 participants