# Session Log: P4 Credential Redesign, New Preflight Checks, Live-Readiness Check

**Date:** 2026-09-01 (continuation of the 2026-08-31 dry-run session)
**Author:** Claude (Claude Code session)
**Requested by:** Tom Tyler

---

## Context

Continuation of `sync_jira_to_p4jobs.sh` hardening, picking up after
`SESSION_2026-08-31_first_dry_run.md` (which found and fixed the
stdout-contamination silent-data-loss bug, change 33384). Tonight's work,
in order:

1. Aligned further with the SDP Bash Coding Standard and its template.sh
   (change 33383): colorized status output (`msg_green`/`msg_yellow`/
   `msg_red`), a `LOGGING:` section in `-man` documenting the self-logging
   behavior, `-D` for full `set -x` tracing, `get_old_log_timestamp()`
   inlined from the logging.lib equivalent, and moving the `EXIT`/`SIGINT`/
   `SIGTERM` trap to the very first line of Main Program so config/lock
   failures also complete through `terminate()` (previously they bypassed
   it and skipped the final summary line). Also fixed a pre-existing
   ShellCheck warning (dead pattern alternative in the status-mapping
   `case`) while at it.
2. **Scaled back the stdout/stderr fix per feedback.** The original fix for
   the stdout-contamination bug moved `msg()`/`dbg()`/`detail()` themselves
   to stderr -- correct, but "a bit extreme" for a two-call-site problem,
   and risked complicating future color-output handling for every caller.
   Reverted those shared functions to plain stdout and instead redirect
   only the ~4 specific `detail()`/`warnmsg()` calls inside `jira_get()`
   and `map_jira_project_to_p4()` (the two functions actually captured via
   command substitution) directly to stderr (change 33388). Saved as a
   general lesson in memory: prefer narrow, call-site-specific fixes over
   changing widely-shared helpers.
3. Fixed a misleading `Limited: no (-max 5)` summary line -- showing the
   configured cap when it wasn't actually the limiting factor implied it
   applied when it didn't (change 33389).
4. **Established a version-bump policy**, per Tom's request: bump the
   script's `declare Version="X.Y.Z"` (semver) with every submit that
   touches it -- Major/Minor/Patch per the usual rules. Caught up from the
   untracked original `1.0.0` to `2.0.0` to reflect the accumulated
   breaking changes already made this week (change 33386), then continued
   bumping with each subsequent submit. Saved as a standing memory so this
   isn't forgotten in future sessions.
5. **Replaced the P4 ticket-handling mechanism entirely.** Previously the
   script read a `P4PasswdFile` into a variable and passed it via
   `P4PASSWD`. Tom asked to stop passing a ticket at all and instead
   reference a `P4CONFIG`-format file. This went through two attempts:
   - **First attempt (wrong):** tried `p4 -config <path>`. That flag does
     not exist in this `p4` client (verified directly against `p4 -h` and
     `p4 help environment` -- `P4CONFIG` is only ever a *filename* p4
     discovers via directory-walk, not an absolute-path flag). Caught this
     myself before the first submit and switched to parsing
     `P4PORT`/`P4USER`/`P4TICKETS`/`P4TRUST` out of the file and exporting
     them per-call via `env` (this working version is what actually
     shipped as change 33390 / v3.0.0) -- but several `-man`/README
     *example* commands for one-time human setup still showed the
     hallucinated `p4 -config ... trust -y` / `login`, which Tom caught
     the next message ("Yikes, I see a hallucination in there").
   - **Second attempt (correct, change 33391 / v3.1.0):** per Tom's
     specific direction, `load_p4_config()` now just exports
     `P4CONFIG="$P4ConfigFile"` (an absolute `P4CONFIG` is used directly,
     no directory-walk, and its values take precedence over the ambient
     environment) and `P4ENVIRO=/dev/null/.p4enviro` (so no personal
     `~/.p4enviro` leaks in). `p4` itself then reads
     `P4PORT`/`P4USER`/`P4TICKETS`/`P4TRUST` from the file and resolves the
     ticket from `P4TICKETS` entirely on its own -- verified every piece
     (absolute-path `P4CONFIG` behavior, `P4ENVIRO` override, the
     `TicketExpiration` ztag field and its not-logged-in empty-string case)
     against the real `p4` client before writing any code, this time.
   - Per further feedback, the script doesn't track `P4PORT`/`P4USER`
     itself at all anymore -- every `p4` call picks them up from
     `P4CONFIG` directly -- and `load_p4_config()` doesn't validate the
     file's contents beyond confirming it exists; any misconfiguration
     surfaces naturally from the preflight checks that follow, rather than
     this script re-implementing p4's own validation.
   - Also fixed the example `P4PORT` from `ssl:workshop.perforce.com:1666`
     to `public.perforce.com:1666` -- the Public Depot isn't actually
     SSL-encrypted (it's a public, read-visible server) -- and changed
     `StateDir`'s default from `${P4HOME:-/p4/1}/tmp/jira_sync` to
     `${P4TMP:-/tmp}/jira_sync` (same value on a standard SDP instance-1
     host, cleaner fallback off SDP).
6. **Added two new preflight checks**, both bailing with a clear message
   before any syncing starts rather than letting every job upsert fail
   individually:
   - `check_p4_login()` (change 33391): verifies the configured P4USER has
     a ticket with at least 60 minutes remaining, via
     `p4 -ztag -F '%TicketExpiration%' login -s`. An empty result means not
     logged in at all.
   - `check_p4_access()` (change 33393): verifies `p4 protects -m` reports
     `admin` or `super` -- confirmed directly via `p4 help job` that
     `-f` (which this script always uses, to set
     `ReportedDate`/`ModifiedBy`/`ModifiedDate`) requires `admin`.

## Live-readiness check

Tom set up a dedicated service account, `bot_p4jira`, with its own
`P4TICKETS` file, and asked directly: have we pushed anything live yet, and
do we think it'll work? Answer: no live runs yet, everything has been
`-C ... -max N -v N` with no `-y`. Checked three things live against the
Public Depot (all read-only, zero risk) to answer honestly rather than
guess:

- `p4 jobspec -o`: real field names/values match exactly what the script
  writes (`Status`/`Type` enums, `perforce-software-sdp` present in the
  `Project` enum).
- `p4 job -o SDP-1172`: came back as a blank template, confirming most/all
  of a first live run would be *creating* jobs, not updating existing
  ones -- consistent with the tool's whole premise (Public Depot jobs
  stopped tracking JIRA a while back).
