Skip to content

Conversation

@cgwalters
Copy link
Contributor

I'm assuming this was accidentally committed. Remove it and add a .gitignore to prevent it from coming back.

I noticed this repository when cloning is now over 1G. This is surprising for a repository of this age and scope. For reference the Linux kernel repository's .git is now 3.4G.

There are a few causes to this (the large high resolution photos in documentation/ are one) but in looking around, this binary executable is one that is almost certainly a mistake. If it's not a mistake it needs a good bit of justification I think.

I'm assuming this was accidentally committed. Remove it
and add a .gitignore to prevent it from coming back.

I noticed this repository when cloning is now over 1G. This
is surprising for a repository of this age and scope. For
reference the Linux kernel repository's `.git` is now 3.4G.

There are a few causes to this (the large high resolution
photos in `documentation/` are one) but in looking around,
this binary executable is one that is almost certainly a
mistake. If it's not a mistake it needs a good bit of
justification I think.

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

Let me use this PR to ask a related question: Is no one actually using AI (via goose even) to review PRs to this repo?

Using the prompt:

review commit 328a456 as if it's a new PR

to try to simulate this, and using claude:

> review commit 328a4560a02757c33caa3f688f62c801b5062cb9 as if it's a new PR
...
  🔍 Minor Concerns

  1. Binary Committed: The temporal-service binary is committed, which is unusual. Consider using release artifacts or build processes instead.
  2. Windows Path Handling: Mixed forward/backward slashes in Windows paths:
  exe_dir.join("resources/bin/temporal-service.exe"),
  exe_dir.join("resources\\bin\\temporal-service.exe"), // Unnecessary - join() handles this
  3. Code Duplication: Some path checking logic could be extracted into a helper function.

I think stuff like this is part of its system prompt? Using goose+gemini-2.5-pro didn't flag it though.

@Kvadratni
Copy link
Contributor

@cgwalters yeah we might not need a compiled version in. the build should build a new one every time.
however it is measly 27 mb so I doubt it is the core of the problem

@cgwalters
Copy link
Contributor Author

yeah we might not need a compiled version in.

I don't think there's a "might" here, I cannot think of any good rationale to have a pregenerated binary committed to the git repo. As claude said, this is what release artifacts are for - but those who want to build from source shouldn't be concerned with those.

however it is measly 27 mb so I doubt it is the core of the problem

Not sure I'd say 27MB is measly to start, I mean the entire history of an active ~12 year old project I have is 57MB. But leaving that aside: it's ~27MB multipled by every commit that changed that code and then blindly committing new versions of the binary:

> git log --oneline temporal-service/temporal-service
328a4560a0 update the path for temporal (#3131)
c3acddc360 reapply temporal fix (#3080)
bdb10e7d97 Mnovich/fix cli permisisons (#3074)
041ebd5aa8 fix(temporal-service): makes the search for possible temporal paths parallel. (#3062)
e6a893254d fix temporal build for windows (#3045)
180b1df25d Mnovich/temporal foreground tasks (#2895)
d9ca16d9e6 Fix window not showing for some users (#2967)
dd5f5eb03d dynamic port selection for temporal (#2865)
cb6a819de4 add temporal service to builds. (#2842)
996f727ec5 Revert "Mnovich/temporal scheduler (#2745)" (#2839)
e6028d2fad Mnovich/temporal scheduler (#2745)

i.e. ~roughly currently 297MB of the .git repo is this binary.

@michaelneale
Copy link
Collaborator

@cgwalters yeah sometimes I do actually use it - mainly for things I don't understand (but if I ask too general stuff, I find often the advice is too general) but use it a lot with build/optimisations, but not so much with PRs (but it is something we will have to do - but it is tricky to not get it in a self contratulatory/approving loop! or you can bias it too negatively and rejects too much!).

yeah I think it is important to not have a binary - this repo is already uncomfortably big due to history of things, so agree, it is nice to housekeep like this.

@michaelneale michaelneale merged commit 67664a2 into block:main Jul 10, 2025
7 checks passed
ahau-square pushed a commit that referenced this pull request Jul 11, 2025
zanesq added a commit that referenced this pull request Jul 11, 2025
* 'main' of github.com:block/goose:
  [cli] Add --provider and --model CLI options to run command (#3295)
  Docs: Lead/worker model in Goose Desktop (#3342)
  revert: refactor: abstract keyring logic to better enable DI (#3358)
  Drop temporal-service binary (#3340)
  docs: add fuzzy search (#3357)
  Fix name of GPT-4.1 System Prompt (#3348) (#3351)
michaelneale added a commit to jackjackbits/goose that referenced this pull request Jul 14, 2025
* main: (51 commits)
  docs: reflecting benefits of CLI providers (block#3399)
  feat: fetch openrouter supported models in `goose configure` (block#3347)
  Add the ability to configure rustyline to use a different edit mode (e.g. vi) (block#2769)
  docs: update CLI provider guide (block#3397)
  Streamable HTTP CLI flag (block#3394)
  docs: Show both remote options for extensions in CLI (block#3392)
  docs: fix YouTube Transcript MCP package manager (block#3390)
  docs: simplify alby mcp (block#3379)
  docs: add max turns (block#3372)
  feat(cli): add cost estimation per provider for Goose CLI (block#3330)
  feat: Allow Ollama for non-tool models for chat only (block#3308)
  [cli] Add --provider and --model CLI options to run command (block#3295)
  Docs: Lead/worker model in Goose Desktop (block#3342)
  revert: refactor: abstract keyring logic to better enable DI (block#3358)
  Drop temporal-service binary (block#3340)
  docs: add fuzzy search (block#3357)
  Fix name of GPT-4.1 System Prompt (block#3348) (block#3351)
  docs: add goose-mobile (block#3315)
  refactor: abstract keyring logic to better enable DI (block#3262)
  fix: correct tool use for anthropic (block#3311)
  ...
atarantino pushed a commit to atarantino/goose that referenced this pull request Jul 14, 2025
Signed-off-by: Colin Walters <[email protected]>
Signed-off-by: Adam Tarantino <[email protected]>
s-soroosh pushed a commit to s-soroosh/goose that referenced this pull request Jul 18, 2025
Signed-off-by: Colin Walters <[email protected]>
Signed-off-by: Soroosh <[email protected]>
kwsantiago pushed a commit to kwsantiago/goose that referenced this pull request Jul 19, 2025
Signed-off-by: Colin Walters <[email protected]>
Signed-off-by: Kyle Santiago <[email protected]>
cbruyndoncx pushed a commit to cbruyndoncx/goose that referenced this pull request Jul 20, 2025
@cgwalters cgwalters mentioned this pull request Dec 9, 2025
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