Skip to content

Conversation

psteinroe
Copy link
Collaborator

@psteinroe psteinroe commented Sep 10, 2025

will have claude do the work over night

closes #131

will port https://github.com/kaaveland/eugene/blob/main/eugene/src/lints/rules.rs afterwards

also added a new agentic/ dir and two new just commands to standardise as well as track the prompts for such work.

UPDATE:

  • refactored some of the rules to make them easier to consume.
  • added a bunch of tests
  • refactored the test framework to allow multiple statements per file (required to test some rules)
  • refactored the test framework to require explicit declaring each expected diagnostic
  • migrated prefer jsonb from eugene too. the remaining ones are either covered be squawk or require a bit more updates to the file context. will do them in a follow-up. see full comparison here: Check eugene lint for migration lint rules? #305 (reply in thread)

@psteinroe psteinroe marked this pull request as draft September 10, 2025 06:17
@psteinroe psteinroe marked this pull request as ready for review September 12, 2025 08:53
justfile Outdated
@@ -152,3 +152,22 @@ quick-modify:
# just show-logs | bunyan
show-logs:
tail -f $(ls $PGT_LOG_PATH/server.log.* | sort -t- -k2,2 -k3,3 -k4,4 | tail -n 1)

agentic name:
unset ANTHROPIC_API_KEY && claude --dangerously-skip-permissions -p "please read agentic/{{name}}.md and follow the instructions closely"
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure we should commit this into an open source repo – seems like a vulnerability for prompt injection :)
We could pass a --settings agentic/{{name}}.settings.json file ?

Also, I'm curious: Why do you unset the API key here? To force somebody to login?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

how would that be an attack vector?

I am removing the env because otherwise it doesn't pick up the Max subscription but the API key (which is usage based)

Copy link
Collaborator

@juleswritescode juleswritescode Sep 18, 2025

Choose a reason for hiding this comment

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

I am removing the env because otherwise it doesn't pick up the Max subscription but the API key (which is usage based)

ah i see! that makes sense.

how would that be an attack vector?

for example if a malicious contributor adds something into a build script that replaces the contents of your agentic/ file. If you then run the agentic file, claude might do stuff you don't expect – with GH permissions, claude might even push unreviewd commits to master etc

If we review everything closely, that won't be an issue, but I could see that someone contributes a C file or something that we just gloss over

Of course, this would still happen if you run --dangerously-skip-permissions via your own terminal, but then at least nobody would know that we use that param 😇

Your call!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, I just commented them out for now with a node 'use at your own risk'

@psteinroe psteinroe merged commit 41d5a8d into main Sep 18, 2025
8 checks passed
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.

Add Analyser Rules from Squawk
2 participants