Record code runs with pull, commit and push

[krystophny]

Closes #2

Problem

A scientist running a recorded code run needs commands to manage their
work across a local working directory (RUN) and long-term storage (DATA).
Issue #2 specifies a workflow where the current directory is RUN:

1. dat pull <path> — pull required inputs/templates from DATA to RUN
2. run the job (e.g. slurm <run_code>)
3. dat commit -m <message> — record the started run in the database with
   status running
4. dat push — set status finished, update the database, and push
   results from RUN to DATA

Changes

• dat pull <path>: unidirectional copy of DATA/<path> into the
  current directory via rclone copy.
• dat commit -m <message>: writes the title (and optional body after
  a newline) plus automatically retrieved information (status running,
  start time) to metadata.json, and creates a database entry with status
  running through the eLabFTW API.
• dat push: sets status finished, records the finish time, updates
  the eLabFTW database entry, and unidirectionally copies the current
  directory from RUN to DATA via rclone.
• Metadata is stored in metadata.json using the eLabFTW standard
  (elabftw.extra_fields). DATA location is configurable via DAT_DATA
  (file system path or rclone remote); eLabFTW credentials come from
  ELAB_URL/ELAB_KEY environment variables.
• Added pytest coverage for pull, commit, and push, including message
  parsing, the running/finished status transition, and the rclone syncs.

Tests

python -m pytest tests/ -q — 7 tests pass.

[krystophny]

Review verdict: Request changes

Summary: The workflow is broadly implemented, but the eLabFTW metadata layout and push failure handling contain release-blocking correctness issues. No CI runs were available, and local pytest could not run because pytest is not installed.

Findings:

1. [major] src/dat/journal.py:39 — extra_fields is nested under metadata.elabftw, but eLabFTW requires it directly under metadata (with elabftw as a sibling). The status, message, and timestamps will therefore not be processed as custom fields. Move extra_fields to the required level.
2. [major] src/dat/dat.py:91 — push marks the run finished locally and in eLabFTW before rclone succeeds. A failed transfer leaves metadata claiming completion although results were not stored. Sync first, then finalize status, or implement explicit rollback/retry state handling.
3. [major] src/dat/dat.py:123 — pull accepts absolute and .. paths; os.path.join can escape both DATA_ROOT and RUN, potentially copying unintended data or overwriting outside the run directory. Require a normalized relative path and reject traversal.

Verdict: Request changes — the core metadata persistence and push consistency guarantees are not currently reliable.

[krystophny]

Review verdict: Request changes

Summary: The workflow is mostly coherent, but push leaves DATA with stale run metadata.

Findings:

1. [major] src/dat/dat.py:96-102 — rclone_copy() runs before the finished status and timestamp are saved to metadata.json. Consequently, DATA receives the old metadata (or none), while RUN and eLabFTW report finished. Save and sync the final metadata as part of the push, with appropriate failure handling.

Verdict: Request changes — push does not reliably persist the completed run state to DATA.

[krystophny]

Review verdict: Request changes

Summary: The workflow is implemented and path/metadata fixes are present, but push still leaves run state inconsistent when the DATA transfer fails. No CI checks were available; local tests could not collect because elabapy is unavailable.

Findings:

1. [major] src/dat/dat.py:97-104 — push saves status=finished locally before rclone_copy succeeds. If the transfer fails, RUN/eLabFTW can report a finished run while DATA lacks the results, and retrying sees no running run. Sync with failure-aware state handling, or roll back/retry the completion state when the transfer fails.

Verdict: Request changes — failed pushes can falsely finalize runs and lose synchronization with DATA.

[krystophny]

Review verdict: Request changes

Summary: The PR adds the requested commands and basic tests, but several core acceptance and data-safety requirements remain unmet. GitHub reports no CI checks, and local tests cannot collect without elabapy.

Findings:

1. [major] src/dat/dat.py:45 — rclone copy omits --ignore-existing and --immutable, so pull can overwrite local edits and push can overwrite existing DATA files. Add both flags and test the exact invocation.

2. [major] src/dat/dat.py:138 — dat pull <path> copies into RUN/<path>, producing ./templates/input.h5 instead of copying the requested DATA subdirectory’s contents into RUN. Use RUN as the destination.

3. [major] src/dat/dat.py:29 — globals() still exposes imported modules as commands; dat os raises a traceback instead of a clean unknown-command error. Use an explicit command map and exit status 2.

4. [major] src/dat/dat.py:98 — DATA receives metadata.json while its status is still running, then local metadata is changed to finished; successful archival data therefore permanently reports the wrong status. Ensure the final metadata is copied while preserving retry-safe failure handling.

5. [major] pyproject.toml:17, src/dat/journal.py:6 — the implementation depends on deprecated API-v1 elabapy, so the eLabFTW integration is not viable against the supported API. Migrate to elabapi-python and update the storage implementation accordingly.

Verdict: Request changes — the current implementation can overwrite data and persist incorrect run status, while the eLabFTW dependency is obsolete.