Summary

Port the path/flag helpers and the per-package install-history ledger from agent/skyhook-agent/src/skyhook_agent/controller.py (lines ~203–253, ~349–438) into two small Go packages: internal/flags (paths, flag read/write, log file paths, log rotation) and internal/history (the per-package JSON ledger).

Both are filesystem-y, both are pure-function-heavy with t.TempDir() tests, and reviewers in this PR are looking at one mental model — "on-disk byte-parity with the Python agent". Combined size is ~250 LOC of source + tests.

Depends on #214 (the config.Config type that several path helpers take as a parameter).

Motivation

These helpers are tiny but high-stakes:

• MakeFlagPath derives a base64 marker from step.arguments and step.returncodes. If the marker bytes change, every existing flag on every node becomes invalid — the operator will think no step has ever run and re-run the entire fleet. We must reproduce Python's exact byte sequence.
• GetLogFile writes under a per-package, per-version directory tree that operators and kubectl skyhook walk for log retrieval. Path drift breaks observability silently.
• CleanupOldLogs keeps disk bounded; if it stops working the agent slowly fills the host disk.
• The history file is the only persistent, non-flag state the agent carries on the host. It drives upgrade-script behavior — packages use PREVIOUS_VERSION to decide upgrade-vs-fresh-install paths — so misreading it causes upgrade scripts to do the wrong thing silently.
• The history file's corrupt-recovery semantics (rename to .backup, re-init) are intentional and exist because parsing failures on a bad disk write would otherwise wedge a node permanently.

Feature description

Two new packages with byte-parity tests against Python.

Proposed direction

1. Path helpers (pure)

In internal/flags/paths.go:

• GetSkyhookDir(rootMount string) string — {rootMount}{SKYHOOK_ROOT_DIR} where SKYHOOK_ROOT_DIR defaults to /etc/skyhook.
• GetFlagDir(rootMount string) string — {skyhookDir}/flags.
• GetHistoryDir(rootMount string) string — {skyhookDir}/history.
• GetLogDir(rootMount string) string — {rootMount}{SKYHOOK_LOG_DIR} where SKYHOOK_LOG_DIR defaults to /var/log/skyhook.
• GetHostPathForSteps(copyDir string) string — {copyDir}/skyhook_dir.
• GetLogFile(stepPath, copyDir string, cfg *config.Config, rootMount string, timestamp string) string — {logDir}/{packageName}/{packageVersion}/{trimmed-step-path}-{timestamp}.log. If timestamp == "", default to time.Now().UTC().Format("2006-01-02-150405") exactly matching Python's %Y-%m-%d-%H%M%S. Creates the parent dir.

