Skip to content

fix: add upper bounds to Python dependencies - #42

Closed
mashkovd wants to merge 1 commit into
mainfrom
fix/agents-bound-dependencies
Closed

mashkovd wants to merge 1 commit into
mainfrom
fix/agents-bound-dependencies

Conversation

@mashkovd

Copy link
Copy Markdown
Member

Caps major versions (esp. claude-agent-sdk) so the harness can't silently break between builds.

All deps were floor-pinned (>=), so a new major of claude-agent-sdk (the agent harness) could silently break the loop between two identical image builds. Caps the major while keeping forward minor/patch range.
@claude

claude Bot commented Jun 12, 2026 •

Copy link
Copy Markdown

Claude finished @mashkovd's task in 22s —— View job


Review Complete

Analysis: Straightforward dependency bounds update adding upper-bound caps to prevent silent major-version breaks between builds.

Changes:

  • claude-agent-sdk>=0.1.68,<0.2 (caps at major version 0)
  • anyio>=4.0,<5 (caps at major version 4)
  • python-dotenv>=1.0,<2 (caps at major version 1)
  • PyYAML>=6.0,<7 (caps at major version 6)
  • Added clarifying comment explaining the rationale

All bounds are sensible (each caps at the next major version). This is good practice for pinning the harness stability.


✅ No P1/P2 findings, good to merge

@mashkovd

mashkovd commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

Superseded by #88. requirements.txt (the only file this PR touches) no longer exists on main — replaced by pyproject.toml + uv.lock. This PR's actual goal ("so the harness can't silently break between builds") is exactly what the lockfile achieves, and more completely: it pins the full transitive dependency tree (40 packages), not just upper bounds on the 4 direct ones. Closing as superseded.

@mashkovd mashkovd closed this Aug 2, 2026
@mashkovd
mashkovd deleted the fix/agents-bound-dependencies branch August 8, 2026 12:19
mashkovd added a commit that referenced this pull request Sep 14, 2026
`--apply` failed on `not report.planned`, and after the shepherd and
pr-steward rungs were removed that condemns the normal state of the
fleet: only live-DevLoop entities are imported, so "no live DevLoop
anywhere right now" means every plan is ambiguous and nothing is written
-- which is correct, not a failure.

The justification was wrong too, and wrong in a checkable way. It claimed
MCTL_TOKEN unset makes the probe answer UNKNOWN silently. It cannot: the
store read uses the same token, OwnershipClient raises
OwnershipUnavailable without one, every id reads UNKNOWN, and the run
ABORTS before any probe matters. The loud path was always the loud path,
and there is now a test that says so.

What must fail is an entity the run could not get an ANSWER about.
Ambiguity has two causes and they mean opposite things -- the probe could
not speak, versus no live DevLoop drives this -- so `Plan.undetermined`
separates them, on the wire as well as in the exit code.

plan_for's docstring still described a five-rung ladder with shepherd and
pr-steward in it, directly above the code explaining why those rungs are
gone. It describes the four that exist, and says why two of them both
report ambiguous.

Three guards from the same review:

_parse_pr_url walks four segments from the right and validates neither
host nor route, so `.../issues/42` parsed cleanly into the entity id of
pull request 42 -- a different thing. Fail-open on the read side, a
durable row against an unrelated entity here.

The org check became a whole-repo check. devloop_workflow_id builds
`dev-loop-mctlhq-{service}-{N}` from the proposal's directory, which is
the DevLoop's id only while service name and repo name are the same
string; the org alone let a proposal under agents-state/mctl-web whose
`pr:` points at mctlhq/something-else name another repository's workflow.

And the probe-budget test synchronises on an Event instead of racing a
200ms wall clock, where a loaded runner's pool startup would have read as
a regression in probe_all.

Refs: mctlhq/.github#57

Claude-Session: https://claude.ai/code/session_01RUQTbQvUQjwbDFd7Nq9bvC
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.

1 participant