Add AGENTS.md with agent/contributor guidance - #541
Merged
Conversation
Setup, commands, package layout, and the conventions that have caused real bugs when broken: libvips input-ref ownership (#531), band-count background vectors (#534), libvips version guards for new properties (#535), signature-based format detection over loader nicknames (#540). Also documents the golden-file scheme, leak detector, byte-identity requirement for streaming savers, and known macOS-local test failures. CLAUDE.md now defers to AGENTS.md for shared content and keeps only the Claude-specific worktree flow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| examples/ runnable examples (separate go modules) | ||
| ``` | ||
|
|
||
| `ImageRef` is a mutable handle: operations replace the underlying `VipsImage` via `setImage` (which unrefs the old one). `Close` and a GC finalizer release the ref; methods guard with `runtime.KeepAlive(r)` and `r.lock`. |
There was a problem hiding this comment.
🟡 Medium AGENTS.md:45
The statement that ImageRef methods are guarded with r.lock is inaccurate: only a small subset of methods (Close, SetKill, setImage, and streaming materialize/write paths) acquire that mutex, while normal transform/export/metadata methods do not. This can mislead contributors into assuming concurrent method calls (including concurrent Close) are safe, when the underlying mutable VipsImage pointer is not protected on most paths. Consider correcting the wording to state that r.lock guards lifecycle/ownership transitions only, not general method calls, and add a note that callers must not invoke methods concurrently on the same ImageRef.
Suggested change
| `ImageRef` is a mutable handle: operations replace the underlying `VipsImage` via `setImage` (which unrefs the old one). `Close` and a GC finalizer release the ref; methods guard with `runtime.KeepAlive(r)` and `r.lock`. | |
| `ImageRef` is a mutable handle: operations replace the underlying `VipsImage` via `setImage` (which unrefs the old one). `Close` and a GC finalizer release the ref. Only lifecycle/ownership transitions (`Close`, `SetKill`, `setImage`, streaming materialize/write paths) acquire `r.lock`; normal transform/export/metadata methods do not, so callers must not invoke methods concurrently on the same `ImageRef`. |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @AGENTS.md around line 45:
The statement that `ImageRef` methods are guarded with `r.lock` is inaccurate: only a small subset of methods (`Close`, `SetKill`, `setImage`, and streaming materialize/write paths) acquire that mutex, while normal transform/export/metadata methods do not. This can mislead contributors into assuming concurrent method calls (including concurrent `Close`) are safe, when the underlying mutable `VipsImage` pointer is not protected on most paths. Consider correcting the wording to state that `r.lock` guards lifecycle/ownership transitions only, not general method calls, and add a note that callers must not invoke methods concurrently on the same `ImageRef`.
davidbyttow
added a commit
that referenced
this pull request
Aug 8, 2026
r.lock only guards lifecycle transitions (Close, SetKill, setImage, streaming materialize/save); ordinary methods take no lock, so state plainly that an ImageRef is not safe for concurrent use rather than implying general methods are mutex-guarded. Flagged by Macroscope on #541. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an
AGENTS.mdso agents (and new contributors) get the repo's working knowledge up front:make test,make clean-cache,go generate ./vips), and that CI is the authoritative gate.OpenImageRefs/AssertNoLeaks), synthesized fixtures over committed binaries, byte-identity between streaming savers andExport*.CLAUDE.mdnow defers toAGENTS.mdfor the shared content and keeps only the Claude-specific worktree flow, so the two files can't drift.🤖 Generated with Claude Code
Note
Add AGENTS.md with contributor and AI agent guidance
Moves setup instructions, common commands, repo layout, cgo/memory rules, testing conventions (golden files, leak detection, streaming parity), and macOS quirks into a new AGENTS.md file. Updates CLAUDE.md to defer to AGENTS.md rather than duplicating that content.
📊 Macroscope summarized 52e336a. 2 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.