- `p4 protects -m`: initially reported `write`, not `admin` -- which would
  have made every live job upsert fail on the `-f` flag. Tom granted
  `admin` access mid-session; re-checked and confirmed `admin`. This is
  exactly the gap `check_p4_access()` (above) now catches automatically.

**Net: as of this session, the identified live-readiness blocker (insufficient
P4 access) has been resolved and both new preflight checks pass. No live run
has been attempted; Tom deferred that to a future session.**

## Design note for later: `-batch` as a separate option from `-max`

Tom observed that `-max` currently always blocks watermark advancement
(true, by design -- it exists as a pilot/safety-valve cap: touch a few real
issues, and if anything's wrong, rerun the exact same ones after fixing
config rather than skipping past unreviewed data). He raised a second, 
different use case: processing a large backlog in controlled batches (e.g.
50 at a time) where the watermark *should* advance after each clean batch,
making forward progress across repeated invocations. Proposed handling this
as a new `-batch` option, mutually exclusive with `-max`, rather than
changing `-max`'s existing behavior. Captured here, not implemented yet.

Three things worth working out before building it:

1. **Needs to be a distinct, explicit flag** (not a mode of `-max`) so the
   existing pilot-cap safety property doesn't quietly change for anyone
   already using `-max` that way.
2. **The watermark-advance target differs from a normal full run.** A full
   clean run advances to `RunStartTime` (everything up to "now" has been
   verified). A batch-capped run has only verified issues up to the
   `updated` timestamp of the last issue actually processed (in the
   `ORDER BY updated ASC` stream) -- advancing to "now" would skip
   unverified issues. `-batch` mode would need to track and persist that
   last-processed timestamp instead of reusing the existing
   `RunStartTime`-based logic.
3. **Multi-project runs make this a real correctness risk, not just an
   edge case.** The watermark is one shared value across every configured
   `JiraProjects` entry. If a batch cap cuts the run short partway through
   project B, with project C not yet even queried this run, advancing the
   shared watermark to B's cutoff point would silently skip project C's
   entire unprocessed backlog between the old watermark and that cutoff --
   C's own progress was never actually verified this run. Safest options:
   (a) only support `-batch` when a single project is targeted via `-p`
   (much smaller, safer first step), or (b) move to a per-project
   watermark store (bigger change -- the watermark file becomes keyed per
   project). Lean toward (a) if/when this gets built.

## Outstanding items (not done this session)

1. **`-batch` option** (above) -- design captured, not implemented.
2. **`curl --user` argv exposure** in `jira_get()`, symmetric to the P4
   ticket exposure already fixed -- still open, noted in
   `SESSION_2026-08-30_dry_run_and_next_steps.md`.
3. **`jira_type_to_p4()` "Feature Request" mapping gap** -- JIRA's actual
   "Feature Request" type falls through to the generic `Problem` default
   rather than `Feature`, since the case pattern only matches `"new
   feature"*`. Still open, noted in `SESSION_2026-08-31_first_dry_run.md`.
4. **`p4 fix` / `P4Blog` idea** -- filed as job **J2J-1** on the Public
   Depot server directly (no JIRA project for j2j itself): "Enhance to
   translate P4Blog from JIRA to P4 fixes." Design questions from the
   original session log
   (`SESSION_2026-08-30_dry_run_and_next_steps.md`) still apply.
5. **First live-fire test** -- readiness blockers now cleared
   (`bot_p4jira` has `admin` access; both new preflight checks pass), but
   deliberately deferred. Recommended starting point when ready: smallest
   possible blast radius, e.g. `-y -max 1`, before removing the cap.