Env defaults read via a small internal/envcfg helper (or whatever #213 named the env-config seam) — do not read os.Getenv directly inside these functions; pass the resolved values in. Easier to test.

2. Flag helpers

In internal/flags/flags.go:

• MakeFlagPath(step Step, cfg *config.Config, rootMount string) string — the marker is base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s_%s", argsRepr, returncodesRepr))) where argsRepr and returncodesRepr reproduce Python's str(list) repr (single quotes, spaces after commas). This is the parity-critical piece. Add helpers pythonListRepr([]string) string and pythonIntListRepr([]int) string and golden-test against fixtures generated by Python.
• SetFlag(flagFile, msg string) error — os.MkdirAll + os.WriteFile.
• CheckFlagFile(step Step, flagFile string, alwaysRunStep bool, mode Mode) bool — port the Python rules:
  • Flag does not exist → false (run).
  • OVERLAY_ALWAYS_RUN_STEP set → false, with a warning print.
  • Mode is CONFIG, UNINSTALL, or UPGRADE → false, with a print explaining "no idempotence support".
  • step.Idempotence == Disabled → false, with a print.
  • Otherwise → true (skip).

Reproduce the print messages — operator e2e logs grep for them.

3. Log rotation

CleanupOldLogs(logFileGlob string) — sort matches by mtime descending, remove everything past the 5th (keep current + 4 previous, matching Python).

4. internal/history package

Two entrypoints matching Python signatures from agent/skyhook-agent/src/skyhook_agent/controller.py lines 376–438:

• Read(rootMount string, cfg *config.Config, step *step.Step) error — populates step.Env["CURRENT_VERSION"] / step.Env["PREVIOUS_VERSION"] and, for UpgradeStep, appends [previous, current] to step.Arguments.
• Write(rootMount string, cfg *config.Config, mode step.Mode) error — appends an entry and updates current-version. Mode UninstallCheck writes "uninstalled"; everything else writes cfg.PackageVersion.

File shape

{
  "current-version": "<semver string>",
  "history": [
    {"version": "<semver string>", "time": "<ISO-8601 UTC>"},
    ...
  ]
}

New entries prepend (insert at index 0). Time format is time.Now().UTC().Format(time.RFC3339Nano) to match Python's datetime.now(timezone.utc).isoformat().

Corrupt-recovery semantics

If the file exists and json.Unmarshal returns an error:

1. Rename {file} → {file}.backup.
2. Print exactly: Failed to load existing history, corrupt history file moved to {file}.backup....
3. Treat as if the file never existed (current-version: "", empty history).

Add a // why: comment naming the constraint: a partially-written history file from a prior crash must not wedge the node.

Missing-file semantics

If the file does not exist at all, set current-version: "unknown" and print no existing history found for {packageName}, could be first installation... exactly. Do not create the file in this branch — only Write creates it.

Read mode behavior

For both step.Step and step.UpgradeStep:

• step.Env["CURRENT_VERSION"] = cfg.PackageVersion
• step.Env["PREVIOUS_VERSION"] = historyData.CurrentVersion

For step.UpgradeStep only, additionally append [previous, current] to step.Arguments. (Python uses isinstance(step, UpgradeStep); in Go this is whatever type discriminator #213 settled on.)

Write mode behavior

• mode == Mode.UninstallCheck → currentVersion = "uninstalled", append {version: "uninstalled", time: now}.
• Else → currentVersion = cfg.PackageVersion, append {version: cfg.PackageVersion, time: now}.

Match Python's plain open(file, "w") write (non-atomic) for parity. If we want atomicity ({file}.tmp + rename) post-cutover, file as a follow-up.

5. Tests

Port the path/flag/history cases from agent/skyhook-agent/tests/test_controller.py. Specifically include:

• A golden parity test for MakeFlagPath covering at least 20 representative (args, returncodes) combinations including: empty args, args with spaces, args with quotes, multi-element returncodes. Failing this test means an in-flight rollout would skip-or-rerun mistakenly after cutover.
• A test that GetLogFile with no timestamp produces a path matching ^.*-\d{4}-\d{2}-\d{2}-\d{6}\.log$.
• CheckFlagFile returning the Python-equivalent value for all four code paths.
• CleanupOldLogs keeps exactly 5 files when 10 exist (5 newest by mtime).
• History: read with no file → "unknown" current; valid file → env populated; corrupt file → .backup rename + "unknown"; write apply-check → entry prepended; write uninstall-check → "uninstalled" entry; UpgradeStep Read appends args.

Scope boundaries

In scope:

• Path helpers, flag read/write/check, log file path computation, log rotation.
• History file read/write with corrupt-recovery.

Out of scope:

• Writing log content ([FEA]: tee streaming and run_step #217 — the tee streaming).
• Step execution ([FEA]: tee streaming and run_step #217).
• Calling Read / Write from the controller ([FEA]: Controller main / agent_main / SIGTERM + CLI parsing and entrypoint banner #219).

Acceptance criteria

• All paths produced match the Python equivalents byte-for-byte against a generated fixture set.
• MakeFlagPath parity test passes for at least 20 combinations.
• CheckFlagFile returns the same value as Python for all four code paths.
• CleanupOldLogs keeps exactly 5 files when 10 exist.
• All ported history test cases pass.
• A test asserting that a corrupt history file is renamed to .backup and that subsequent Read returns "unknown" passes.
• No env vars read directly inside helpers; all configuration is parameter-passed.
• Output strings match Python verbatim where the test pins them.

Open questions

• Should the MakeFlagPath parity helper live behind a build tag so the marker format can never silently change? Recommend yes — a single flag_marker_test.go with a hard-coded golden table is the canary.
• The Python helpers print plain text (fmt.Println) for the CheckFlagFile cases. Go convention says use a logger. Recommend matching Python's print form to keep operator log greps working — comment it as // why: log line is part of the public contract; operator e2e greps it.
• History atomicity: keep Python's non-atomic write or improve to .tmp + rename? Default to matching Python; revisit if e2e flakes show partial writes in practice.
• Should Read mutate the passed-in *step.Step (Python's behavior) or return values for the caller to apply? Python mutates; Go convention prefers immutable inputs. Recommend mutate for parity simplicity, with a comment.

References (codebase)

• agent/skyhook-agent/src/skyhook_agent/controller.py lines 203–253, 349–438.
• agent/skyhook-agent/tests/test_controller.py.

Alternatives considered

• "Improve" the marker format to a hash (e.g. SHA256 of args+returncodes). Rejected — backward-incompatible with deployed Python agents; would force a node-state migration and a coordinated operator+agent release.
• Store history as Node annotations. Rejected as scope creep — would change the agent/operator contract.
• Splitting flags and history into separate PRs. Rejected — both are small filesystem-y modules with the same review mental model; one PR is faster to land.

Code of Conduct

• I agree to follow Skyhook's Code of Conduct.