Make the faction leader the default holder of special positions

[XxFran10xX]

Summary

• Leader is always the default holder. Any vacant special position falls to the faction leader, whatever the faction's size. This replaces the old rule that a leader could only hold the Spymaster office in a one-person faction.
  • The office falls back to the leader on founding, on dismissal, when the appointee dies or leaves, and when the leadership changes. A default holding moves to the new leader; a deliberate appointment stays.
  • Until the leader has a live active character, the office waits for them, as founding already did. While it waits, the vacancy penalty and the "unguarded" exposure still apply.
  • /faction spymaster remove with nobody appointed now says the office already rests with the leader, instead of emptying it.
• Aptitude per office held. A character's aptitude now drops by espionage.aptitude.extra-position-penalty (default 0.25) for each office they hold beyond their first: 1 office 100%, 2 offices 75%, 3 offices 50%, and never below 0%. Setting it to 0 disables the penalty. It applies to any holder, the leader included, so a leader with a single office keeps full aptitude (the old rule cut a solo leader to 25%).
  • Only the Spymaster exists today, so the penalty takes effect once more offices are added.
• Config cleanup. The retired espionage.aptitude.solo-leader-multiplier key, and its misleading comment, is removed from existing special-positions.yml files on load. The new key is added with its comment.
• GUI and chat text updated: office lore, the remove button and the death message.

Tests

• Rewrote the four tests that encoded the solo-leader rule.
• Added tests for the multiplier curve and config bounds, the leader keeping the office after others join, the default following a new leader while appointments stay, any vacancy falling to the leader, a vacant office in a 3-member faction being assigned to the leader's character at full aptitude (without using the free appointment), the leader being unable to dismiss their own default holding, and removal of the retired config key.
• mvn clean verify locally: 2692 tests, 0 failures.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: d4dea106-4069-4734-9aad-228695892421

📥 Commits

Reviewing files that changed from the base of the PR and between c943cb2 and 75b5569.

📒 Files selected for processing (10)

• src/main/java/net/tfminecraft/simplefactions/espionage/EspionageConfig.java
• src/main/java/net/tfminecraft/simplefactions/espionage/EspionageService.java
• src/main/java/net/tfminecraft/simplefactions/espionage/SpecialPositionsConfigFile.java
• src/main/java/net/tfminecraft/simplefactions/managers/inventory/EspionageView.java
• src/main/resources/special-positions.yml
• src/test/java/net/tfminecraft/simplefactions/espionage/EspionagePermissionsTest.java
• src/test/java/net/tfminecraft/simplefactions/espionage/OfficeAppointmentsTest.java
• src/test/java/net/tfminecraft/simplefactions/espionage/OfficePersistenceTest.java
• src/test/java/net/tfminecraft/simplefactions/espionage/SpecialPositionsConfigFileTest.java
• src/test/java/net/tfminecraft/simplefactions/espionage/UnguardedFactionTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• New Features

  • Vacant Spymaster offices are automatically assigned to the faction leader when they have an active character. Leader appointments remain valid as faction membership changes.
  • Holding multiple offices now reduces aptitude for each additional office, according to a configurable penalty.
  • Removing an appointed Spymaster returns the office to the leader.

• Updates

  • The office menu now explains automatic assignments and displays the impact of holding multiple offices.
  • The aptitude setting has been renamed; existing configurations are updated to use the new setting.

Walkthrough

Spymaster offices now fall back to the faction leader when vacant. The aptitude setting now applies a penalty for each additional office held. Configuration migration, menu text and tests reflect these changes.

Changes

Spymaster offices

Layer / File(s)	Summary
Multi-office aptitude and configuration src/main/java/net/tfminecraft/simplefactions/espionage/EspionageConfig.java, src/main/java/net/tfminecraft/simplefactions/espionage/EspionageService.java, src/main/java/net/tfminecraft/simplefactions/espionage/SpecialPositionsConfigFile.java, src/main/java/net/tfminecraft/simplefactions/managers/inventory/EspionageView.java, src/main/resources/special-positions.yml, src/test/java/net/tfminecraft/simplefactions/espionage/EspionagePermissionsTest.java, src/test/java/net/tfminecraft/simplefactions/espionage/SpecialPositionsConfigFileTest.java	The configuration replaces the solo-leader multiplier with an extra-position penalty. Aptitude now decreases for each additional office held, and the menu reports multiple-office aptitude and assignment status. The configuration loader removes the retired key. Tests cover the penalty and configuration migration.
Leader fallback and office lifecycle src/main/java/net/tfminecraft/simplefactions/espionage/EspionageService.java, src/test/java/net/tfminecraft/simplefactions/espionage/EspionagePermissionsTest.java, src/test/java/net/tfminecraft/simplefactions/espionage/OfficeAppointmentsTest.java, src/test/java/net/tfminecraft/simplefactions/espionage/OfficePersistenceTest.java, src/test/java/net/tfminecraft/simplefactions/espionage/UnguardedFactionTest.java	Leaders are eligible to hold the office, and vacant or invalid Spymaster assignments fall back to the leader. Removal restores the leader’s default holding. Tests cover appointment state, leader changes, persistence and faction protection.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Faction
  participant EspionageService
  participant Bukkit
  participant Leader
  participant Character
  participant FactionSave
  Faction->>EspionageService: report vacant Spymaster office
  EspionageService->>Bukkit: check server availability
  EspionageService->>Leader: resolve faction leader
  Leader->>Character: provide active character
  EspionageService->>Faction: assign leader's character
  EspionageService->>FactionSave: save changed office state

Loading

Merge Risk: ⚪ Minimal · up to 75b55

The leader fallback and appointment-removal behavior are consistent with the intended change. No identified issue remains that should prevent merging after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 75b55

Default leader protection can disappear after a leadership change until another action reinitializes the office, allowing outsiders to see exact faction information in the meantime. Appointment and removal authority remain restricted to the faction leader.

Retained concerns

• Medium · security · inferred: Leadership transfer invalidates the automatic Spymaster before establishing the successor's holding. The leadership command updates the leader and returns without office reconciliation; hasSpymaster then returns false, and exact-information reads do not initialize fallback. Consequently, outsiders can receive exact faction and guild information until a separate spymaster call repairs the office, even if the new leader already has a live active character. The base accepted a living former leader who remained a member, so this automatic-holder invalidation introduces a distinct exposure transition rather than merely preserving the documented no-character vacancy policy.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is faction information, including guild rosters and exact numeric values routed through the shared predicate. A viewer need not be a member of the affected faction. Leadership transfer requires leader authority, or administrative authority for the inspected forced-transfer command; no server credential or infrastructure privilege gain was established.

Security Findings and Attack Paths

• inferred — After a leadership transfer with an automatic holding, an outsider can reach an exact-information read while the old holding is invalid and the successor is not yet assigned. That read permits exact data instead of repairing the office. Intelligence refresh can close the interval, but returns immediately for a viewer without an observing faction, so it is not a universal recovery prerequisite.

Trust Boundaries and Controls

• observed — The PR generally strengthens protection by allowing a living current leader to remain Spymaster after other members join. Pending, deceased and departed holders remain unguarded under the existing information-access policy. Fallback resolves the current leader, not an attacker-supplied identity or the stored pending character identifier.

Hardening Proposals

• proposed — Make leadership transfer reconcile the successor's default office before subsequent reads can classify the faction as unguarded. Preserve the intentional pending policy when the successor lacks a usable character. Treat dismissal and fallback as an explicit recoverable transition, with checked persistence of the terminal state.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}