Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent/go/cmd/agent/main.go`:
- Around line 26-31: The startup logging setup in main is introducing a global
slog default, which should be removed. Update main to avoid calling
slog.SetDefault and keep logging on the repo’s logr path by using
controller-runtime’s log.FromContext(ctx) / logr.Logger pattern instead of
process-wide logging state. Use the existing entrypoint in main and any
downstream logger wiring to pass an explicit logger boundary rather than relying
on hidden global defaults.

In `@agent/go/internal/config/config.go`:
- Around line 62-96: In Load, the config’s declared root_dir in cfg.RootDir is
used separately from the stepRootDir argument when calling step.Validate, which
can cause confusing path resolution drift. Add a brief guard or comment near
materialize and the step.Validate call to make the intended relationship
explicit, and if they are meant to match, assert that cfg.RootDir and
stepRootDir agree before validating steps.

In `@agent/go/internal/step/validate.go`:
- Around line 165-179: The checkStepFilesExist helper is treating every os.Stat
failure as a missing step, which masks permission or I/O problems. Update
checkStepFilesExist to distinguish os.IsNotExist(err) from other errors when
checking stepPath(s) under rootDir: only append truly missing paths to missing,
and return non-existence errors immediately so real filesystem failures are not
reported as “do not exist.”

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent/go/internal/config/config_test.go`:
- Around line 61-71: The current schema compilation tests cover valid and
unknown versions, but they do not verify byte-for-byte parity with the Python
source schemas. Add the schema drift golden test to config_test.go alongside
compiledSchemaFor/schema.V1 coverage so the embedded schema output is compared
against the expected Python-derived golden content and will fail on any
byte-level drift.

In `@agent/go/internal/config/config.go`:
- Around line 102-139: Dump currently serializes values that Load will later
reject because it does not validate the caller-supplied stage keys or the final
wire representation. Update Dump to validate stage values in the modes map and
ensure the marshaled wireConfig is accepted by Load before returning, using the
existing Dump and Load paths as the source of truth. Focus on the Dump function,
wireConfig construction, and any stage/scheme validation helpers so bogus
step.Stage values or invalid packageVersion inputs fail early.

In `@agent/go/internal/config/schemas/v1/skyhook-agent-schema.json`:
- Around line 7-20: The v1 schema contract is missing the root_dir field even
though Load, Dump, and the test fixture treat it as part of the config shape.
Update skyhook-agent-schema.json to declare root_dir in the properties for the
schema alongside schema_version, package_name, and package_version, and make it
required so malformed configs cannot pass validation; use the existing config
handling in Load/Dump to keep the schema aligned with the serialized model.

In `@agent/go/internal/step/validate.go`:
- Around line 137-159: The missing-check warning in warnMissingCheckPairs is
matching against all check stages, which lets an unrelated check suppress a
warning for the paired step. Update the logic so each step only looks for its
corresponding check stage derived from expectedCheckName(stepPath(s)) and
compare against the matching Stage rather than the aggregated checkPaths map.
Keep the warning behavior in warnMissingCheckPairs and use stepPath,
expectedCheckName, and IsStepStage to locate the affected flow.
- Around line 90-101: The check-only guard in checkNonCheckStagePresent is too
loose because it treats an empty non-check stage slice as present. Update the
logic to verify that at least one stage in steps has a non-empty []Step, rather
than only checking for the map key. Keep the existing ApplyToCheck and error
message behavior, but make the presence check ignore empty slices so configs
like apply: [] do not bypass validation.
- Around line 165-176: `checkStepFilesExist` currently trusts
`filepath.Join(rootDir, stepPath(s))`, so absolute paths and `../` traversal can
escape the package root. Add a guard before `os.Stat` that resolves each
`stepPath(s)` to a clean relative path and rejects any entry that is absolute or
climbs outside `rootDir`, then only validate paths that stay under the root.
Keep the existing missing-file collection and error reporting, but ensure
`stepPath(s)` is validated as an in-root path before it is checked.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent/go/internal/interrupts/interrupts.go`:
- Around line 57-71: In Decode, base64 decoding and json.Unmarshal currently
drop the underlying error and return only errInvalidSerializedInterrupt, which
removes useful diagnostics. Update the Decode function to wrap the original
decode/unmarshal error while preserving the sentinel behavior so callers can
still branch with errors.Is on errInvalidSerializedInterrupt. Keep the changes
localized to the Decode path and ensure both the base64.StdEncoding.DecodeString
and json.Unmarshal failure cases retain their original error context.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent/go/internal/config/config.go`:
- Around line 21-24: The config loading and validation flow still depends on
*slog.Logger and falls back to slog.Default(), so update the API to use
context.Context and logr instead. Thread the reconcile context through Load and
validateModes, and switch logging to log.FromContext(ctx) at the call sites in
config.go and validate.go. Remove the logger parameter/defaulting pattern and
ensure the existing load/validation paths get the context from their callers.

