feat(executors): add Aider adapter - #55
Conversation
|
Thanks for this, @adity982 — this is a solid implementation of #53. I reviewed it end to end:
Two small housekeeping items before merge:
This project is now © 2026 Binay Dalai — All Rights Reserved (the MIT license was removed). By opening this PR you agree to the Contribution Agreement: you assign and transfer all copyright and ownership of your contributed code to the repository owner. Concretely, that means:
Contributions are genuinely appreciated and credited — but they are made under these proprietary terms, not as open source. If you're good with that, please rebase and I'll merge. If not, no worries and no hard feelings — just let me know. |
|
recheck |
|
Thanks for your contribution! Before it can be merged, please read the Contributor License Agreement — Umbra is All Rights Reserved, and by signing you assign copyright/ownership of your contribution to the Owner (you may not use, sell, or commercialize it yourself). To agree, reply with exactly: I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
|
Thanks for this — the Aider adapter is well done and follows the existing executor security model closely (opt-in + fail-closed via Before this can be merged, a few things are needed:
One optional consistency nit (not a blocker): unlike Once the CLA is signed, the branch is rebased, and CI is green, this is good to go. |
Closes #53. Ports @adity982's implementation from PR #55. That PR could not be rebased: it predates the umbra_core -> signetry_core rename (#90), so every file it touched had moved. The adapter design is theirs — the flag set, deriving the result from the repository diff rather than from Aider's own claim, and redacting the prompt from the replay command. This port updates the package path, the env-var prefix (UMBRA_ -> SIGNETRY_), and adds the safety tests. Registered as `aider`. Fail-closed on two independent conditions: SIGNETRY_ENABLE_AIDER=true AND the CLI actually responding to --version. Commit authority stays with the pipeline: --no-auto-commits and --no-dirty-commits mean Aider edits the working tree but never creates a commit, so the change stays governable by admission. --no-suggest-shell-commands keeps it from proposing shell execution, and --dry-run is used for read-only (reason) runs. model_identity reports model_resolved as "unavailable" rather than back-filling the requested model, because Aider does not echo the provider model it resolved — claiming otherwise would put an unverified value on a receipt. 13 tests: fail-closed gating (flag missing, CLI missing), protocol + registry membership, diff capture, the safety-flag contract (asserting no commit is created in a real temp repo), --dry-run on read_only, prompt redaction, honest "unavailable" on a failed run, the disabled result, model pass-through, and that registration does not let aider be auto-selected ahead of a configured preference. Also updates the pre-existing registry test, which pinned the exact executor set, and tidies a line wrap left in README by the Kotlin change. Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
@adity982 never signed the CLA. Verified against the authoritative store — signatures/cla.json on the cla-signatures branch lists exactly one signed contributor (AdvaitVarhade, from #73). The two "I have read the CLA" strings on #55 are the bot's own instruction text and a quote of it in a maintainer comment, not a signature. CONTRIBUTORS.md states that the people listed are acknowledged "for contributions accepted under the Contributor License Agreement", so an unsigned contribution does not belong in that list. NOTE — this leaves an open provenance question, not resolved here. The Aider adapter merged in #96 is a port of their #55, and aider.py still records that derivation in its module docstring. Without a signed CLA the copyright assignment never happened, so the repo now contains code derived from an unassigned contribution. That needs a decision (reimplement from the sibling adapters, or obtain a signature) — flagged rather than settled by this commit.
@adity982 never signed the CLA. Verified against the authoritative store — signatures/cla.json on the cla-signatures branch lists exactly one signed contributor (AdvaitVarhade, from #73). The two "I have read the CLA" strings on #55 are the bot's own instruction text and a quote of it in a maintainer comment, not a signature. CONTRIBUTORS.md states that the people listed are acknowledged "for contributions accepted under the Contributor License Agreement", so an unsigned contribution does not belong in that list. NOTE — this leaves an open provenance question, not resolved here. The Aider adapter merged in #96 is a port of their #55, and aider.py still records that derivation in its module docstring. Without a signed CLA the copyright assignment never happened, so the repo now contains code derived from an unassigned contribution. That needs a decision (reimplement from the sibling adapters, or obtain a signature) — flagged rather than settled by this commit. Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
Resolves the provenance question flagged in #99. The version merged in #96 was a port of an unsigned contribution (#55). Without a signed CLA the copyright assignment never happened, so keeping derived code — especially after removing the attribution — was the least defensible combination. This rewrites the adapter from the sibling adapters (codex.py / claude_code.py, both owner-authored) so nothing in the tree derives from an unassigned contribution, and drops the attribution from the module docstring and the v0.7.0 changelog entry. Behaviour is unchanged: all 33 executor tests written against the previous implementation pass without modification, because they assert behaviour rather than structure. Two things the rewrite adds, both from house patterns the port lacked: * --model is validated before use, like the Codex adapter's -m value, so a caller-supplied string cannot smuggle shell metacharacters or extra arguments into the command. The first character must be alphanumeric — my own test caught that a naive character class accepts "--dangerously-x", since "-" is legal inside a model name, and an argument parser may read a leading dash as a new option rather than as --model's value. * The withheld-authority flags are a named constant, so removing one shows up in review instead of disappearing into the command construction. A test asserts the set, not just individual flags. Also logs a non-zero exit like claude_code.py does, which the port did not. Co-authored-by: Binay <bkd-dotcom@users.noreply.github.com>
|
Closing this. #53 is implemented and shipped in v0.7.0, but not from this branch, and I want to be straight with you about why. The CLA was never signed. The bot asked on 2026-08-03 and there's no signature. I checked the authoritative store rather than going by the thread — Without a signature, the copyright assignment in the CLA never happened, so the code can't be merged or credited — that's the term the project runs on, not a judgement about the work. Separately, this branch had become unmergeable. It predates the The adapter that shipped (#102) was written from the existing On the implementation itself: it was good. The security posture in particular — withholding commit authority via If you'd like to contribute again: sign the CLA on your next PR (reply to the bot with the exact phrase it gives you) and it can be merged and credited normally. Happy to point you at something scoped — the board is at Signetry/signetry#10. Genuinely sorry this one ends here after the work you put in. |
2 participants
Summary
Validation
The full suite otherwise ran successfully except for nine pre-existing Windows-specific failures: checks that invoke the Unix true command and tests requiring unprivileged symlink creation.
Closes #53