DEV Community

Cover image for I fixed what my agent left behind
owen adira
owen adira

Posted on Originally published at owenadirah.com

I fixed what my agent left behind

This is a follow-up to three posts about handing work to coding agents: "Fix the build" is not a task, Show your work and Where should a coding agent stop and ask?. All three used the same September session on schoolCRM, my school operations platform. I typed "fix it", an agent turned a failing test green, and it left an inconsistent API behind.

In November I went back to fix it properly. This time the agent got the brief I should have written in September. Here's what happened.

The leftover problem

In September, the agent changed three tests to expect 403 when a signed-in user tries to create a record they aren't allowed to create. That part was right. But the same wrong-user request on update and delete still returned 401, because three middleware functions hard-coded it. The tests passed, and the API said two different things about the same situation.

The brief

The new brief covered the parts "fix it" left out:

  • Goal: an authenticated user who fails an authorisation rule gets the same status on every route. Real authentication failures (a missing, invalid or expired token) stay 401.
  • Context: the files involved, the September commit and the error-code mapping.
  • Tests: a matrix test that calls every route as every kind of caller and asserts the status code, with the authorisation policy as the source of truth.
  • Boundaries: a feature branch and a pull request, never a merge. No changes to Git hooks or CI configuration. No --no-verify. Change existing test expectations only where the fix deliberately changes behaviour, and list each one.
  • Stop conditions: stop and report before editing if the policy disagrees with the brief, or if a route's current behaviour looks intentional.
  • Handoff: report back as Changed, Checks run, Not run or unfinished, and Questions for the reviewer.

The last two bullets did most of the work. Here's how the session went:

Three stops: the November session

Explore the interactive diagram of the three stops in the November session

Stop 1: the brief was wrong

The agent stopped before it changed a line of code. Its message started:

STOPPED before editing: changing wrong-user to 403 would reveal which record IDs exist.

It had found something I'd missed. Requesting a record that doesn't exist also returned 401. So in September's half-fixed state, a student asking for someone else's record and a student asking for a record that doesn't exist got the same answer. That's accidental, but useful: the API didn't reveal whether a record existed.

If I'd moved only the wrong-user case to 403, as my brief said, a 403 would mean "this exists, but it's not yours" and a 401 would mean "this doesn't exist". That's an existence oracle on every record route.

The agent laid out three options and their trade-offs. I chose a fourth: return 404 for both "not yours" and "doesn't exist", with an identical body, so the two can't be told apart. The finished rule looks like this:

```plain text
no, bad or expired token → 401 Unauthenticated
role denied → 403 PermissionDenied
record route: not yours OR doesn't exist → 404 NotFound, identical body
invalid ID in the path → 400 InvalidArgument
record lookup fails for another reason → 500 Internal





![How schoolCRM now answers a record request](https://owenadirah.com/blog/authz-status-rule.png)


[Explore the interactive diagram of how schoolCRM now answers a record request](https://owenadirah.com/blog/authz-status-rule.html)


If the agent hadn't stopped, it would have followed my brief exactly and shipped a security leak. The stop condition was there to catch the agent's mistakes. It caught mine.


## Stop 2: the hook was right


With the code done and the tests passing, the push failed. The pre-push hook runs `govulncheck`, and it found a vulnerability in Go's standard library. The fix was a newer Go toolchain.


The agent checked whether its own change had caused the failure. It ran the same scan on the untouched development branch, which failed the same way. Then it stopped:

> I haven't used --no-verify and haven't changed the hook.

In September, an agent in the same repository hit a failing hook and narrowed the hook until the commit got through. This time the brief said hooks were out of scope, so the agent reported the problem and offered options. The toolchain bump went into its own small pull request, which merged first. Then the auth change was rebased onto it and pushed normally.


## Stop 3: a question I'd have missed


The 404 rule had one awkward case. `PUT /users/role/{id}` is admin-only. Under the new rule, a student calling it on their own record would get 404 "user not found", for a record that obviously exists. The agent asked about it instead of guessing. The answer was to check the role before loading the record. A role denial says nothing about the record, so 403 is safe there.


It asked five other questions in the same handoff. Each took a one-line answer.


## Proof, not "Done!"


The handoff is where the difference from September shows most clearly.


**The test proves something.** The new matrix test calls 16 routes as 4 kinds of caller: anonymous, the owner, another student and an admin. It asserts each status code. For every record route, it also checks that "not yours" and "doesn't exist" return byte-identical responses. The agent then put the old middleware back temporarily, and 31 of the 84 checks failed. A test that passes with or without your fix proves nothing. This one only passes with it.


**Every changed test expectation is listed.** In September, three changed expectations sat inside a commit named after a hook. This time, the pull request has a table of every expectation that changed, with the before and after values. The commits are named after what they change.


**Checks run are kept apart from checks not run.** The handoff listed the full test suite run, the hooks and the environment it ran in. Under "Not run" it also listed one Dockerfile it hadn't built, and documentation that still mentions the old Go version.


**It checked a risk outside its own change.** The handoff asked whether the admin web app treats 401 specially, for example by signing the user out. It does, but only for a missing or expired token, so the new codes don't affect it. That was a question worth asking and quick to check.


## The rules are now in the repository


The same pull request added the rules to the repository's `AGENTS.md`, so the next agent starts with them instead of me retyping them:

- Ask before you change the expected result of an existing test.
- Ask before you change Git hooks, CI configuration or the scope of any check.
- Ask before you start or stop local services, such as Docker or a database.
- If a failing test and the code disagree, report which one you think is wrong and why. Then wait.

There's also a required handoff format, with the same four headings as the brief.


## What it cost


The agent stopped three times, and each stop needed me. A few small things fell through. One follow-up change arrived after its pull request had merged, so it became a separate pull request, and the agent couldn't open that one because its own permission settings refused. The September agent never interrupted me once, and it left a problem behind for two months.


The brief took a few minutes to write. Answering the stops took a few more. In exchange, I got a fix I could review in one sitting, a test that would have caught the original bug, and rules that apply to the next agent too.


The agent wasn't smarter this time. It had a clearer job, and it knew when to stop.
Enter fullscreen mode Exit fullscreen mode

Top comments (0)