In `@agent/go/internal/config/validate.go`:
- Around line 299-308: expectedCheckName is deriving the suffix from the full
path, so dots in parent directories produce incorrect check names and false
warnings. Update expectedCheckName to compute the name from the basename only
(for example by separating directory and base first, then applying the “_check”
suffix to the base), and keep warnMissingCheckPairs using the corrected result
so paths like dir.v1/foo map to dir.v1/foo_check.

In `@agent/go/internal/step/step_test.go`:
- Around line 121-133: The ParseStage spec currently depends on the Stages
registry, so it only checks that ParseStage and Stages agree instead of
verifying the full expected set. Update the ParseStage test in step_test.go to
assert against an explicit list of known stage values rather than ranging over
Stages, using ParseStage and the existing stage constants/types to validate each
expected stage. Keep the unknown-stage assertion as-is so registry drift is
caught if a new Stage is added but not registered.

In `@agent/go/internal/step/step.go`:
- Around line 56-71: The stage registry is currently exposed as the exported
slice Stages, which lets other packages mutate the runtime validation set used
by ParseStage. Make the registry immutable by hiding the underlying list behind
an unexported variable or by returning a copied slice from an accessor, and
update any uses of Stages in step.go to go through the safe immutable path so
accepted wire keys cannot be changed at runtime.

---

Duplicate comments:
In `@agent/go/internal/config/config_test.go`:
- Around line 61-71: Add the missing schema-drift golden test to the existing
schema compilation specs so the embedded JSON is compared byte-for-byte against
the Python source schemas, not just compiled successfully. Update the "schema
compilation" Describe block in config_test.go to include a golden-style
assertion using compileSchema and the embedded schema constants (for example
schema.V1), and make sure the test fails when the embedded schema content drifts
from the source fixture rather than only on invalid JSON or version errors.

In `@agent/go/internal/config/schemas/v1/skyhook-agent-schema.json`:
- Around line 7-20: The v1 schema contract is missing the root_dir field even
though Config.MarshalJSON, Config.UnmarshalJSON, and the test fixtures already
treat it as part of the wire format. Update the schema definition in
skyhook-agent-schema.json to declare root_dir alongside schema_version,
package_name, and package_version, and make it a required property so malformed
configs are rejected during validation.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent/go/internal/config/config.go`:
- Around line 167-199: Dump currently only validates schema and misses the
cross-step checks that Load performs, so configs can serialize successfully and
still fail on reload. Update Loader.Dump to run the same validateModes logic
used by Load before marshaling, using the same inputs from modes and
expectedConfigFiles so cases like empty steps, missing paired check/apply
stages, or misplaced UpgradeStep are rejected up front. Keep the fix localized
to Dump and reference the existing Load/validateModes behavior to ensure the
Dump→Load contract stays consistent.

---

Outside diff comments:
In `@agent/go/internal/step/step_test.go`:
- Around line 146-159: The defaulting spec in Decode is still including the
legacy idempotence field, so it does not verify the missing-field fallback.
Update the “applies defaults to a minimal payload” test in step_test.go to omit
idempotence entirely when constructing the JSON passed to Decode, and keep the
assertion that RegularStep.Idempotence defaults to Auto. If you still want
coverage for the legacy boolean form, add a separate test case for that Decode
behavior.

---

Duplicate comments:
In `@agent/go/internal/config/config_test.go`:
- Around line 61-71: The current schema compilation tests in compileSchema only
verify embedded schema validity and do not protect against drift from the Python
source schemas. Add the schema-drift golden test from the PR scope alongside the
existing Describe("schema compilation") coverage so it compares the generated
embedded schema output against the Python source schema artifacts. Use the
existing compileSchema and schema.V1/schema.SchemaVersion symbols as the anchor,
and ensure the new test fails when embedded JSON diverges even if it still
compiles.

In `@agent/go/internal/config/config.go`:
- Around line 21-24: `Loader.Load`, `validateModes`, and `warnMissingCheckPairs`
are still tied to `*slog.Logger`, which violates the repo logging contract and
makes the config package harder to compose. Refactor these APIs to accept
`context.Context` and pull a controller-runtime `logr.Logger` via
`log.FromContext(ctx)` at the call sites, then thread the reconcile context
through instead of introducing `context.Background()` or any `slog` dependency
in this path.

In `@agent/go/internal/config/schemas/v1/skyhook-agent-schema.json`:
- Around line 7-20: The v1 schema contract is missing the root_dir field, so
configs can validate without populating Config.RootDir. Update the schema object
in skyhook-agent-schema.json to declare root_dir alongside the existing
properties, and mark it as required so the contract matches what config.go
serializes and config_test.go expects. Keep the change aligned with the schema’s
existing property naming and validation style.